Skip to content

[Routing] Url Generator and slashes in url parameters #13017

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

Closed
imunhatep opened this issue Dec 17, 2014 · 11 comments · Fixed by #39732
Closed

[Routing] Url Generator and slashes in url parameters #13017

imunhatep opened this issue Dec 17, 2014 · 11 comments · Fixed by #39732

Comments

@imunhatep
Copy link

Currently symfony does not allow to generate urls with encoded slash in slug. Ex:

_some_route:
    path: /create/{folder}/zip

If folder parameter value is: /home/user, either I have to allow / in url parameter or do parameter encoding before passing it to the UrlGenerator.
In the first case we get raw path: /create//home/user/zip
In the second case, UrlGeneretor will double encode the parameter: /create/%252Fhome%252Fuser/zip.
But neither give desired output: /create/%2Fhome%2Fuser/zip

I saw remark in the UrlGenerator code to problems with JBOSS and Apache for url with encoded slashes (http://stackoverflow.com/questions/4069002/http-400-if-2f-part-of-get-url-in-jboss), also it looks more like a bug to me, nevertheless I think it should be handled by programmers that work with that systems, cause encoded slash is fully valid in url.

Also current approach forces to use double decoding on servers with proper handling of urls, or overriding url generator class in symfony.

I do understand that my proposal will break back-compatibility, but I think this behavior should be discussed at least for future versions.

@imunhatep imunhatep changed the title Url generator and slashes in url parameters [Routing] Url Generator and slashes in url parameters Dec 17, 2014
@inmarelibero
Copy link
Contributor

maybe you already considered it, but a quick solution is to allow slashes in parameters, just by doing:

_some_route:
    path: /create/zip/{folder}
    defaults: { _controller: YourController }
    requirements:
        folder: .+

but of course you should put folder as last parameter.

You could also consider this:

_some_route:
    path: /create/{folder}
    defaults: { _controller: YourController }
    requirements:
        folder: .+

then generate your url with

path('_some_route', {'folder': '/home/user', 'format': 'zip'})

which will give you:

/create//home/user?format=zip

@Tobion
Copy link
Contributor

Tobion commented Dec 19, 2014

Why do you need {folder} in the path? Just make it a query param. Then you don't have this problem.
IMO there are 2 cases:

  1. You want a user friendly URL and have the param in the path. In this case you also do not want to encode it.
  2. It's not intended for users (e.g. API) which your path (/create/{folder}/zip) looks like. Then you can just use a query param or body param for that. Btw, having the verb (create) in the path is not best practice if that is for a REST API. You might want to use POST instead.

@imunhatep
Copy link
Author

It's a REST API, and path in the main comment is just an example. Of course I use POST/GET e.t.c. for creation/update, not create path in the url.

There're many possible solution, but neither solve the main problem of "incorrect" url param encoding.
Currently I extended UrlGenerator class, to produce urls in format I need.

I don't consider possibility to move {folder} to get params or body, cause REST path structure assume that accessed resource name should be in the path not in the params, ex.:
GET /user/{username}/folder/{folder}/file/{filename}?search=asd

To @inmarelibero: you descriveb my first case, but this solution will produce incorrect urls for rest api.

@fabpot fabpot added the Routing label Jan 2, 2015
@nacmartin
Copy link
Contributor

Even when %2F is, strictly speaking, allowed in URLs, if it is disallowed in Apache and JBoss for security reasons, it makes it in practice disallowed, IMO. Apache is not just another marginal web server and Symfony cannot just say "let their users deal with the problem".

Anyways, could this be solved by adding a method setDecodedChars so you could override the default pairs of decodedChars?

@imunhatep
Copy link
Author

That is not the fix I was hoping for.. but yes, that will help, thought not for current implementation of UrlGenerator class, it will incorrectly encode url because of code at line 192.

IMHO: There're way too many other web servers (nginx, tomcat, netty, jetty, e.t.c.), if someone introduced something stupid, it doesn't mean that others should support it. It's a bit strange to have advantage for 2 servers, and break compatibility with others. A specially with URI RFC.

P.S. That apache and jboss behavior is configurable.

@teohhanhui
Copy link
Contributor

Double URL encoding could be a security risk: https://www.owasp.org/index.php/Double_Encoding

@teohhanhui
Copy link
Contributor

Perhaps instead of

$url = strtr(rawurlencode($url), $this->decodedChars);

we can do the rawurlencode before we prepend to $url, and for each of them we can detect whether they're already URL-encoded (is that safe?) or best if the user can set an already_encoded flag for each parameter...

The strtr seems safe to be done only once at the current line.

@HeahDude
Copy link
Contributor

HeahDude commented Jul 8, 2016

Solutions proposed by @Tobion should be the way to go.

Let's close this old issue as "won't fix", since it's not a good practice to resolve parameters containing slashes.

Thanks.

@fabpot fabpot closed this as completed Jul 8, 2016
@MarioHoberg
Copy link

I strongly disagree. A framework such as Synfomy should be able to handle and generate all kind of standard compliant URIs. If, for any reason, certain properly encoded characters should be forbidden in the path, this should be done by some configurable validation component. Given that this is not possible with the current UrlGenerator is a strong indication to rewrite the UrlGenerator.

@imunhatep
Copy link
Author

imunhatep commented Jul 8, 2016

2016.. The most popular php web framework, is not compatible with uri rfc (rfc3986).

MarioHoberg pushed a commit to MarioHoberg/symfony that referenced this issue Dec 5, 2020
This is a proof of concept for symfony#13017 and addresses some pressing issues when comparing the current implementation with RFC 3986.

If this would be adapted in Symfony, the changed behaviour probably should become optional as some changes will break backwards compatibility.
For now this should be fine to point out the changed behaviour.
fabpot added a commit that referenced this issue Feb 5, 2021
…ed slashes when generating URLs (nicolas-grekas)

This PR was merged into the 5.3-dev branch.

Discussion
----------

[Routing] don't decode nor double-encode already encoded slashes when generating URLs

| Q             | A
| ------------- | ---
| Branch?       | 5.x
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | -
| Tickets       | Fix #13017
| License       | MIT
| Doc PR        | -

Replaces #39339

Commits
-------

eaac18b [Routing] don't decode nor double-encode already encoded slashes when generating URLs
@lazka
Copy link

lazka commented Jan 3, 2022

What's the status of this? Are there any workarounds that don't require changes to the URL schema?

nicolas-grekas added a commit that referenced this issue Mar 8, 2022
…ters (usu)

This PR was merged into the 6.1 branch.

Discussion
----------

[Routing] Avoid double encoded slashes in query parameters

| Q             | A
| ------------- | ---
| Branch?       | 6.1
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | Fix #45114
| License       | MIT
| Doc PR        |

Similar feature as discussed in #13017 and implemented in #39732, however for query parameters and not for path parameters.

This allows to manually encode slashes in query parameters before handing them over to `UrlGenerator` and avoids these slashes to be double encoded. Currently, it's not possible to generate an output with single encoded slashes in query parameters.

See #45114 for further details.

Commits
-------

1421298 [Routing] query parameters: don't decode nor double-encode already encoded slashes when generating URLs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment