-
Notifications
You must be signed in to change notification settings - Fork 837
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
Make Exists Mode for QueryParameters support empty parameters #2568
Comments
FWIW the query parameters here is matching what we do for headers: reverse-proxy/src/ReverseProxy/Routing/HeaderMatcherPolicy.cs Lines 78 to 83 in b1a1d8b
reverse-proxy/src/ReverseProxy/Routing/QueryParameterMatcherPolicy.cs Lines 77 to 81 in b1a1d8b
and the behavior was intentional for headers at least. |
Why do you assume the behavior was intentional for headers and not just the easiest approach? To me it sounds a bit counter-intuitive that Exists goes on whether it has a value instead of whether it actually is present or not - both for headers and query parameters. |
It was intentional for headers. Empty headers have no semantic meaning and may be dropped by intermediaries. The query situation is a bit different. We assume key-value pairs (?key1=a&key2=b), but having a single key without an equals sign is still meaningful (?key1&key2). AspNetCore isn't great at representing this case, it doesn't distinguish between |
Discussed in #2553
Originally posted by ppv-milestone July 17, 2024
We need to match queries to an interface that accepts a request like this:
http://path/component?hello
But according to documentation the "Exists" Mode in QueryParameters will only match against non-empty parameters and our tests show that Prefix, Exact and Contains seem to be following the same (providing "" as a value to match for causes the service to fail).
Also tried with regular expressions in Path, but those does not seem to match on parameters.
Anyone has a workaround for this? Or maybe supporting empty parameters for Exists could be supported in next version?
Thanks a lot in advance!
Br,
Peter
The text was updated successfully, but these errors were encountered: