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

Erroneously returns 405 when the last segment of the request URL matches a parameter in some other handled URL #87

Open
rengawm opened this issue Jun 20, 2015 · 2 comments

Comments

@rengawm
Copy link

rengawm commented Jun 20, 2015

I'm handling a couple of URLs:

mux.Handle("GET", "/api/{organization}/groups", ...)
mux.Handle("GET", "/api/{organization}/groups/{group}/hosts", ...)

Given this set of handlers:
GET @ /api/12345/foo - Returns 404, as I'd expect
...but...
GET @ /api/12345/groups/ - Returns 405.
GET @ /api/12345/groups/foo - Also returns 405.

Both 405 responses come back claiming:

{
    "description": "only OPTIONS are allowed",
    "error": "tigertonic.MethodNotAllowed"
}

There seems to be some weird behavior if it's trying to match URLs and the last segment of the requested URL matches a parameter portion of any other URL, even if there's no way the URL could match, as in /api/12345/groups/foo. It also doesn't seem to be checking for empty parameter values in cases like this, causing /api/12345/groups to work as expected but /api/12345/groups/ to fail with 405.

Also, if I add another handler:

mux.Handle("GET", "/api/{organization}/groups/{group}", ...)

...then requests to /api/12345/groups/ do get routed to this new handler, but it gets an empty "group" value.

@willfaught
Copy link
Contributor

Check out TrieServeMux.add to see how trailing slashes are treated.

Long story short, the slashes have to match. In your example, your handler paths don't end in slashes, but your requests did. Not sure why GET /api/12345/groups/foo returned 405, I would expect 404...maybe double check that wasn't the case?

Regarding the GET /api/12345/groups/ request, that seems right to me, I'm not sure what you're expecting. "12345" matches with {organization} and "" matches with {group}.

@rengawm
Copy link
Author

rengawm commented Jun 24, 2015

/api/12345/groups/foo - It definitely returns 405, as least as of a few commits ago. There's a specific condition where if it is processing part of a path which is the parameter for another, longer path (as in, /api/{group}/groups/{group}/hosts) it will always think OPTIONS is a valid request.

Look at trie_serve_mux.go:109 - it will never return NotFoundHandler in this case. It'll call find where paths = ["foo"], mux.param will not be nil, so it'll recurse into the next part of the path and call itself with paths = [], then return MethodNotAllowedHandler. However, MethodNotAllowedHandler won't find any valid HTTP methods for the given path, so it'll always claim that OPTIONS is the only valid method.

I get what you're saying about "" matching {groups} from a purely technical perspective, although I'd think that from a user's perspective, it's not desirable. It would seem to me that a trailing slash on a URL should generally be ignored for the sake of usability, but that's probably pretty subjective.

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