HTTPCLIENT-1325: Using a HttpRequest that is not an HttpUriRequest wh…

…en host in URI is not target host results in an invalid request

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

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

@@ -67,18 +67,15 @@ public class ProtocolExec implements ClientExecChain {
     private final ClientExecChain requestExecutor;
     private final HttpProcessor httpProcessor;
 
-    public ProtocolExec(
-            final ClientExecChain requestExecutor,
-            final HttpProcessor httpProcessor) {
+    public ProtocolExec(final ClientExecChain requestExecutor, final HttpProcessor httpProcessor) {
         Args.notNull(requestExecutor, "HTTP client request executor");
         Args.notNull(httpProcessor, "HTTP protocol processor");
         this.requestExecutor = requestExecutor;
         this.httpProcessor = httpProcessor;
     }
 
-    private void rewriteRequestURI(
-            final HttpRequestWrapper request,
-            final HttpRoute route) throws ProtocolException {
+    private void rewriteRequestURI(final HttpRequestWrapper request, final HttpRoute route)
+        throws ProtocolException {
         try {
             URI uri = request.getURI();
             if (uri != null) {
@@ -101,16 +98,13 @@ private void rewriteRequestURI(
                 request.setURI(uri);
             }
         } catch (final URISyntaxException ex) {
-            throw new ProtocolException("Invalid URI: " +
-                    request.getRequestLine().getUri(), ex);
+            throw new ProtocolException("Invalid URI: " + request.getRequestLine().getUri(), ex);
         }
     }
 
-    public CloseableHttpResponse execute(
-            final HttpRoute route,
-            final HttpRequestWrapper request,
-            final HttpClientContext context,
-            final HttpExecutionAware execAware) throws IOException, HttpException {
+    public CloseableHttpResponse execute(final HttpRoute route, final HttpRequestWrapper request,
+        final HttpClientContext context, final HttpExecutionAware execAware) throws IOException,
+        HttpException {
         Args.notNull(route, "HTTP route");
         Args.notNull(request, "HTTP request");
         Args.notNull(context, "HTTP context");
@@ -122,8 +116,8 @@ public CloseableHttpResponse execute(
             if (uri != null) {
                 final String userinfo = uri.getUserInfo();
                 if (userinfo != null) {
-                    targetAuthState.update(
-                            new BasicScheme(), new UsernamePasswordCredentials(userinfo));
+                    targetAuthState.update(new BasicScheme(), new UsernamePasswordCredentials(
+                        userinfo));
                 }
             }
         }
@@ -136,8 +130,9 @@ public CloseableHttpResponse execute(
         // HTTPCLIENT-1092 - add the port if necessary
         if (virtualHost != null && virtualHost.getPort() == -1) {
             final int port = route.getTargetHost().getPort();
-            if (port != -1){
-                virtualHost = new HttpHost(virtualHost.getHostName(), port, virtualHost.getSchemeName());
+            if (port != -1) {
+                virtualHost = new HttpHost(virtualHost.getHostName(), port,
+                    virtualHost.getSchemeName());
             }
             if (this.log.isDebugEnabled()) {
                 this.log.debug("Using virtual host" + virtualHost);
@@ -149,11 +144,20 @@ public CloseableHttpResponse execute(
             target = virtualHost;
         } else {
             final HttpRequest original = request.getOriginal();
+            URI uri = null;
             if (original instanceof HttpUriRequest) {
-                final URI uri = ((HttpUriRequest) original).getURI();
-                if (uri.isAbsolute()) {
-                    target = new HttpHost(uri.getHost(), uri.getPort(), uri.getScheme());
+                uri = ((HttpUriRequest) original).getURI();
+            } else {
+                final String uriString = original.getRequestLine().getUri();
+                try {
+                    uri = URI.create(uriString);
+                } catch (IllegalArgumentException e) {
+                    log.debug("Could not parse " + uriString + " as a URI to set Host header");
                 }
+
+            }
+            if (uri != null && uri.isAbsolute() && uri.getHost() != null) {
+                target = new HttpHost(uri.getHost(), uri.getPort(), uri.getScheme());
             }
         }
         if (target == null) {
@@ -167,7 +171,8 @@ public CloseableHttpResponse execute(
 
         this.httpProcessor.process(request, context);
 
-        final CloseableHttpResponse response = this.requestExecutor.execute(route, request, context, execAware);
+        final CloseableHttpResponse response = this.requestExecutor.execute(route, request,
+            context, execAware);
         try {
             // Run response protocol interceptors
             context.setAttribute(ExecutionContext.HTTP_RESPONSE, response);

httpclient/src/test/java/org/apache/http/impl/execchain/TestProtocolExec.java

@@ -0,0 +1,84 @@
+/*
+ * ====================================================================
+ * 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.execchain;
+
+import org.apache.http.HttpHost;
+import org.apache.http.client.methods.HttpExecutionAware;
+import org.apache.http.client.methods.HttpRequestWrapper;
+import org.apache.http.client.protocol.HttpClientContext;
+import org.apache.http.conn.routing.HttpRoute;
+import org.apache.http.message.BasicHttpRequest;
+import org.apache.http.params.BasicHttpParams;
+import org.apache.http.params.HttpParams;
+import org.apache.http.protocol.ExecutionContext;
+import org.apache.http.protocol.HttpProcessor;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+public class TestProtocolExec {
+
+    private ClientExecChain requestExecutor;
+    private HttpProcessor httpProcessor;
+    private ProtocolExec protocolExec;
+    private HttpClientContext context;
+    private HttpRequestWrapper request;
+    private HttpExecutionAware execAware;
+    private HttpRoute route;
+    private HttpParams params = new BasicHttpParams();
+
+    @Before
+    public void setup() throws Exception {
+        requestExecutor = Mockito.mock(ClientExecChain.class);
+        httpProcessor = Mockito.mock(HttpProcessor.class);
+        protocolExec = new ProtocolExec(requestExecutor, httpProcessor);
+        route = new HttpRoute(new HttpHost("foo", 8080));
+        context = new HttpClientContext();
+        execAware = Mockito.mock(HttpExecutionAware.class);
+    }
+
+    @Test
+    public void testHostHeaderWhenNonUriRequest() throws Exception {
+        request = HttpRequestWrapper.wrap(new BasicHttpRequest("GET", "http://bar/test"));
+        protocolExec.execute(route, request, context, execAware);
+        // ProtocolExect should have extracted the host from request URI
+        Assert.assertEquals(new HttpHost("bar", -1, "http"),
+            context.getAttribute(ExecutionContext.HTTP_TARGET_HOST));
+    }
+
+    @Test
+    public void testHostHeaderWhenNonUriRequestAndInvalidUri() throws Exception {
+        request = HttpRequestWrapper.wrap(new BasicHttpRequest("GET", "http://bar/test|"));
+        protocolExec.execute(route, request, context, execAware);
+        // ProtocolExect should have fall back to physical host as request URI
+        // is not parseable
+        Assert.assertEquals(new HttpHost("foo", 8080, "http"),
+            context.getAttribute(ExecutionContext.HTTP_TARGET_HOST));
+    }
+
+}