HTTPCLIENT-730: Fixed rewriting of URIs containing escaped characters

Contributed by Sam Berlin <sberlin at gmail.com> and Oleg Kalnichevski 


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

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 <sberlin at gmail.com> and 
+  Oleg Kalnichevski <olegk at apache.org>
+ 
 * [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. 

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.
      */

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(
+                    uri = URLUtils.rewriteURI(uri, target);
-                            target.getSchemeName(), 
-                            null, 
-                            target.getHostName(), 
-                            target.getPort(), 
-                            uri.getPath(), 
-                            uri.getQuery(), 
-                            uri.getFragment());
                     request.setURI(uri);
                 }
             } else {
                 // Make sure the request URI is relative
                 if (uri.isAbsolute()) {
-                    uri = new URI(null, null, null, -1, 
+                    uri = URLUtils.rewriteURI(uri, null);
-                            uri.getPath(), 
-                            uri.getQuery(), 
-                            uri.getFragment());
                     request.setURI(uri);
                 }
             }

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(
+                URI absoluteRequestURI = URLUtils.rewriteURI(requestURI, target, true);
-                        target.getSchemeName(),
-                        null,
-                        target.getHostName(),
-                        target.getPort(),
-                        requestURI.getPath(),
-                        requestURI.getQuery(),
-                        null);
                 uri = absoluteRequestURI.resolve(uri); 
             } catch (URISyntaxException ex) {
                 throw new ProtocolException(ex.getMessage(), ex);

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
+ * <http://www.apache.org/>.
+ *
+ */
+
+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);
+    }
+}