-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
'Flag' URL Query params don't decode into structs #3163
Comments
While I agree that in general feature parity is a good thing, in this case I'm not sure I'm behind it. The existing flag behavior is already a bit odd, but I went to the trouble of fixing it because it was previously supported functionality that I'd later broken. This would be, as far as I'm aware, a completely new and potentially unexpected behavior that changes the semantic meaning of existing code 😕. @0xTim Any thoughts? |
For this I'm actually in favour of it. Assuming we don't break any existing decoding, I think defaulting to a |
I think the saving grace here is that implementations that decode flag query parameters into anything other than bools are already using custom decoding. If someone specified a route's query to allow In the case of URL queries, then, the effect of this change is that a client that has been sending a flag parameter to a Vapor route that used struct decoding and decoded the flag into an optional Bool value would start decoding this value instead of ignoring it. Decoding into a non-optional Bool via struct decoding works currently and would continue to work (but is not often used for URL queries). Similarly, routes that current specify I don't believe actual HTML forms ever produce flag values with |
Describe the issue
req.query.decode(Struct.self) fails to decode '?flag1' query into 'let flag1: Bool?' property
Vapor version
4.92.5
Operating system and version
mac OS 14.3.1
Swift version
Apple Swift version 5.9.2
Steps to reproduce
vapor new hello -n
.?flag1
query:Outcome
It's not feasible for a generalized decoder to capture all the possible semantic meanings of a query; a query that relied on order of query params couldn't use either single-value decoding nor struct decoding. There's no standard here; the query coders are a utility to help with commonly-used cases.
But single-value and struct decoding should work the same, as much as they can. 4.92.2 fixed flag query params for the single-value decode method; struct decoding (into optional Bools) ought to have parity in this case.
Additional notes
Cause of this issue appears to be that
URLEncodedFormParser
parses flags intoURLEncodedFormData.values
and key-value params intoURLEncodedFormData.children
whileURLEncodedFormDecoder
's_Decoder.KeyedContainer.contains()
only checks.children
. This makesdecodeIfPresent()
always return nil for flags._Decoder.KeyedContainer.decode()
contains a case for decoding flag values into Bools, so decoding into a non-optional Bool works.PR incoming.
The text was updated successfully, but these errors were encountered: