Skip to content
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

Add support for parsing URL query parameters with no value #3053

Open
calebhailey opened this issue Aug 9, 2023 · 9 comments
Open

Add support for parsing URL query parameters with no value #3053

calebhailey opened this issue Aug 9, 2023 · 9 comments
Labels
enhancement New feature or request

Comments

@calebhailey
Copy link

calebhailey commented Aug 9, 2023

Is your feature request related to a problem? Please describe.

I'm developing an HTTP API using Vapor where some endpoints may offer support for URL query parameters that have a key but no value (e.g. ?foo vs ?foo=bar). As far as I can tell, the URI Spec doesn't specify that query parameters must have a value, so my assumption was that this should be possible.

Describe the solution you'd like

It should be possible to decode query parameters and capture keys with no values:

var params: Dictionary<String, String?> = try req.query.decode(Dictionary<String, String?>.self)

Describe alternatives you've considered

This works as expected but it appears to skip query params with no values:

var params: Dictionary<String, String> = try req.query.decode(Dictionary<String, String>.self)

I've tried switching the Dictionary value type to an optional string and it still won't decode url query parameters without values

var params: Dictionary<String, String?> = try req.query.decode(Dictionary<String, String?>.self)

NOTE: I suspect a Struct attribute with an optional value would work, but I have not tried decoding to a struct because in my case I need support for dynamic query parameter keys, thus the Dictionary<String, String>.

Additional context

Vapor appears to parse url params as long as there is an =; so ?foo= works, but ?foo does not get parsed.

PS – I'm new to Vapor but I'm loving it so far! Keep up the great work! 💪

EDIT: added link to the URI Spec

@calebhailey calebhailey added the enhancement New feature or request label Aug 9, 2023
@jhoughjr
Copy link

jhoughjr commented Aug 9, 2023

I've checked URLEncodedFormParser.parse and its behavior appears correct. I havent found the next responsible code in the chain. My suspicion is it makes no sense for the decoder to decode a key as a value maybe. Maybe bogging the key into the value would behave properly by falling through the existing logic. I cant say until I find the chain of execution.
It may even be it doesnt try to decode it as a dictionary of strings as a dictionary with a key and no value is undefined? I'll keep looking.

@jhoughjr
Copy link

jhoughjr commented Aug 9, 2023

let decoder = _Decoder(data: parsedData, codingPath: [], configuration: configuration)
    return try D(from: decoder) this init and decode appears to be what fails.
(lldb) po D.self
Swift.Dictionary<Swift.String, Swift.Optional<Swift.String>>```

```(lldb) po parsedData
▿ URLEncodedFormData
  ▿ values : 1 element
    ▿ 0 : URLQueryFragment
      - urlEncoded : "hello"
  - children : 0 elements
  - maxRecursionDepth : 100```
  

@jhoughjr
Copy link

jhoughjr commented Aug 9, 2023

Decoding string.self does capture it solo or with keyed values but it seems to capture the last string in the URI. I'm still not sure what the solution should be. Perhaps I should check the RFC.

@mkll
Copy link
Sponsor

mkll commented Aug 9, 2023

Are you guys sure that "the URI Spec doesn't specify that query parameters must have a value" also mean that it may not have the = sign after the key? In my experience, I often see key= in the wild, but so far I have never seen just key.

@jhoughjr
Copy link

jhoughjr commented Aug 9, 2023

Checking the RFC it only indicates that key value pairs are commonly used and does not appear to state any requirements on the structure of the query string other than its termination and how to specify pairs. I think we need to confirm this shold be supported before doing much more work. I think it is a conceptual bug more than an implementation bug perhaps.

In my opinion treating flags as bool arguments should be a workaround for getting flags from query params.

@calebhailey
Copy link
Author

This won't make or break my project, it was more of a curiosity whether this was an oversight/omission in the query parsing code, or an active design decision. I don't have any precedent to refer to in terms of patterns in other projects, so maybe all that suggests this is a bad idea 😅

@calebhailey
Copy link
Author

But the spec is pretty clear about the query component definition:

The query component is indicated by the first question mark ("?") character and terminated by a number sign ("#") character or by the end of the URI.

For reference, I typically defer to MDN docs and/or Javascript implementations as a battle-tested standard, and it appears that JavasScript will parse valueless query parameters.

var u = new URL("https://example.com/?foo=bar&baz")
console.log(u.searchParams) // outputs: URLSearchParams { 'foo' => 'bar', 'baz' => '' }

I get the same result in Safari and NodeJS repl.

@calebhailey
Copy link
Author

calebhailey commented Aug 9, 2023

Found a workaround via the Vapor.Request.url property. I can get at the raw query that way.

let u: URLComponents? = URLComponents(string: String(describing: request.url))

The Foundation.URLComponents class captures query params without values, so it works exactly as I was hoping.

EDIT: I'll leave it up to the Vapor team to decide if they want to pursue this enhancement or close it.

Cheers ✌

@jhoughjr
Copy link

I'm not sure if the current behavior is correct or not. I'm not sure what would change.
Since you have a workaround I vote it can be closed unless more specific action is determined.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants