hc-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ol...@apache.org
Subject svn commit: r1343355 - in /httpcomponents/httpclient/trunk: RELEASE_NOTES.txt httpclient/src/main/java/org/apache/http/client/utils/URIBuilder.java httpclient/src/test/java/org/apache/http/client/utils/TestURIBuilder.java
Date Mon, 28 May 2012 18:29:50 GMT
Author: olegk
Date: Mon May 28 18:29:50 2012
New Revision: 1343355

URL: http://svn.apache.org/viewvc?rev=1343355&view=rev
Log:
HTTPCLIENT-1192: URIBuilder encodes query parameters twice

Modified:
    httpcomponents/httpclient/trunk/RELEASE_NOTES.txt
    httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/client/utils/URIBuilder.java
    httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/client/utils/TestURIBuilder.java

Modified: httpcomponents/httpclient/trunk/RELEASE_NOTES.txt
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/RELEASE_NOTES.txt?rev=1343355&r1=1343354&r2=1343355&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/RELEASE_NOTES.txt (original)
+++ httpcomponents/httpclient/trunk/RELEASE_NOTES.txt Mon May 28 18:29:50 2012
@@ -1,6 +1,12 @@
 Changes since 4.2 
 -------------------
 
+* [HTTPCLIENT-1192] URIBuilder encodes query parameters twice.
+  Contributed by Oleg Kalnichevski <olegk at apache.org>
+
+* [HTTPCLIENT-1196] Fixed NPE in UrlEncodedFormEntity constructor thrown if charset is null.
+  Contributed by Oleg Kalnichevski <olegk at apache.org>
+
 * [HTTPCLIENT-1193] Fixed regression in the route tracking logic of the default connection
manager 
   causing cross-site redirect failures.
   Contributed by Oleg Kalnichevski <olegk at apache.org>

Modified: httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/client/utils/URIBuilder.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/client/utils/URIBuilder.java?rev=1343355&r1=1343354&r2=1343355&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/client/utils/URIBuilder.java
(original)
+++ httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/client/utils/URIBuilder.java
Mon May 28 18:29:50 2012
@@ -76,11 +76,22 @@ public class URIBuilder {
         return null;
     }
 
-    private String formatQuery(final List<NameValuePair> parameters, final Charset
charset) {
+    private String formatQuery(final List<NameValuePair> parameters) {
         if (parameters == null) {
             return null;
         }
-        return URLEncodedUtils.format(parameters, charset);
+        final StringBuilder result = new StringBuilder();
+        for (final NameValuePair parameter : parameters) {
+            if (result.length() > 0) {
+                result.append("&");
+            }
+            result.append(parameter.getName());
+            if (parameter.getValue() != null) {
+                result.append("=");
+                result.append(parameter.getValue());
+            }
+        }
+        return result.toString();
     }
 
     /**
@@ -91,11 +102,11 @@ public class URIBuilder {
             return new URI(this.scheme, this.schemeSpecificPart, this.fragment);
         } else if (this.authority != null) {
             return new URI(this.scheme, this.authority,
-                    this.path, formatQuery(this.queryParams, Consts.UTF_8), this.fragment);
+                    this.path, formatQuery(this.queryParams), this.fragment);
 
         } else {
             return new URI(this.scheme, this.userInfo, this.host, this.port,
-                    this.path, formatQuery(this.queryParams, Consts.UTF_8), this.fragment);
+                    this.path, formatQuery(this.queryParams), this.fragment);
         }
     }
 
@@ -111,11 +122,17 @@ public class URIBuilder {
         this.fragment = uri.getFragment();
     }
 
+    /**
+     * Sets URI scheme.
+     */
     public URIBuilder setScheme(final String scheme) {
         this.scheme = scheme;
         return this;
     }
 
+    /**
+     * Sets URI user info.
+     */
     public URIBuilder setUserInfo(final String userInfo) {
         this.userInfo = userInfo;
         this.schemeSpecificPart = null;
@@ -123,10 +140,16 @@ public class URIBuilder {
         return this;
     }
 
+    /**
+     * Sets URI user info as a combination of username and password.
+     */
     public URIBuilder setUserInfo(final String username, final String password) {
         return setUserInfo(username + ':' + password);
     }
 
+    /**
+     * Sets URI host.
+     */
     public URIBuilder setHost(final String host) {
         this.host = host;
         this.schemeSpecificPart = null;
@@ -134,6 +157,9 @@ public class URIBuilder {
         return this;
     }
 
+    /**
+     * Sets URI port.
+     */
     public URIBuilder setPort(final int port) {
         this.port = port < 0 ? -1 : port;
         this.schemeSpecificPart = null;
@@ -141,24 +167,37 @@ public class URIBuilder {
         return this;
     }
 
+    /**
+     * Sets URI path. The value is expected to be unescaped and may contain non ASCII characters.
+     */
     public URIBuilder setPath(final String path) {
         this.path = path;
         this.schemeSpecificPart = null;
         return this;
     }
 
+    /**
+     * Removes URI query.
+     */
     public URIBuilder removeQuery() {
         this.queryParams = null;
         this.schemeSpecificPart = null;
         return this;
     }
 
+    /**
+     * Sets URI query. The value is expected to be escaped and may NOT contain non ASCII
characters.
+     */
     public URIBuilder setQuery(final String query) {
         this.queryParams = parseQuery(query, Consts.UTF_8);
         this.schemeSpecificPart = null;
         return this;
     }
 
+    /**
+     * Adds parameter to URI query. The parameter name and value are expected to be unescaped

+     * and may contain non ASCII characters.
+     */
     public URIBuilder addParameter(final String param, final String value) {
         if (this.queryParams == null) {
             this.queryParams = new ArrayList<NameValuePair>();
@@ -168,6 +207,10 @@ public class URIBuilder {
         return this;
     }
 
+    /**
+     * Sets parameter of URI query overriding existing value if set. The parameter name and
value 
+     * are expected to be unescaped and may contain non ASCII characters.
+     */
     public URIBuilder setParameter(final String param, final String value) {
         if (this.queryParams == null) {
             this.queryParams = new ArrayList<NameValuePair>();
@@ -185,6 +228,9 @@ public class URIBuilder {
         return this;
     }
 
+    /**
+     * Sets URI fragment.
+     */
     public URIBuilder setFragment(final String fragment) {
         this.fragment = fragment;
         return this;

Modified: httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/client/utils/TestURIBuilder.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/client/utils/TestURIBuilder.java?rev=1343355&r1=1343354&r2=1343355&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/client/utils/TestURIBuilder.java
(original)
+++ httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/client/utils/TestURIBuilder.java
Mon May 28 18:29:50 2012
@@ -91,7 +91,7 @@ public class TestURIBuilder {
         URIBuilder uribuilder = new URIBuilder(uri).setParameter("param", "some other stuff")
             .setParameter("blah", "blah");
         URI result = uribuilder.build();
-        Assert.assertEquals(new URI("http://localhost:80/?param=some+other+stuff&blah=blah"),
result);
+        Assert.assertEquals(new URI("http://localhost:80/?param=some%20other%20stuff&blah=blah"),
result);
     }
 
     @Test
@@ -101,7 +101,28 @@ public class TestURIBuilder {
             .addParameter("blah", "blah");
         URI result = uribuilder.build();
         Assert.assertEquals(new URI("http://localhost:80/?param=stuff&blah&blah&"
+
-                "param=some+other+stuff&blah=blah"), result);
+                "param=some%20other%20stuff&blah=blah"), result);
     }
 
+    @Test
+    public void testQueryEncoding() throws Exception {
+        URI uri1 = new URI("https://somehost.com/stuff?client_id=1234567890" +
+        		"&redirect_uri=https://somehost.com/blah%20blah/");
+        URI uri2 = new URIBuilder("https://somehost.com/stuff")
+            .addParameter("client_id","1234567890")
+            .addParameter("redirect_uri","https://somehost.com/blah blah/").build();
+        Assert.assertEquals(uri1, uri2);
+    }
+    
+    @Test
+    public void testPathEncoding() throws Exception {
+        URI uri1 = new URI("https://somehost.com/some%20path%20with%20blanks/");
+        URI uri2 = new URIBuilder()
+            .setScheme("https")
+            .setHost("somehost.com")
+            .setPath("/some path with blanks/")
+            .build();
+        Assert.assertEquals(uri1, uri2);
+    }
+    
 }



Mime
View raw message