-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Encoded slashes are expanded by the proxy #17
Conversation
Thanks for the report, that's illuminating. We've actually seen that error message on our own jenkins instance, though we just ignored it (because we were pretty sure the basic host and path parts of rewriting were correct, and it's worked OK for our projects and integrations for many months). |
Any chance of a fix for this? The google_auth_proxy is actually decoding the URL and then doing a 301 redirect to the decoded version. Not only that, but a pair of slashes |
I don't know Golang, but I think I found the root cause within Golang's httputil package. http://golang.org/src/pkg/net/http/httputil/reverseproxy.go#L66
When you use I don't really know where to proceed from here. I'm going to dig around to see if anyone has reported this issue in httputil previously. |
It appears to me that you'd have to not use NewSingleHostReverseProxy(), and make your own reverse proxy, to fix this. |
I guess that's doable if this doesn't get fixed upstream - NewSingleHostReverseProxy is a relatively small method. Looks like singleJoiningSlash is the only non-public method it calls, so not too much code to copy? |
Hi Is there any fix for this issue, today I've upgraded jenkins and I see the same error although we are still not using it in production so I am not sure whether it will cause problems or not. |
@ploxiln mind taking a look at this? @adrian-gomez I saw you also had a similar patch for this in 3af83e0 so If you have any comments I'd appreciate it. |
Hi, yeah i had the issue but with gerrit. The fix as someone mentioned is to use a custom proxy instead of NewSingleHostReverseProx for now at least. The real issue is with NewSingleHostReverseProxy that is not doing a great job as a proxy. I have fixed this in the commit you mentioned i can do a PR ir you want |
@adrian-gomez Yeah your commit was helpful. I've updated this PR w/ a patch and I was hoping you could help verify that a build from this branch works as expected for you. |
Yeah np, i'll test and comment back before the end of the day. |
} | ||
expected := backend.URL + encodedPath | ||
if seen != expected { | ||
t.Errorf("got bad requst %q expected %q", seen, expected) |
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.
s/requst/request/
I'm actually not using google_auth_proxy at the present moment, so take this with a grain of salt, but this LGTM. |
ddf5ed7
to
71ae708
Compare
Encoded slashes are expanded by the proxy
To reproduce, hit this URL on the proxy:
http://proxy/a%2Fb/
It is passed to the backend as
http://backend/a/b/
The expected result is for the backend to receive
http://backend/a%2Fb/
This is a problem for Jenkins and is described here: https://wiki.jenkins-ci.org/display/JENKINS/Jenkins+says+my+reverse+proxy+setup+is+broken