diff --git a/RELEASE_NOTES.txt b/RELEASE_NOTES.txt index d5c703dc7..bb513457f 100644 --- a/RELEASE_NOTES.txt +++ b/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 + * [HTTPCLIENT-1290] 304 cached response never reused with If-modified-since conditional requests. Contributed by Francois-Xavier Bonnet 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 7a48f64c8..d7f226e35 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 @@ -380,14 +380,6 @@ public HttpResponse execute(HttpHost target, HttpRequest request, 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 HttpResponse execute(HttpHost target, HttpRequest request, 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 diff --git a/httpclient/src/test/java/org/apache/http/impl/client/integration/TestClientRequestExecution.java b/httpclient/src/test/java/org/apache/http/impl/client/integration/TestClientRequestExecution.java index 2ba97e84c..e9c42fb37 100644 --- a/httpclient/src/test/java/org/apache/http/impl/client/integration/TestClientRequestExecution.java +++ b/httpclient/src/test/java/org/apache/http/impl/client/integration/TestClientRequestExecution.java @@ -38,6 +38,7 @@ 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.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 void testNonCompliantURI() throws Exception { 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());