You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by kw...@apache.org on 2013/01/10 03:47:32 UTC

svn commit: r1431183 - in /httpcomponents/httpclient/trunk: RELEASE_NOTES.txt httpclient/src/main/java/org/apache/http/impl/client/DefaultRequestDirector.java httpclient/src/test/java/org/apache/http/impl/client/integration/TestClientRequestExecution.java

Author: kwright
Date: Thu Jan 10 02:47:32 2013
New Revision: 1431183

URL: http://svn.apache.org/viewvc?rev=1431183&view=rev
Log:
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.

Modified:
    httpcomponents/httpclient/trunk/RELEASE_NOTES.txt
    httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/client/DefaultRequestDirector.java
    httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/client/integration/TestClientRequestExecution.java

Modified: httpcomponents/httpclient/trunk/RELEASE_NOTES.txt
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/RELEASE_NOTES.txt?rev=1431183&r1=1431182&r2=1431183&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/RELEASE_NOTES.txt (original)
+++ httpcomponents/httpclient/trunk/RELEASE_NOTES.txt Thu Jan 10 02:47:32 2013
@@ -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>

Modified: httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/client/DefaultRequestDirector.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/client/DefaultRequestDirector.java?rev=1431183&r1=1431182&r2=1431183&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/client/DefaultRequestDirector.java (original)
+++ httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/client/DefaultRequestDirector.java Thu Jan 10 02:47:32 2013
@@ -380,14 +380,6 @@ public class DefaultRequestDirector impl
 
         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 impl
                             new BasicScheme(), new UsernamePasswordCredentials(userinfo));
                 }
 
-                if (virtualHost != null) {
-                    target = virtualHost;
-                } else {
-                    URI requestURI = wrapper.getURI();
-                    if (requestURI.isAbsolute()) {
-                        target = new HttpHost(
-                                requestURI.getHost(), requestURI.getPort(), requestURI.getScheme());
-                    }
+                // Get target.  Even if there's virtual host, we may need the target to set the port.
+                URI requestURI = wrapper.getURI();
+                if (requestURI.isAbsolute()) {
+                    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

Modified: httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/client/integration/TestClientRequestExecution.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/client/integration/TestClientRequestExecution.java?rev=1431183&r1=1431182&r2=1431183&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/client/integration/TestClientRequestExecution.java (original)
+++ httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/client/integration/TestClientRequestExecution.java Thu Jan 10 02:47:32 2013
@@ -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.HttpCont
 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;
@@ -221,6 +224,24 @@ public class TestClientRequestExecution 
     }
 
     @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());
         this.httpclient = HttpClients.createDefault();