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

Sparse field sets examples not RFC 3986 compliant #647

Closed
jaspervz opened this issue May 21, 2015 · 5 comments
Closed

Sparse field sets examples not RFC 3986 compliant #647

jaspervz opened this issue May 21, 2015 · 5 comments

Comments

@jaspervz
Copy link

The current examples for sparse field sets use non-RFC 3986 compliant URLs: [ and ] are reserved characters and must be percent encoded.

The RFC 3986 compliant version of the example is: GET /articles?include=author&fields%5Barticles%5D=title,body&fields%5Bpeople%5D=name

Should [ and ] be used to denote the type if the client needs to encode these? At least the example needs to be updated because it isn't RFC 3986 compliant.

See: https://www.ietf.org/rfc/rfc3986.txt
3.4 Query:
query = *( pchar / "/" / "?" )

And
pchar = unreserved / pct-encoded / sub-delims / ":" / "@"
unreserved = ALPHA / DIGIT / "-" / "." / "_" / "~"
reserved = gen-delims / sub-delims
gen-delims = ":" / "/" / "?" / "#" / "[" / "]" / "@"
sub-delims = "!" / "$" / "&" / "'" / "(" / ")"
/ "*" / "+" / "," / ";" / "="

So [ and ] must be percent encoded.

Proposal: perhaps the dot could work instead, the example becomes than:
GET /articles?include=author&fields.articles=title,body&fields.people=name

@tkellen
Copy link
Member

tkellen commented May 28, 2015

ref #544

@ethanresnick
Copy link
Member

@tkellen If our spec were the only thing in the world, I'd be in favor of using ( and ), or similar, instead of [ and ] to avoid this percent encoding trouble, but the [] syntax has become so common for expressing nested query parameters that I can't imagine we'd move away from it. So we just have to mention in our examples somewhere that the characters should be percent encoded (maybe even if we don't render them that way for usability?).

@tkellen
Copy link
Member

tkellen commented May 28, 2015

Precisely my thinking as well @ethanresnick. Would you mind working up a PR?

@ethanresnick
Copy link
Member

@tkellen Sure!

ethanresnick added a commit to ethanresnick/json-api-1 that referenced this issue Jun 7, 2015
This is simply making explicit a requirement from the URI spec that was
already implicitly in effect, so it’s not a BC break.

Addresses json-api#647
ethanresnick added a commit to ethanresnick/json-api-1 that referenced this issue Jun 19, 2015
@tkellen
Copy link
Member

tkellen commented Jun 29, 2015

Resolved!

@tkellen tkellen closed this as completed Jun 29, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants