-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
'/' not escaped in url_map.build #21
Comments
The problem here is that this is still undefined behavior. A conforming WSGI implementation cannot give you the information if %2f or / was used in the URL so the reversal of that URL would not match unless path: is used in which case / is fine again. |
Then why not leave that trouble for the WSGI middleware. The matcher is able to make the difference between %2f and / as you can see there: http://paste.pocoo.org/show/319607/ Why shouldn't we do what locally (at the Map level) makes sense, not caring about which environment we might actually be running in ... I'm still in favor of my patch (or similar). |
Because Werkzeug is a WSGI library. The path passed to the match method is already unescaped, so at that point there is no %2f any more. |
I made some tests and read a bit around, it and turns out that among all the servers I tested, Apache mod_wsgi is the only one behaving "according to the rules": it actually forward a '/' to the middleware if '%2f' appears in the URI. Encouraging me to remove all opportunities to get that '%2f' in the URI as part of a string variable,what I did. Nevertheless, werkzeug own server does not unquote the URI before forwarding it to the middleware, and I still think that to keep the behavior consistent inside werkzeug, and get an idempotent operation, either my patch should be applied, or the server should unquote the '%2f'. |
In the following scenario:
http://paste.pocoo.org/show/319042/
I would expect the last line to be '/a%2Fb'. The actual shown output should only be correct in the case of the following rule: "/path:test".
The following patch make this happen:
http://paste.pocoo.org/show/319051/
The text was updated successfully, but these errors were encountered: