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

fix Location header protocol #88

Closed
wants to merge 2 commits into from
Closed

fix Location header protocol #88

wants to merge 2 commits into from

Conversation

noahdietz
Copy link
Contributor

  • fixes the Location header by adding the protocol used by the request
  • passes along x-forwarded-* headers in response if they are present

Fixes #86

@AdamMagaluk
Copy link

I think you miss understood what the use of x-forwarded-host and x-forwarded-proto are used for.

These are always request headers and not generally sent on the response, though I don't think that is necessarily hurting anything. They are generally set by a proxy sitting between the client and our service to tell our service how the original client request was made.

Kiln needs to generate a Host and Protocol for the absolute url. To get the host it should first look at x-forwarded-host if set use that and if not we default to the Host header. This is because a proxy is required to set a valid Host header when making the intermediate request. So a client may request Host: shipyard.apigee.net but our Edge or nginx proxy may actually be requesting a static IP or our service or some other hostname and is required to set that as the Host header.

The same logic applies to x-forwarded-proto but we'd default to http if TLS property is not set on kiln. Which should always be the case because we are only ever spinning up a http server.

@noahdietz
Copy link
Contributor Author

Thanks for the clarification, I'll fix it.

@mpnally
Copy link
Contributor

mpnally commented Feb 6, 2017

Good description by Adam. Let me add a couple more points. x-forwarded-host and x-forwarded-proto are not standard headers. x-forwarded-host was popularized by the Apache http server — they may be the ones who invented it. The proper behavior of any proxy is to forward the original Host header unmodified. x-forwarded-host is only necessary as a workaround for badly-behaved proxies that corrupt the Host header. If all the proxies ahead of you are well-behaved proxies, then the Host header — which is a standard header — will contain the original value set by the original client. Unfortunately, most Edge proxies are badly-behaved — you have to include special instructions to make a well-behaved Edge proxy — the default is to be badly-behaved. There is no standard equivalent of x-forwarded-proto. You would think that this would be a useful header, but it doesn't work as well as you would hope. Firstly, most proxies will not set this header, so unless you control and can configure the first proxy the client hits, the value will be missing or possibly wrong. If the client first hits a proxy that terminates https but doesn't set the header, and then forwards the message via http to another proxy that does set the header, the value will be wrong. Also, suppose a client sends an https request via Edge to enrober and enrober then calls kiln passing along this header. Kiln will then return URLs with https in them. This works well if enrober subsequently returns those URLs to the original client, but may or may not work if enrober itself wants to use the URL to make a request. The strategy of basing absolute URLs on the value of x-forwarded-proto works well if you are using https everywhere or http everywhere, but if you are mixing them, for example using https externally and http internally, then it doesn't work. Maybe this doesn't matter and we will always be in a world where it is all one (production) or all the other (test).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invalid Location url returned when http proxies are in front of Kiln
3 participants