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

Behaviour regarding percent-encoding of commas in URLSearchParams #423

Closed
nickshanks opened this issue Jan 8, 2019 · 3 comments
Closed

Comments

@nickshanks
Copy link

I wish to propose a change in behaviour in the following situation, created by ticket #27:

const params = new URLSearchParams({ q: ['a,b', 'c'] });
console.log(params.toString());

Chrome, Firefox, and Safari (all as installed on my machine) currently output q=a%2Cb%2Cc.
I think this should be considered non-ideal, and propose two alternatives:

  1. the output instead be q=a%2Cb,c i.e. only commas within a multi-value parameter get percent-encoded, and literal commas are used to delineate array members. Servers can now distinguish between requests generated by {q: ['a,b', 'c']} from those generated by {q: ['a,b,c']} or {q: ['a', 'b', 'c']} or {q: 'a,b,c'} (though the latter two would still be indistinguishable). This distinction might be meaningful to the server. Or, better still;

  2. the output instead be q=a,b&q=c. Passing an ordered map to the URLSearchParams prohibits the possibility of using the same key twice. If it's value is an array and the presence of that array causes the key to be repeated in the URL, then the facility of a generating a URL where a key appears multiple times can be restored to this constructor. Some webserver frameworks only keep the last value of plain keys, and would require the key be given as {'q[]': [values...]} for an array to be reconstructed on the server-side, which is fine. That would only be possible with this second option.

Both of these seem better than the status quo, with my preference being for the latter.

@annevk
Copy link
Member

annevk commented Jan 8, 2019

I don't think this can be changed, but if anything this would have to be solved by solving #18 somehow.

@TimothyGu
Copy link
Member

FWIW there is also #204.

@annevk
Copy link
Member

annevk commented Apr 26, 2020

Closing this in favor of the existing issues.

@annevk annevk closed this as completed Apr 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants