From e798d4947cb86373431020a41da9d24a5f16e414 Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Thu, 20 Dec 2012 13:05:41 +0000 Subject: [PATCH] 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 | 4 + .../impl/client/DefaultRequestDirector.java | 23 +-- .../impl/client/execchain/ProtocolExec.java | 35 +++- .../integration/TestCookieVirtualHost.java | 153 ++++++++++++++++++ 4 files changed, 198 insertions(+), 17 deletions(-) create mode 100644 httpclient/src/test/java/org/apache/http/impl/client/integration/TestCookieVirtualHost.java diff --git a/RELEASE_NOTES.txt b/RELEASE_NOTES.txt index d4e5216c3..f35e80849 100644 --- a/RELEASE_NOTES.txt +++ b/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 + * Kerberos and SPNego auth schemes use incorrect authorization header name when authenticating with a proxy. Contributed by Oleg Kalnichevski diff --git a/httpclient/src/main/java/org/apache/http/impl/client/DefaultRequestDirector.java b/httpclient/src/main/java/org/apache/http/impl/client/DefaultRequestDirector.java index e6ec1a536..eccf2bb85 100644 --- a/httpclient/src/main/java/org/apache/http/impl/client/DefaultRequestDirector.java +++ b/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 - wrapper.resetHeaders(); - - // Re-write request URI if needed - rewriteRequestURI(wrapper, route); - - // Use virtual host if set - target = virtualHost; - + if (virtualHost != null) { + target = virtualHost; + } else { + URI requestURI = wrapper.getURI(); + if (requestURI.isAbsolute()) { + target = new HttpHost( + requestURI.getHost(), requestURI.getPort(), requestURI.getScheme()); + } + } 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); diff --git a/httpclient/src/main/java/org/apache/http/impl/client/execchain/ProtocolExec.java b/httpclient/src/main/java/org/apache/http/impl/client/execchain/ProtocolExec.java index 1d6c1f2a0..376ae2794 100644 --- a/httpclient/src/main/java/org/apache/http/impl/client/execchain/ProtocolExec.java +++ b/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(); - if (userinfo != null) { - targetAuthState.update( - new BasicScheme(), new UsernamePasswordCredentials(userinfo)); + URI uri = request.getURI(); + if (uri != null) { + String userinfo = uri.getUserInfo(); + 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); diff --git a/httpclient/src/test/java/org/apache/http/impl/client/integration/TestCookieVirtualHost.java b/httpclient/src/test/java/org/apache/http/impl/client/integration/TestCookieVirtualHost.java new file mode 100644 index 000000000..576174f31 --- /dev/null +++ b/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 + * . + */ +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 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); + } + +}