-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(proxy-wasm) set querystring when setting ":path" #627
Conversation
...to clarify that it matches `":path"` (that is, path including querystring), making the field name match the pseudo-header, as is the case with the other entries in `ngx_http_proxy_wasm_dispatch_s`.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #627 +/- ##
===================================================
- Coverage 90.84180% 90.82724% -0.01457%
===================================================
Files 52 52
Lines 11214 11218 +4
===================================================
+ Hits 10187 10189 +2
- Misses 1027 1029 +2
... and 3 files with indirect coverage changes
Flags with carried forward coverage won't be shown. Click here to find out more.
|
… passed through. This matches the plain Nginx behavior, and showcased in one of the tests.
|
||
|
||
|
||
=== TEST 17: proxy_wasm - showcase that that path with invalid querystring passes through |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to make tests 16 and 17 into a single test using --- request eval
and ["GET /set_by_proxy_wasm", "GET /raw_nginx"]
. That worked in isolation, but it messed up the plan_tests
count.
Merged! |
:path
to dispatch already worked; I added a test to verify this.":path"
(that is, path including querystring), making the field name match the pseudo-header, as is the case with the other entries inngx_http_proxy_wasm_dispatch_s
.