Fix for HTTPCLIENT-1296. Change where the virtual host is evaluated until after we figure out the target in the absence of a virtual host.

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

RELEASE_NOTES.txt

@@ -1,6 +1,10 @@
 Changes in trunk
 -------------------
 
+* [HTTPCLIENT-1296] NPE gets thrown if you combine a default host with a virtual host
+  that has a -1 value for the port.
+  Contributed by Karl Wright <daddywri at gmail.com>
+  
 * [HTTPCLIENT-1290] 304 cached response never reused with If-modified-since conditional 
   requests. 
   Contributed by Francois-Xavier Bonnet <francois-xavier.bonnet at centraliens.net>

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

@@ -380,14 +380,6 @@ public class DefaultRequestDirector implements RequestDirector {
 
         virtualHost = (HttpHost) origWrapper.getParams().getParameter(ClientPNames.VIRTUAL_HOST);
 
-        // HTTPCLIENT-1092 - add the port if necessary
-        if (virtualHost != null && virtualHost.getPort() == -1) {
-            int port = target.getPort();
-            if (port != -1){
-                virtualHost = new HttpHost(virtualHost.getHostName(), port, virtualHost.getSchemeName());
-            }
-        }
-
         RoutedRequest roureq = new RoutedRequest(origWrapper, origRoute);
 
         boolean reuse = false;
@@ -456,19 +448,28 @@ public class DefaultRequestDirector implements RequestDirector {
                             new BasicScheme(), new UsernamePasswordCredentials(userinfo));
                 }
 
-                if (virtualHost != null) {
+                // Get target.  Even if there's virtual host, we may need the target to set the port.
-                    target = virtualHost;
+                URI requestURI = wrapper.getURI();
-                } else {
+                if (requestURI.isAbsolute()) {
-                    URI requestURI = wrapper.getURI();
+                    target = new HttpHost(
-                    if (requestURI.isAbsolute()) {
+                        requestURI.getHost(), requestURI.getPort(), requestURI.getScheme());
-                        target = new HttpHost(
-                                requestURI.getHost(), requestURI.getPort(), requestURI.getScheme());
-                    }
                 }
                 if (target == null) {
                     target = route.getTargetHost();
                 }
 
+                // Override the target if the virtual host is present.  But make sure the port is right.
+                if (virtualHost != null) {
+                    // HTTPCLIENT-1092 - add the port if necessary
+                    if (virtualHost.getPort() == -1 && target != null) {
+                        int port = target.getPort();
+                        if (port != -1) {
+                            virtualHost = new HttpHost(virtualHost.getHostName(), port, virtualHost.getSchemeName());
+                        }
+                    }
+                    target = virtualHost;
+                }
+
                 // Reset headers on the request wrapper
                 wrapper.resetHeaders();
                 // Re-write request URI if needed

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

@@ -38,6 +38,7 @@ import org.apache.http.HttpRequest;
 import org.apache.http.HttpRequestInterceptor;
 import org.apache.http.HttpResponse;
 import org.apache.http.HttpStatus;
+import org.apache.http.impl.client.DefaultHttpClient;
 import org.apache.http.client.ClientProtocolException;
 import org.apache.http.client.HttpRequestRetryHandler;
 import org.apache.http.client.NonRepeatableRequestException;
@@ -54,6 +55,8 @@ import org.apache.http.protocol.HttpContext;
 import org.apache.http.protocol.HttpRequestExecutor;
 import org.apache.http.protocol.HttpRequestHandler;
 import org.apache.http.util.EntityUtils;
+import org.apache.http.client.params.ClientPNames;
+
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
@@ -220,6 +223,24 @@ public class TestClientRequestExecution extends IntegrationTestBase {
         Assert.assertEquals("blah.:.blah.:.", reqWrapper.getRequestLine().getUri());
     }
 
+    @Test
+    public void testDefaultPortVirtualHost() throws Exception {
+        this.localServer.register("*", new SimpleService());
+        this.httpclient = new DefaultHttpClient();
+        HttpHost target = getServerHttp();
+        HttpHost hostHost = new HttpHost(target.getHostName(),-1,target.getSchemeName());
+
+        httpclient.getParams().setParameter(ClientPNames.DEFAULT_HOST,target);
+        httpclient.getParams().setParameter(ClientPNames.VIRTUAL_HOST,hostHost);
+
+        HttpGet httpget = new HttpGet("/stuff");
+        HttpContext context = new BasicHttpContext();
+
+        HttpResponse response = this.httpclient.execute(null, httpget, context);
+        Assert.assertEquals(HttpStatus.SC_OK, response.getStatusLine().getStatusCode());
+        EntityUtils.consume(response.getEntity());
+    }
+
     @Test
     public void testRelativeRequestURIWithFragment() throws Exception {
         this.localServer.register("*", new SimpleService());