Skip to content

The "," character should not be escaped in query strings using the URL generator #27266

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.

Sign up for GitHub

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

Jump to bottom
Closed
egonolieux opened this issue May 14, 2018 · 8 comments
Closed

The "," character should not be escaped in query strings using the URL generator #27266

egonolieux opened this issue May 14, 2018 · 8 comments
Labels
Bug Routing Status: Needs Review

Comments

@egonolieux
Copy link

Symfony version(s) affected: 3.4.8

Description

I have a query string in which multiple values for a single parameter are delimited by a comma ",". According to RFC 3986 this comma does not need to be escaped in the query string. However, when I try to generate routes using the URL generator, the comma seems to get encoded. Is this by design or is this an oversight?

For example, the following code:

$url = $urlGenerator->generate('routeName', array(
    'category' => '1,2'
));

produces https://www.example.com/path?category=1%2C2 instead of https://example.com/path?category=1,2.
@ismail1432
Copy link
Contributor

@egonolieux I'm not agree. The ',' seems to be a reserved character read more here

@sstok
Copy link
Contributor

sstok commented May 16, 2018

A common approach is to use ~ instead of a comma for separating values.

@egonolieux
Copy link
Author

It might be a reseverd character indeed, but that does not mean it cannot be used in a certain context.
The accepted answer from the stackoverflow question you referred to states this:

Thus commas are explicitly allowed within query strings and only need to be escaped in data if specific schemes define it as a delimiter. The HTTP scheme doesn't use the comma or semi-colon as a delimiter in query strings, so they don't need to be escaped

and

Since commas don't have a reserved purpose under the HTTP scheme, they don't have to be escaped in data. The note from § 2.3 about reserved characters being those that change semantics when percent-encoded applies only generally; characters may be percent-encoded without changing semantics for specific schemes and yet still be reserved.

@ostrolucky
Copy link
Contributor

ostrolucky commented May 16, 2018 •
edited
Loading

#12223 is a precedent things like this are allowed to be done in symfony

In any case, this is not a bug

@egonolieux
Copy link
Author

egonolieux commented May 19, 2018 •
edited
Loading

@ostrolucky I wouldn't suggest hardcoding things like comma's though. The issue you referred to regarding / is generally applicable to all URIs: https://tools.ietf.org/html/rfc3986#section-3.4.

Because there are many more characters than , which might be allowed in the query string in certain circumstances, I feel like it should be possible to override these characters by configuration.

The main issue is that if an URL with such characters in the query string is entered, the next page (using pagination for example), encodes these characters by default causing the URL to change.

@Simperfit
Copy link
Contributor

Is there something to do here ?

@Tobion
Copy link
Contributor

Tobion commented Apr 16, 2019

I'm fine with adding the , to the decoded chars similar to https://github.com/symfony/symfony/pull/12223/files.

@Tobion Tobion mentioned this issue Apr 16, 2019
Merged
@Tobion
Copy link
Contributor

Tobion commented Apr 16, 2019

I opened a PR for this.

@fabpot fabpot closed this as completed Apr 27, 2019
fabpot added a commit that referenced this issue Apr 27, 2019
@fabpot
feature #31134 [Routing] do not encode comma in query and fragment (T…
1725a3c 
…obion)

This PR was merged into the 4.3-dev branch.

Discussion
----------

[Routing] do not encode comma in query and fragment

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no <!-- don't forget to update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | #27266
| License       | MIT
| Doc PR        |

Commits
-------

76f6c97 [Routing] allow comma and other reserved chars without special meaing to not be encoded in the query and fragment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Routing Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
9 participants
@fabpot @ostrolucky @Tobion @sstok @xabbuh @Simperfit @egonolieux @ismail1432 @carsonbot