diff --git a/RELEASE_NOTES.txt b/RELEASE_NOTES.txt index 1defc0a75..8b91ac9a6 100644 --- a/RELEASE_NOTES.txt +++ b/RELEASE_NOTES.txt @@ -1,6 +1,10 @@ Changes since 4.0 Alpha 2 ------------------- +* [HTTPCLIENT-730] Fixed rewriting of URIs containing escaped characters + Contributed by Sam Berlin and + Oleg Kalnichevski + * [HTTPCLIENT-667] Added 'Meta' cookie specification that selects a cookie policy depending on the format of the cookie(s). It is no longer necessary to know beforehand what kind of HTTP cookie support the target host provides. diff --git a/module-client/src/main/java/org/apache/http/client/utils/URLUtils.java b/module-client/src/main/java/org/apache/http/client/utils/URLUtils.java index 97111f76f..66c14cd6e 100644 --- a/module-client/src/main/java/org/apache/http/client/utils/URLUtils.java +++ b/module-client/src/main/java/org/apache/http/client/utils/URLUtils.java @@ -30,8 +30,11 @@ package org.apache.http.client.utils; import java.io.UnsupportedEncodingException; +import java.net.URI; +import java.net.URISyntaxException; import org.apache.commons.codec.net.URLCodec; +import org.apache.http.HttpHost; import org.apache.http.NameValuePair; import org.apache.http.util.CharArrayBuffer; @@ -123,6 +126,75 @@ public class URLUtils { return buf.toString(); } + public static URI createURI( + final String scheme, + final String host, + int port, + final String path, + final String query, + final String fragment) throws URISyntaxException { + + StringBuilder buffer = new StringBuilder(); + if (host != null) { + if (scheme != null) { + buffer.append(scheme); + buffer.append("://"); + } + buffer.append(host); + if (port > 0) { + buffer.append(":"); + buffer.append(port); + } + } + if (path == null || !path.startsWith("/")) { + buffer.append("/"); + } + if (path != null) { + buffer.append(path); + } + if (query != null) { + buffer.append("?"); + buffer.append(query); + } + if (fragment != null) { + buffer.append("#"); + buffer.append(fragment); + } + return new URI(buffer.toString()); + } + + public static URI rewriteURI( + final URI uri, + final HttpHost target, + boolean dropFragment) throws URISyntaxException { + if (uri == null) { + throw new IllegalArgumentException("URI may nor be null"); + } + if (target != null) { + return URLUtils.createURI( + target.getSchemeName(), + target.getHostName(), + target.getPort(), + uri.getRawPath(), + uri.getRawQuery(), + dropFragment ? null : uri.getRawFragment()); + } else { + return URLUtils.createURI( + null, + null, + -1, + uri.getRawPath(), + uri.getRawQuery(), + dropFragment ? null : uri.getRawFragment()); + } + } + + public static URI rewriteURI( + final URI uri, + final HttpHost target) throws URISyntaxException { + return rewriteURI(uri, target, false); + } + /** * This class should not be instantiated. */ diff --git a/module-client/src/main/java/org/apache/http/impl/client/DefaultClientRequestDirector.java b/module-client/src/main/java/org/apache/http/impl/client/DefaultClientRequestDirector.java index 08643b6ad..ba0fe8163 100644 --- a/module-client/src/main/java/org/apache/http/impl/client/DefaultClientRequestDirector.java +++ b/module-client/src/main/java/org/apache/http/impl/client/DefaultClientRequestDirector.java @@ -66,6 +66,7 @@ import org.apache.http.client.methods.HttpGet; import org.apache.http.client.params.ClientPNames; import org.apache.http.client.params.HttpClientParams; import org.apache.http.client.protocol.ClientContext; +import org.apache.http.client.utils.URLUtils; import org.apache.http.conn.BasicManagedEntity; import org.apache.http.conn.ClientConnectionManager; import org.apache.http.conn.ConnectionPoolTimeoutException; @@ -246,23 +247,13 @@ public class DefaultClientRequestDirector // Make sure the request URI is absolute if (!uri.isAbsolute()) { HttpHost target = route.getTargetHost(); - uri = new URI( - target.getSchemeName(), - null, - target.getHostName(), - target.getPort(), - uri.getPath(), - uri.getQuery(), - uri.getFragment()); + uri = URLUtils.rewriteURI(uri, target); request.setURI(uri); } } else { // Make sure the request URI is relative if (uri.isAbsolute()) { - uri = new URI(null, null, null, -1, - uri.getPath(), - uri.getQuery(), - uri.getFragment()); + uri = URLUtils.rewriteURI(uri, null); request.setURI(uri); } } diff --git a/module-client/src/main/java/org/apache/http/impl/client/DefaultRedirectHandler.java b/module-client/src/main/java/org/apache/http/impl/client/DefaultRedirectHandler.java index 3164a5225..8f999d7d6 100644 --- a/module-client/src/main/java/org/apache/http/impl/client/DefaultRedirectHandler.java +++ b/module-client/src/main/java/org/apache/http/impl/client/DefaultRedirectHandler.java @@ -45,6 +45,7 @@ import org.apache.http.ProtocolException; import org.apache.http.client.CircularRedirectException; import org.apache.http.client.RedirectHandler; import org.apache.http.client.params.ClientPNames; +import org.apache.http.client.utils.URLUtils; import org.apache.http.params.HttpParams; import org.apache.http.protocol.HttpContext; import org.apache.http.protocol.ExecutionContext; @@ -136,14 +137,7 @@ public class DefaultRedirectHandler implements RedirectHandler { try { URI requestURI = new URI(request.getRequestLine().getUri()); - URI absoluteRequestURI = new URI( - target.getSchemeName(), - null, - target.getHostName(), - target.getPort(), - requestURI.getPath(), - requestURI.getQuery(), - null); + URI absoluteRequestURI = URLUtils.rewriteURI(requestURI, target, true); uri = absoluteRequestURI.resolve(uri); } catch (URISyntaxException ex) { throw new ProtocolException(ex.getMessage(), ex); diff --git a/module-client/src/test/java/org/apache/http/client/protocol/TestUriEscapes.java b/module-client/src/test/java/org/apache/http/client/protocol/TestUriEscapes.java new file mode 100644 index 000000000..10c0f7c6a --- /dev/null +++ b/module-client/src/test/java/org/apache/http/client/protocol/TestUriEscapes.java @@ -0,0 +1,157 @@ +/* + * $HeadURL:$ + * $Revision:$ + * $Date:$ + * ==================================================================== + * + * 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.client.protocol; + +import java.io.IOException; + +import junit.framework.Test; +import junit.framework.TestSuite; + +import org.apache.http.HttpException; +import org.apache.http.HttpHost; +import org.apache.http.HttpRequest; +import org.apache.http.HttpResponse; +import org.apache.http.HttpStatus; +import org.apache.http.ProtocolVersion; +import org.apache.http.client.methods.HttpGet; +import org.apache.http.entity.StringEntity; +import org.apache.http.impl.client.DefaultHttpClient; +import org.apache.http.localserver.ServerTestBase; +import org.apache.http.protocol.HttpContext; +import org.apache.http.protocol.HttpRequestHandler; + +public class TestUriEscapes extends ServerTestBase { + + public TestUriEscapes(final String testName) throws IOException { + super(testName); + } + + public static void main(String args[]) { + String[] testCaseName = { TestUriEscapes.class.getName() }; + junit.textui.TestRunner.main(testCaseName); + } + + public static Test suite() { + return new TestSuite(TestUriEscapes.class); + } + + private class UriListeningService implements HttpRequestHandler { + + private volatile String requestedUri; + + public void handle( + final HttpRequest request, + final HttpResponse response, + final HttpContext context) throws HttpException, IOException { + ProtocolVersion ver = request.getRequestLine().getProtocolVersion(); + this.requestedUri = request.getRequestLine().getUri(); + response.setStatusLine(ver, HttpStatus.SC_OK); + StringEntity entity = new StringEntity("Response Body"); + response.setEntity(entity); + } + + public String getRequestedUri() { + return requestedUri; + } + } + + private void doTest(String uri, boolean relative) throws Exception { + int port = this.localServer.getServicePort(); + String host = "localhost"; + UriListeningService listener = new UriListeningService(); + this.localServer.register("*", listener); + + DefaultHttpClient client = new DefaultHttpClient(); + HttpResponse response; + + if(!relative) { + String request = "http://" + host + ":" + port + uri; + HttpGet httpget = new HttpGet(request); + response = client.execute(httpget); + response.getEntity().consumeContent(); + } else { + HttpHost target = new HttpHost(host, port); + HttpGet httpget = new HttpGet(uri); + response = client.execute(target, httpget); + response.getEntity().consumeContent(); + } + + assertEquals(HttpStatus.SC_OK, response.getStatusLine().getStatusCode()); + assertEquals(uri, listener.getRequestedUri()); + } + + public void testEscapedAmpersandInQueryAbsolute() throws Exception { + doTest("/path/a=b&c=%26d", false); + } + + public void testEscapedAmpersandInQueryRelative() throws Exception { + doTest("/path/a=b&c=%26d", true); + } + + public void testPlusInPathAbsolute() throws Exception { + doTest("/path+go", false); + } + + public void testPlusInPathRelative() throws Exception { + doTest("/path+go", true); + } + + public void testEscapedSpaceInPathAbsolute() throws Exception { + doTest("/path%20go?a=b&c=d", false); + } + + public void testEscapedSpaceInPathRelative() throws Exception { + doTest("/path%20go?a=b&c=d", true); + } + + public void testEscapedAmpersandInPathAbsolute() throws Exception { + doTest("/this%26that?a=b&c=d", false); + } + + public void testEscapedAmpersandInPathRelative() throws Exception { + doTest("/this%26that?a=b&c=d", true); + } + + public void testEscapedSpaceInQueryAbsolute() throws Exception { + doTest("/path?a=b&c=d%20e", false); + } + + public void testEscapedSpaceInQueryRelative() throws Exception { + doTest("/path?a=b&c=d%20e", true); + } + + public void testPlusInQueryAbsolute() throws Exception { + doTest("/path?a=b&c=d+e", false); + } + + public void testPlusInQueryRelative() throws Exception { + doTest("/path?a=b&c=d+e", true); + } +}