-
Notifications
You must be signed in to change notification settings - Fork 9.1k
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
How are conflicts between allowReserved
and RFC 3986 handled?
#3759
Comments
|
My $0.02: dump How I've handled this in my implementation: I parse the URL string after it has been normalized (i.e. any characters that should become encoded are encoded; any that are encoded needlessly are decoded). |
Also, using |
Here is one of the many examples I have seen APIs being designed using RFC3986: https://developers.google.com/maps/documentation/aerial-view/web-api-best-practices?hl=en Ofcourse there are always exceptions, but I am going IMHO for the greater good. So, my $0.02 as well would be to comply with RC3986 and eliminate |
Ugh. Digging through the history I found OAI/learn.openapis.org#100 that indicates However, instead of relying on RFC 3986's segment = *pchar
segment-nz = 1*pchar
segment-nz-nc = 1*( unreserved / pct-encoded / sub-delims / "@" )
; non-zero-length segment without any colon ":"
query = *( pchar / "/" / "?" )
fragment = *( pchar / "/" / "?" )
pchar = unreserved / pct-encoded / sub-delims / ":" / "@"
pct-encoded = "%" HEXDIG HEXDIG
unreserved = ALPHA / DIGIT / "-" / "." / "_" / "~"
reserved = gen-delims / sub-delims
gen-delims = ":" / "/" / "?" / "#" / "[" / "]" / "@"
sub-delims = "!" / "$" / "&" / "'" / "(" / ")"
/ "*" / "+" / "," / ";" / "=" This seems to be because some of the But allowing reserved characters allows all However, in §3.2.1 Variable expansion, we find this as the third paragraph:
I find this a bit hard to parse, but I think the key is that last sentence: when using I don't see any examples of this in RFC 6570, and this is pretty buried in the text. I also notice that while we correlate some other things with RFC 6570 syntax, we do not do so for Anyway, @karenetheridge @miqui we can't remove this keyword because it's the only way to get certain valid parameter values serialized correctly. But we need to be a lot more clear on the different times at which encoding happens in various serialization configurations. |
So what I've settled on is that this is intended to be used for when you need to pass through some allowed reserved characters (and most of them are allowed in the query string). But when you do that, it is your responsibility to make sure that any illegal reserved characters are encoded separately. Since RFC6570 reserved expansion leaves percent-encoded sequences untouched, the safest and easiest way to do this is to do your percent-encoding manually before RFC 6570 value expansion.
|
PRs merged for 3.0.4 and ported to 3.1.1 via PR #3921! |
The
allowReserved
keyword in the Parameter Object is specified as follows (from 3.1.0):This is strange in several ways. Most of those characters are not forbidden in query strings. Here is the relevant ABNF:
What this means is that the actual set of characters not allowed in the query string by RFC 3986 is just
#
,[
, and]
. Everything else is allowed by either thepchar
production or thequery
production.It does not make sense to allow
#
in a query string value because it will cause everything after it to parse as a fragment. The OAS cannot change that behavior, and implying that we can is problematic at best.While the failure mode of including
[
or]
is less clear (they're only used for IPv6 or IPvFuture literals in the ABNF), it's still strange to encourage something that correctly implemented URI parsers will reject (admittedly, the percentage of URI parsers that are implemented correctly is depressingly low).So...
[
or]
in a query parameter value? How are correct URI parsers expected to handle this? Are we requiring tools to parse URIs incorrectly? We might need some guidance here - I would have no idea what to do with this, or whether to validate it in OASComply#
here at all as it's not going to work even in poorly-implemented URI parsers. Since we can't technically remove the feature, we should put a strongly worded SHOULD NOT around itThe text was updated successfully, but these errors were encountered: