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

Requests with URL-encoded characters are not forwarded correctly #684

Closed
ydubreuil opened this issue Sep 19, 2016 · 10 comments
Closed

Requests with URL-encoded characters are not forwarded correctly #684

ydubreuil opened this issue Sep 19, 2016 · 10 comments

Comments

@ydubreuil
Copy link
Contributor

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:

GET /administrativeMonitor/hudson.diagnosis.ReverseProxySetupMonitor/testForReverseProxySetup/http%3A%2F%2Flocalhost%3A8000%2Fmanage/ HTTP/1.1
Host: localhost:8000
Authorization: Basic YWRtaW46MzJiZDM0MTJlMmFlYmVkYzgwNjQzMDUxM2VkYzA2ZGY=
User-Agent: curl/7.50.2
Accept: */*
Referer: http://localhost:8000/manage

HTTP/1.1 301 Moved Permanently
Location: /administrativeMonitor/hudson.diagnosis.ReverseProxySetupMonitor/testForReverseProxySetup/http:/localhost:8000/manage/
Date: Mon, 19 Sep 2016 14:56:47 GMT
Content-Length: 0
Content-Type: text/plain; charset=utf-8

GET /administrativeMonitor/hudson.diagnosis.ReverseProxySetupMonitor/testForReverseProxySetup/http:/localhost:8000/manage/ HTTP/1.1
Host: localhost:8000
Authorization: Basic YWRtaW46MzJiZDM0MTJlMmFlYmVkYzgwNjQzMDUxM2VkYzA2ZGY=
User-Agent: curl/7.50.2
Accept: */*
Referer: http://localhost:8000/manage

HTTP/1.1 404 Not Found
Cache-Control: must-revalidate,no-cache,no-store
Content-Length: 459
Content-Type: text/html; charset=ISO-8859-1
Date: Mon, 19 Sep 2016 14:56:47 GMT
Server: Jetty(9.2.z-SNAPSHOT)
X-Content-Type-Options: nosniff
....

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:

traefik:
  image: traefik:v1.0.2
  command: --web --docker
  ports:
    - "8000:80"
    - "8080:8080"
  volumes:
    - /var/run/docker.sock:/var/run/docker.sock

jenkins:
  image: jenkins:2.7.4-alpine
  labels:
    - "traefik.backend=jenkins"
    - "traefik.frontend.rule=Host:localhost"
    - "traefik.port=8080"
  environment:
    - JAVA_OPTS=-Xmx256m -Dhudson.DNSMultiCast.disabled=true
  expose:
    - "8080"

To get the admin API token, browse http://localhost:8000/user/admin/configure

Then, launch the following curl command to reproduce:

curl -iL --user "admin:$API_TOKEN" \
  -e "http://localhost:8000/manage" \
  "http://localhost:8000/administrativeMonitor/hudson.diagnosis.ReverseProxySetupMonitor/test"

Any pointer to help me digging into Traefik code to propose a fix would be greatly appreciated.

@emilevauge
Copy link
Member

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 :)

@ydubreuil
Copy link
Contributor Author

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.

@sybrandy
Copy link

@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.

@ydubreuil
Copy link
Contributor Author

No worries, I'll find some time to move your work forward @sybrandy . I'm happy to have a fix for the issue.

@ydubreuil
Copy link
Contributor Author

I wonder if there should be an option to enable / disable calling router.SkipClean at all. IMO, we should always skip the URL cleaning code of gorilla/mux, after all, Traefik should not interfere with forward requests. It's up to backend servers to clean up URLs if required.

@sybrandy WDYT?

@emilevauge
Copy link
Member

@ydubreuil I kind of agree with you. WDYT @containous/traefik ?

@sybrandy
Copy link

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.

@ydubreuil
Copy link
Contributor Author

Right, seems like we all agree, I'll do a PR then

@ydubreuil
Copy link
Contributor Author

With #688 , when I launch cURL against Jenkins, I get the following network capture:

GET /administrativeMonitor/hudson.diagnosis.ReverseProxySetupMonitor/test HTTP/1.1
Host: localhost:8000
Authorization: Basic YWRtaW46MzJiZDM0MTJlMmFlYmVkYzgwNjQzMDUxM2VkYzA2ZGY=
User-Agent: curl/7.50.2
Accept: */*
Referer: http://localhost:8000/manage

HTTP/1.1 302 Found
Content-Length: 0
Date: Tue, 20 Sep 2016 15:17:40 GMT
Location: http://localhost:8000/administrativeMonitor/hudson.diagnosis.ReverseProxySetupMonitor/testForReverseProxySetup/http%3A%2F%2Flocalhost%3A8000%2Fmanage/
Server: Jetty(9.2.z-SNAPSHOT)
X-Content-Type-Options: nosniff
Content-Type: text/plain; charset=utf-8

GET /administrativeMonitor/hudson.diagnosis.ReverseProxySetupMonitor/testForReverseProxySetup/http%3A%2F%2Flocalhost%3A8000%2Fmanage/ HTTP/1.1
Host: localhost:8000
Authorization: Basic YWRtaW46MzJiZDM0MTJlMmFlYmVkYzgwNjQzMDUxM2VkYzA2ZGY=
User-Agent: curl/7.50.2
Accept: */*
Referer: http://localhost:8000/manage

HTTP/1.1 200 OK
Content-Length: 0
Date: Tue, 20 Sep 2016 15:17:40 GMT
Server: Jetty(9.2.z-SNAPSHOT)
X-Content-Type-Options: nosniff
Content-Type: text/plain; charset=utf-8

which means the fix works as expected and Jenkins is happy :)

@emilevauge
Copy link
Member

Fixed by #688

@traefik traefik locked and limited conversation to collaborators Sep 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants