Return-Path: 
 <commits-return-5066-apmail-hc-commits-archive=hc.apache.org@hc.apache.org>
X-Original-To: apmail-hc-commits-archive@www.apache.org
Delivered-To: apmail-hc-commits-archive@www.apache.org
Received: from mail.apache.org (hermes.apache.org [140.211.11.3])
	by minotaur.apache.org (Postfix) with SMTP id B5AE6C2B2
	for <apmail-hc-commits-archive@www.apache.org>;
 Mon, 28 May 2012 18:30:13 +0000 (UTC)
Received: (qmail 87286 invoked by uid 500); 28 May 2012 18:30:13 -0000
Delivered-To: apmail-hc-commits-archive@hc.apache.org
Received: (qmail 87230 invoked by uid 500); 28 May 2012 18:30:13 -0000
Mailing-List: contact commits-help@hc.apache.org; run by ezmlm
Precedence: bulk
List-Help: <mailto:commits-help@hc.apache.org>
List-Unsubscribe: <mailto:commits-unsubscribe@hc.apache.org>
List-Post: <mailto:commits@hc.apache.org>
List-Id: <commits.hc.apache.org>
Reply-To: "HttpComponents Project" <dev@hc.apache.org>
Delivered-To: mailing list commits@hc.apache.org
Received: (qmail 87219 invoked by uid 99); 28 May 2012 18:30:13 -0000
Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136)
    by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 28 May 2012 18:30:13 +0000
X-ASF-Spam-Status: No, hits=-2000.0 required=5.0
	tests=ALL_TRUSTED
X-Spam-Check-By: apache.org
Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4)
    by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 28 May 2012 18:30:11 +0000
Received: from eris.apache.org (localhost [127.0.0.1])
	by eris.apache.org (Postfix) with ESMTP id 25D1023888CD
	for <commits@hc.apache.org>; Mon, 28 May 2012 18:29:51 +0000 (UTC)
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
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 -0000
To: commits@hc.apache.org
From: olegk@apache.org
X-Mailer: svnmailer-1.0.8-patched
Message-Id: <20120528182951.25D1023888CD@eris.apache.org>
X-Virus-Checked: Checked by ClamAV on apache.org

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);
+    }
+    
 }