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

Impossible to quote slashes in path parameters #182

Closed
jannic opened this issue Apr 16, 2018 · 5 comments
Closed

Impossible to quote slashes in path parameters #182

jannic opened this issue Apr 16, 2018 · 5 comments

Comments

@jannic
Copy link
Contributor

jannic commented Apr 16, 2018

I failed to use a pattern like route("/some/path/{url}" where url should match a percent-encoded URL like in http://127.0.0.1:8080/some/path/http%3A%2F%2Fexample.com
This fails because the %-encoding is decoded before the pattern is matched, so %2f gets expanded to / and the pattern doesn't match any more.
IMHO the fix to #137 in commit 90e3aaa is wrong or incomplete.
I understand it's desirable to be able to directly match on decoded utf-8 characters, but if the decoding is done unconditionally, it makes other use cases like the one mentioned above impossible.

@fafhrd91
Copy link
Member

i think current behavior is right. it cause less problems. you can use handler App::handler("/some/path")
tail of the path is available as tail in req.match_info()

@fafhrd91
Copy link
Member

i'll think about this problem. may need to introduce custom decoder for encoded uri

@jannic
Copy link
Contributor Author

jannic commented Apr 17, 2018

Well yes, as I said, I understand why the current behavior is useful. But at the same time, from a logical point of view, it's kind of wrong: A URI/URL consists of components separated by (literal) slashes. Therefore, you can't easily use slashes within those components. And that (and other special characters) is the very reason why percent encoding was invented.
RFC-2396 explicitly states: "Likewise, a URI must be separated into its components before the escaped characters within those components can be safely decoded." So one could even say that the current behavior is wrong because it violates the standard.
But surely, just reverting the fix for #137 would not be a good solution. If I thought it was (or had some other simple solution), I would have sent a PR. This needs some good idea how we can have both behaviors available to users, without becoming too confusing because of too many choices.

@jannic
Copy link
Contributor Author

jannic commented Apr 17, 2018

Further thinking about it, the current behavior could even cause bugs: Whenever the contents of the percent-encoded string are provided by the user of the web site (which is often the case), things will break whenever the user enters a '/' in the input box. So, perhaps, the current behavior causes more problems than the old one.

fafhrd91 added a commit that referenced this issue Apr 17, 2018
@fafhrd91
Copy link
Member

fixed by a826d11

will release new version later today

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

2 participants