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

URL escaped paths don't match #32

Closed
raphael opened this issue Jun 24, 2016 · 11 comments
Closed

URL escaped paths don't match #32

raphael opened this issue Jun 24, 2016 · 11 comments

Comments

@raphael
Copy link
Contributor

raphael commented Jun 24, 2016

Consider the following route:

router.GET("/foo/\\*starToken", handler) // matches /foo/*starToken

And the following client code:

u := URL{Host: "localhost:8080", Scheme: "http", Path: "/foo/*starToken"}
req, _ := http.NewRequest("GET", u.String(), nil)
resp, _ := http.DefaultClient.Do(req)
if resp.StatusCode == 404 {
    fmt.Println("oh noes")
}

Then "oh noes" gets printed. That's because u.String() returns:

"http://localhost:8080/foo/%2AstarToken"

It seems httptreemux is not handling the escape properly. Now it could also be argued that the stdlib is being a bit too generous in its escaping, http://www.rfc-editor.org/rfc/rfc1738.txt states that "*" is OK in URLs but it is what it is...

@raphael raphael changed the title URL escaped path don't match URL escaped paths don't match Jun 24, 2016
@dimfeld
Copy link
Owner

dimfeld commented Jun 24, 2016

Hmm, interesting. I may fix this by adding both escaped and unescaped versions of the URL to the tree, when applicable. I won't be able to do any work on this until next week some time, but will think about it in the meantime. Thanks for the report!

@raphael
Copy link
Contributor Author

raphael commented Jun 24, 2016

Yes I was thinking the same (adding both routes to the tree). FWIW the issue doesn't seem to be there for : https://play.golang.org/p/gs1DkJ5Bt0

@dimfeld
Copy link
Owner

dimfeld commented Jul 8, 2016

Sorry for the delay. Been really busy with work and also moving interstate. But I haven't forgotten about this. I'll be able to get it done some time this month.

@raphael
Copy link
Contributor Author

raphael commented Jul 8, 2016

np, thanks for the heads up!

@dimfeld
Copy link
Owner

dimfeld commented Jul 29, 2016

I have this implemented, still need to write a couple tests. But I remembered that you can tell httptreemux to use r.URL.Path instead of r.RequestURI by setting router.PathSource = httptreemux.URLPath. That might actually be the better solution since we then don't have to worry about inconsistencies with which characters Go chooses to escape vs. the characters that any particular client escapes. As an example, Go does not encode the : character, as you pointed out, but the Javascript encodeURIComponent function does. What do you think of this?

@raphael
Copy link
Contributor Author

raphael commented Jul 30, 2016

Thanks! yes that might work. Need to understand better how RequestURI and URL.Path defer...

@dimfeld
Copy link
Owner

dimfeld commented Jul 30, 2016

My understanding is that URLs generated by url.Parse have the Path
field unescaped, so the %2A will be converted back to a * and the router
will match on the unescaped path. But let me know if you try this and it
still isn't working for you.

On Sat, Jul 30, 2016 at 7:05 AM, Raphaël Simon notifications@github.com
wrote:

Thanks! yes that might work. Need to understand better how RequestURI and
URL.Path defer...


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#32 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABTl1kWwCYxDhAplFxWJ6P4dQfE7zeDvks5qa4RagaJpZM4I-EJQ
.

@raphael
Copy link
Contributor Author

raphael commented Aug 2, 2016

So I tried that and it won't work for my use case :( I have slashes in my URLs that need to stay escaped for the route to match.

@dimfeld
Copy link
Owner

dimfeld commented Aug 2, 2016

Yeah, that is the caveat of using URL.Path. Ok, I'll see about getting the "escape the route" change in this week. :)

@raphael
Copy link
Contributor Author

raphael commented Aug 2, 2016

Awesome, thank you!

@dimfeld dimfeld closed this as completed in 2b442cd Aug 4, 2016
raphael added a commit to goadesign/goa that referenced this issue Aug 4, 2016
This fixes an issue where requests sent by clients would not
match routes that contain characters that are escaped by the
net/url algorithm used to build URL strings. In particular
this affected routes with '*' in them.

See dimfeld/httptreemux#32
raphael added a commit to goadesign/goa that referenced this issue Aug 4, 2016
This fixes an issue where requests sent by clients would not
match routes that contain characters that are escaped by the
net/url algorithm used to build URL strings. In particular
this affected routes with '*' in them.

See dimfeld/httptreemux#32
@raphael
Copy link
Contributor Author

raphael commented Aug 4, 2016

Thank you!

raphael pushed a commit to goadesign/goa that referenced this issue Aug 4, 2016
This fixes an issue where requests sent by clients would not
match routes that contain characters that are escaped by the
net/url algorithm used to build URL strings. In particular
this affected routes with '*' in them.

See dimfeld/httptreemux#32
raphael pushed a commit to goadesign/goa that referenced this issue Aug 4, 2016
This fixes an issue where requests sent by clients would not
match routes that contain characters that are escaped by the
net/url algorithm used to build URL strings. In particular
this affected routes with '*' in them.

See dimfeld/httptreemux#32
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