Fix open redirect with backslash URL #94
Conversation
|
WTF. Why do browsers convert backslashes to forward slashes instead of rejecting such nonsense?! |
|
Looks like |
Indeed well checked, simple backslash surprised me also. |
|
I pushed a commit that should fix all those cases. Please have another look before I merge it :) |
| # Browsers treat backslashes like forward slashes, while urllib doesn't. | ||
| # Since we just want to validate scheme and netloc here, we normalize | ||
| # slashes to those recognized by urllib. | ||
| url_info = urlsplit(url.replace('\\', '/')) |
There was a problem hiding this comment.
I'm not sure if there are cases where the backslash has a function like in older/unusual browsers (e.g. some Japanese institutions still use Internet Explorer :( ).
So I didn't use replace() but yes it seems logical.
There was a problem hiding this comment.
At least in case of Indico there aren't any cases where we'd ever expect backslashes in the next URL, and I'd expect anything else using this library to not do so either.
PS: Indico no longer supports IE for many years now, so if things there break even more, so be it. :)
There was a problem hiding this comment.
ahah wise decision 👍
The goal of this PR is to disallow open redirect for URLs formed with backslashes.
Example: