HTTPCLIENT-1284: HttpClient incorrectly generates Host header when physical connection route differs from the host name specified in the request URI

git-svn-id: https://svn.apache.org/repos/asf/httpcomponents/httpclient/trunk@1424448 13f79535-47bb-0310-9956-ffa450edef68

RELEASE_NOTES.txt

@@ -1,6 +1,10 @@
 Changes in trunk
 -------------------
 
+* [HTTPCLIENT-1284] HttpClient incorrectly generates Host header when physical connection
+  route differs from the host name specified in the request URI. 
+  Contributed by Oleg Kalnichevski <olegk at apache.org>
+
 * Kerberos and SPNego auth schemes use incorrect authorization header name when authenticating
   with a proxy.
   Contributed by Oleg Kalnichevski <olegk at apache.org>

httpclient/src/main/java/org/apache/http/impl/client/DefaultRequestDirector.java

@@ -456,19 +456,24 @@ public class DefaultRequestDirector implements RequestDirector {
                             new BasicScheme(), new UsernamePasswordCredentials(userinfo));
                 }
 
-                // Reset headers on the request wrapper
+                if (virtualHost != null) {
-                wrapper.resetHeaders();
+                    target = virtualHost;
-
+                } else {
-                // Re-write request URI if needed
+                    URI requestURI = wrapper.getURI();
-                rewriteRequestURI(wrapper, route);
+                    if (requestURI.isAbsolute()) {
-
+                        target = new HttpHost(
-                // Use virtual host if set
+                                requestURI.getHost(), requestURI.getPort(), requestURI.getScheme());
-                target = virtualHost;
+                    }
-
+                }
                 if (target == null) {
                     target = route.getTargetHost();
                 }
 
+                // Reset headers on the request wrapper
+                wrapper.resetHeaders();
+                // Re-write request URI if needed
+                rewriteRequestURI(wrapper, route);
+
                 // Populate the execution context
                 context.setAttribute(ExecutionContext.HTTP_TARGET_HOST, target);
                 context.setAttribute(ClientContext.ROUTE, route);

httpclient/src/main/java/org/apache/http/impl/client/execchain/ProtocolExec.java

@@ -35,6 +35,7 @@ import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apache.http.HttpException;
 import org.apache.http.HttpHost;
+import org.apache.http.HttpRequest;
 import org.apache.http.ProtocolException;
 import org.apache.http.annotation.Immutable;
 import org.apache.http.auth.AuthState;
@@ -42,6 +43,7 @@ import org.apache.http.auth.UsernamePasswordCredentials;
 import org.apache.http.client.methods.CloseableHttpResponse;
 import org.apache.http.client.methods.HttpExecutionAware;
 import org.apache.http.client.methods.HttpRequestWrapper;
+import org.apache.http.client.methods.HttpUriRequest;
 import org.apache.http.client.params.ClientPNames;
 import org.apache.http.client.protocol.ClientContext;
 import org.apache.http.client.protocol.HttpClientContext;
@@ -113,15 +115,16 @@ public class ProtocolExec implements ClientExecChain {
         Args.notNull(request, "HTTP request");
         Args.notNull(context, "HTTP context");
 
-        HttpHost target = route.getTargetHost();
-
         // Get user info from the URI
         AuthState targetAuthState = context.getTargetAuthState();
         if (targetAuthState != null) {
-            String userinfo = request.getURI().getUserInfo();
+            URI uri = request.getURI();
-            if (userinfo != null) {
+            if (uri != null) {
-                targetAuthState.update(
+                String userinfo = uri.getUserInfo();
-                        new BasicScheme(), new UsernamePasswordCredentials(userinfo));
+                if (userinfo != null) {
+                    targetAuthState.update(
+                            new BasicScheme(), new UsernamePasswordCredentials(userinfo));
+                }
             }
         }
 
@@ -132,7 +135,7 @@ public class ProtocolExec implements ClientExecChain {
         HttpHost virtualHost = (HttpHost) params.getParameter(ClientPNames.VIRTUAL_HOST);
         // HTTPCLIENT-1092 - add the port if necessary
         if (virtualHost != null && virtualHost.getPort() == -1) {
-            int port = target.getPort();
+            int port = route.getTargetHost().getPort();
             if (port != -1){
                 virtualHost = new HttpHost(virtualHost.getHostName(), port, virtualHost.getSchemeName());
             }
@@ -141,8 +144,24 @@ public class ProtocolExec implements ClientExecChain {
             }
         }
 
+        HttpHost target = null;
+        if (virtualHost != null) {
+            target = virtualHost;
+        } else {
+            HttpRequest original = request.getOriginal();
+            if (original instanceof HttpUriRequest) {
+                URI uri = ((HttpUriRequest) original).getURI();
+                if (uri.isAbsolute()) {
+                    target = new HttpHost(uri.getHost(), uri.getPort(), uri.getScheme());
+                }
+            }
+        }
+        if (target == null) {
+            target = route.getTargetHost();
+        }
+
         // Run request protocol interceptors
-        context.setAttribute(ExecutionContext.HTTP_TARGET_HOST, virtualHost != null ? virtualHost : target);
+        context.setAttribute(ExecutionContext.HTTP_TARGET_HOST, target);
         context.setAttribute(ClientContext.ROUTE, route);
         context.setAttribute(ExecutionContext.HTTP_REQUEST, request);
 

httpclient/src/test/java/org/apache/http/impl/client/integration/TestCookieVirtualHost.java

@@ -0,0 +1,153 @@
+/*
+ * ====================================================================
+ *
+ *  Licensed to the Apache Software Foundation (ASF) under one or more
+ *  contributor license agreements.  See the NOTICE file distributed with
+ *  this work for additional information regarding copyright ownership.
+ *  The ASF licenses this file to You under the Apache License, Version 2.0
+ *  (the "License"); you may not use this file except in compliance with
+ *  the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ * ====================================================================
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals on behalf of the Apache Software Foundation.  For more
+ * information on the Apache Software Foundation, please see
+ * <http://www.apache.org/>.
+ */
+package org.apache.http.impl.client.integration;
+
+import java.io.IOException;
+import java.net.URI;
+import java.util.List;
+
+import org.apache.http.HttpEntity;
+import org.apache.http.HttpException;
+import org.apache.http.HttpRequest;
+import org.apache.http.HttpResponse;
+import org.apache.http.HttpStatus;
+import org.apache.http.HttpVersion;
+import org.apache.http.client.CookieStore;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.client.protocol.ClientContext;
+import org.apache.http.cookie.Cookie;
+import org.apache.http.impl.client.BasicCookieStore;
+import org.apache.http.impl.client.HttpClients;
+import org.apache.http.localserver.LocalTestServer;
+import org.apache.http.message.BasicHeader;
+import org.apache.http.protocol.BasicHttpContext;
+import org.apache.http.protocol.HttpContext;
+import org.apache.http.protocol.HttpRequestHandler;
+import org.apache.http.util.EntityUtils;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+/**
+ * This class tests cookie matching when using Virtual Host.
+ */
+public class TestCookieVirtualHost extends IntegrationTestBase {
+
+    @Before
+    public void setUp() throws Exception {
+        this.localServer = new LocalTestServer(null, null);
+        this.localServer.registerDefaultHandlers();
+        this.localServer.start();
+    }
+
+    @Test
+    public void testCookieMatchingWithVirtualHosts() throws Exception {
+        this.localServer.register("*", new HttpRequestHandler() {
+            public void handle(
+                    final HttpRequest request,
+                    final HttpResponse response,
+                    final HttpContext context) throws HttpException, IOException {
+
+                int n = Integer.parseInt(request.getFirstHeader("X-Request").getValue());
+                switch (n) {
+                case 1:
+                    // Assert Host is forwarded from URI
+                    Assert.assertEquals("app.mydomain.fr", request
+                            .getFirstHeader("Host").getValue());
+
+                    response.setStatusLine(HttpVersion.HTTP_1_1,
+                            HttpStatus.SC_OK);
+                    // Respond with Set-Cookie on virtual host domain. This
+                    // should be valid.
+                    response.addHeader(new BasicHeader("Set-Cookie",
+                            "name1=value1; domain=mydomain.fr; path=/"));
+                    break;
+
+                case 2:
+                    // Assert Host is still forwarded from URI
+                    Assert.assertEquals("app.mydomain.fr", request
+                            .getFirstHeader("Host").getValue());
+
+                    // We should get our cookie back.
+                    Assert.assertNotNull("We must get a cookie header",
+                            request.getFirstHeader("Cookie"));
+                    response.setStatusLine(HttpVersion.HTTP_1_1,
+                            HttpStatus.SC_OK);
+                    break;
+
+                case 3:
+                    // Assert Host is forwarded from URI
+                    Assert.assertEquals("app.mydomain.fr", request
+                            .getFirstHeader("Host").getValue());
+
+                    response.setStatusLine(HttpVersion.HTTP_1_1,
+                            HttpStatus.SC_OK);
+                    break;
+                }
+            }
+
+        });
+
+        this.httpclient = HttpClients.createDefault();
+
+        CookieStore cookieStore = new BasicCookieStore();
+        HttpContext context = new BasicHttpContext();
+        context.setAttribute(ClientContext.COOKIE_STORE, cookieStore);
+
+        // First request : retrieve a domain cookie from remote server.
+        URI uri = new URI("http://app.mydomain.fr");
+        HttpRequest httpRequest = new HttpGet(uri);
+        httpRequest.addHeader("X-Request", "1");
+        HttpResponse response1 = this.httpclient.execute(getServerHttp(),
+                httpRequest, context);
+        HttpEntity e1 = response1.getEntity();
+        EntityUtils.consume(e1);
+
+        // We should have one cookie set on domain.
+        List<Cookie> cookies = cookieStore.getCookies();
+        Assert.assertNotNull(cookies);
+        Assert.assertEquals(1, cookies.size());
+        Assert.assertEquals("name1", cookies.get(0).getName());
+
+        // Second request : send the cookie back.
+        uri = new URI("http://app.mydomain.fr");
+        httpRequest = new HttpGet(uri);
+        httpRequest.addHeader("X-Request", "2");
+        HttpResponse response2 = this.httpclient.execute(getServerHttp(),
+                httpRequest, context);
+        HttpEntity e2 = response2.getEntity();
+        EntityUtils.consume(e2);
+
+        // Third request : Host header
+        uri = new URI("http://app.mydomain.fr");
+        httpRequest = new HttpGet(uri);
+        httpRequest.addHeader("X-Request", "3");
+        HttpResponse response3 = this.httpclient.execute(getServerHttp(),
+                httpRequest, context);
+        HttpEntity e3 = response3.getEntity();
+        EntityUtils.consume(e3);
+    }
+
+}