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

Proxy handling does not properly set destination Host header #774

Closed
shaleh opened this issue Apr 27, 2016 · 2 comments
Closed

Proxy handling does not properly set destination Host header #774

shaleh opened this issue Apr 27, 2016 · 2 comments
Assignees

Comments

@shaleh
Copy link

shaleh commented Apr 27, 2016

Here is a script transcript of a working proxy session

Script started on Wed 27 Apr 2016 09:32:58 AM PDT
[~/repos/rust/hyper-upstream]
~$ telnet my-proxy 8080
Trying x.x.x.x...
Connected to my-proxy.
Escape character is '^]'.
GET http://xkcd.com http/1.1
host: xkcd.com:80

HTTP/1.1 200 OK
Cache-Control: public, max-age=300
Expires: Wed, 27 Apr 2016 16:23:15 GMT
Content-Type: text/html; charset=utf-8
ETag: "1703862119"
Last-Modified: Wed, 27 Apr 2016 05:16:40 GMT
Server: lighttpd/1.4.28
Content-Length: 6196
Accept-Ranges: bytes
Date: Wed, 27 Apr 2016 16:18:15 GMT
Via: 1.1 varnish
X-Served-By: cache-dfw1838-DFW
X-Cache: HIT
X-Cache-Hits: 1
X-Timer: S1461772234.306036,VS0,VE0
Vary: Accept-Encoding
Proxy-Connection: Keep-Alive
Connection: Keep-Alive
Age: 971

Note the destination is specified TWICE. Once on the GET and once again in the Host line. The current client code does not set the Host correctly.

Here is the diff needed to fix it:

diff --git a/src/client/mod.rs b/src/client/mod.rs
index 4ad5c2d..ad0b14d 100644
--- a/src/client/mod.rs
+++ b/src/client/mod.rs
@@ -271,10 +271,10 @@ impl<'a> RequestBuilder<'a> {

         loop {
             let mut req = {
+                let hp = try!(get_host_and_port(&url));
                 let (scheme, host, port) = match client.proxy {
                     Some(ref proxy) => (proxy.0.as_ref(), proxy.1.as_ref(), proxy.2),
                     None => {
-                        let hp = try!(get_host_and_port(&url));
                         (url.scheme(), hp.0, hp.1)
                     }
                 };
@@ -282,11 +282,14 @@ impl<'a> RequestBuilder<'a> {
                     Some(ref headers) => headers.clone(),
                     None => Headers::new(),
                 };
+                // Host needs to be the destination host not the proxy
                 headers.set(Host {
-                    hostname: host.to_owned(),
-                    port: Some(port),
+                    hostname: hp.0.to_owned(),
+                    port: Some(hp.1),
                 });
+                // now connect to the proxy
                 let message = try!(client.protocol.new_message(&host, port, scheme));
+                // and make a request for the destination
                 Request::with_headers_and_message(method.clone(), url.clone(), headers, messag\

e)
};

@seanmonstar
Copy link
Member

I see, I misunderstood the spec. I understood that the Host header is the host of the proxy server, with the absolute-uri in the request line aimed at the target host. Re-reading the spec, it seems I am indeed incorrect, as HTTP/1.0 proxies may just forward the Host header directly.

It may be that the proxy you are using is indeed using version HTTP/1.0, or otherwise has a bug itself
as I noticed this is also in the spec:

When a proxy receives a request with an absolute-form of
request-target, the proxy MUST ignore the received Host header field
(if any) and instead replace it with the host information of the
request-target. A proxy that forwards such a request MUST generate a
new Host field-value based on the received request-target rather than
forward the received Host field-value.

@shaleh
Copy link
Author

shaleh commented Apr 27, 2016

That would not surprise me. yay corp IT.

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

No branches or pull requests

2 participants