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

cleanPath() causing a change in the URL should not necessarily cause a redirect #94

Closed
philcluff opened this issue Apr 22, 2015 · 8 comments
Labels

Comments

@philcluff
Copy link

Say I have an API with a URL structure as follows: http://example.com/library/$BOOKID/chapters

If someone hits the URL http://example.com/library//chapters (Fails to provide a bookID), the client will receive a 301 to a location which doesn't exist (301 -> follow -> 404)

In my opinion, this behaviour should be optional, the problem is of course that path.Clean() is called, and (As per documentation) // 1. Replace multiple slashes with a single slash. is performed.

Most my reasoning for not wanting this behaviour is based on me not wanting my routing library to redirect my clients when I'm not explicitly asking it to.

What are others thoughts on this?

Cheers,

@bundah
Copy link

bundah commented Aug 6, 2015

I agree with your reasoning. Granted, the client is hitting the endpoint incorrectly. However path.Clean doesn't just trim trailing slashes (as code comments say https://github.com/gorilla/mux/blob/master/mux.go#L348), it trims all extra slashes and in some cases dots too... because it is meant more for filesystem pathing IMO. Perhaps a better approach ( don't know the exact rationale for the current implementation) is logic surrounding StrictSlash and https://golang.org/pkg/strings/#TrimRight with "/" as the cutset...

@kisielk
Copy link
Contributor

kisielk commented Aug 13, 2015

The cleanPath behaviour is identical to that of the net/http mux.

@kisielk
Copy link
Contributor

kisielk commented Aug 13, 2015

and actually the redirect behaviour is basically the same as well... so I'm not really inclined to change the way it is working right now

@dylanmei
Copy link

Since I've started using Traefik -- which enlists gorilla/mux in its routing layer -- I've discovered two backend apps that no longer function as designed.

They both use urls with their proxy data in the path, as opposed to a query-string. For example:

http://my-app/proxy/http://another-app

While I don't personally agree with their design decisions, these apps and probably many more are unusable due to the path cleaning / redirect.

@elithrar
Copy link
Contributor

Related: #142

@elithrar elithrar added the bug label Jan 16, 2016
@elithrar
Copy link
Contributor

One (quick and dirty) option would be to provide a .NoClean() method on Route to skip the path cleaning logic and leave that up to the package user.

The risk there is that it's a sharp edge that—if you don't carefully write your own path-cleaning logic—you could break your application in other ways.

@elithrar
Copy link
Contributor

elithrar commented May 2, 2016

This should be fixed in #154.

@elithrar elithrar closed this as completed May 2, 2016
@philcluff
Copy link
Author

Great, thanks!

sybrandy added a commit to sybrandy/traefik that referenced this issue Jul 12, 2016
ydubreuil added a commit to ydubreuil/traefik that referenced this issue Sep 20, 2016
This fixes traefik#167 and traefik#651. By default, gorilla/mux cleans URL paths
such that adjacent slashes are collapsed into one single slash. This
behavior is pretty common in application stacks and it is fine, but
for Traefik, this can lead to incorrect URL paths forwarded to backend
servers.

See gorilla/mux#94 for background.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants