-
-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
Requests with URL-encoded characters are not forwarded correctly #684
Comments
Hi @ydubreuil, this is duplicate of #167, and due to gorilla/mux#94. A PR has been made to fix this #521 but I have no news from @sybrandy. If you want, you can carry his commit in a new PR, I will merge it right away :) |
That's great to see there's already a PR for this :) I missed the original issue, sorry! I will try to follow up on #521 tomorrow. |
@ydubreuil Let me know if you have any questions regarding the PR. I feel bad having someone else working on it, but I haven't had the time to get to any of this. Short version: did the work on company time to fulfill some requirements, created PRs to share the changes, company didn't allocate time for me to work on the PRs. |
No worries, I'll find some time to move your work forward @sybrandy . I'm happy to have a fix for the issue. |
I wonder if there should be an option to enable / disable calling @sybrandy WDYT? |
@ydubreuil I kind of agree with you. WDYT @containous/traefik ? |
I'm good with it. I made it optional primarily because, and I hope I remember this correctly, I read somewhere where this was not wanted and that all URLs should be cleaned. I also didn't want to change the default behaviour since that is what's expected and I wasn't sure of the consequences. However, I believe you are right: a proxy shouldn't mess with the URLs. Path prefixes being the exception to the rule, but otherwise they should just pass them along in the same manner they were created. I guess there's always a chance that a server can't handle a non-cleaned URL, but that seems more like a bug that should be fixed vs. masked by a proxy. I can't think of an exception to this right now. |
Right, seems like we all agree, I'll do a PR then |
With #688 , when I launch cURL against Jenkins, I get the following network capture:
which means the fix works as expected and Jenkins is happy :) |
Fixed by #688 |
It seems that Traefik doesn't forward requests with URL encoded parts correctly. Instead of forwarding requests with encoded characters straight to back-end servers, it issues a 301 Moved reponse with a Location header containing the decoded URL. In the decoded URL, double-slashes strings are replaced a single slash. This breaks the Jenkins Reverse Proxy monitor which reports a warning telling the reverse proxy set up is broken.
For example, when the following request is sent to Traefik
http://localhost:8000/administrativeMonitor/hudson.diagnosis.ReverseProxySetupMonitor/testForReverseProxySetup/http%3A%2F%2Flocalhost%3A8000%2Fmanage/
, Traefik responds with a 301 containing a wrong Location response header, leading to a 404 on the next request.Here is the network traffic between cURL and Traefik:
As you can see, the first HTTP response (301) contains the incorrectly decoded URL. The response is issued by Traefik and doesn't come from Jenkins (no
Server
header and the response is not in the network capture of traffic between Traefik and Jenkins)In case someone wants to reproduce with Jenkins, I baked a
docker-compose.yml
to launch both Traefik and Jenkins:To get the admin API token, browse http://localhost:8000/user/admin/configure
Then, launch the following curl command to reproduce:
Any pointer to help me digging into Traefik code to propose a fix would be greatly appreciated.
The text was updated successfully, but these errors were encountered: