-
Notifications
You must be signed in to change notification settings - Fork 133
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
Fix handling of encoded slash in url path #302
Fix handling of encoded slash in url path #302
Conversation
@jfraudeau Thank you! Could you please add a test for this as well? |
@ocramz I added a test, let me know if anything else is missing or if there ara conventions I didn't follow |
Thank you @jfraudeau , it looks good but I'd like to merge it after #303 and make it use |
Yes no problem, do as you see fit |
Hi @ocramz I updated the pr to with upstream changes. Let me know if anything is still blocking |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry @jfraudeau for sleeping on this. Thanks! 👍
Web/Scotty/Route.hs
Outdated
@@ -115,7 +115,7 @@ matchRoute :: RoutePattern -> Request -> Maybe [Param] | |||
matchRoute (Literal pat) req | pat == path req = Just [] | |||
| otherwise = Nothing | |||
matchRoute (Function fun) req = fun req | |||
matchRoute (Capture pat) req = go (T.split (=='/') pat) (compress $ T.split (=='/') $ path req) [] | |||
matchRoute (Capture pat) req = go (T.split (=='/') pat) (compress $ T.fromStrict <$> "":pathInfo req) [] -- add empty segment to simulate being at the root |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On second thought, I'm not so sure I like a corner case to be encoded in this function.
Is this issue a special case of capture parameters not being url-decoded first? see #262
Currently url encoded forward slash is decoded before the routing logic is applied. This is a problem when we want to allow arbitrary user data to be passed as a path segment.
For example
/test/some%2Fdata/path
will be routed as/test/some/data/path
it thus becomes impossible to have a forward slash in a captured path segment.This fix bypasses the broken rountrip of
intercalate "/"
<->split (=='/')