use %20 to escpae spaces in URI templates (#973)

Author: elharo

Diff for: google-http-client/src/main/java/com/google/api/client/http/UriTemplate.java

@@ -55,7 +55,7 @@
  */
 public class UriTemplate {
 
-  static final Map<Character, CompositeOutput> COMPOSITE_PREFIXES =
+  private static final Map<Character, CompositeOutput> COMPOSITE_PREFIXES =
       new HashMap<Character, CompositeOutput>();
 
   static {
@@ -98,14 +98,14 @@ private enum CompositeOutput {
     private final boolean reservedExpansion;
 
     /**
-     * @param propertyPrefix The prefix of a parameter or {@code null} for none. In {+var} the
+     * @param propertyPrefix the prefix of a parameter or {@code null} for none. In {+var} the
      *     prefix is '+'
-     * @param outputPrefix The string that should be prefixed to the expanded template.
-     * @param explodeJoiner The delimiter used to join composite values.
-     * @param requiresVarAssignment Denotes whether or not the expanded template should contain an
-     *     assignment with the variable.
-     * @param reservedExpansion Reserved expansion allows pct-encoded triplets and characters in the
-     *     reserved set.
+     * @param outputPrefix the string that should be prefixed to the expanded template.
+     * @param explodeJoiner the delimiter used to join composite values.
+     * @param requiresVarAssignment denotes whether or not the expanded template should contain an
+     *     assignment with the variable
+     * @param reservedExpansion reserved expansion allows percent-encoded triplets and characters in the
+     *     reserved set
      */
     CompositeOutput(
         Character propertyPrefix,
@@ -149,26 +149,22 @@ int getVarNameStartIndex() {
     }
 
     /**
-     * Encodes the specified value. If reserved expansion is turned on then pct-encoded triplets and
+     * Encodes the specified value. If reserved expansion is turned on, then percent-encoded triplets and
      * characters are allowed in the reserved set.
      *
-     * @param value The string to be encoded.
-     * @return The encoded string.
+     * @param value the string to be encoded
+     * @return the encoded string
      */
-    String getEncodedValue(String value) {
+    private String getEncodedValue(String value) {
       String encodedValue;
       if (reservedExpansion) {
-        // Reserved expansion allows pct-encoded triplets and characters in the reserved set.
+        // Reserved expansion allows percent-encoded triplets and characters in the reserved set.
         encodedValue = CharEscapers.escapeUriPathWithoutReserved(value);
       } else {
-        encodedValue = CharEscapers.escapeUri(value);
+        encodedValue = CharEscapers.escapeUriConformant(value);
       }
       return encodedValue;
     }
-
-    boolean getReservedExpansion() {
-      return reservedExpansion;
-    }
   }
 
   static CompositeOutput getCompositeOutput(String propertyName) {
@@ -334,12 +330,12 @@ private static String getSimpleValue(String name, String value, CompositeOutput
    * Expand the template of a composite list property. Eg: If d := ["red", "green", "blue"] then
    * {/d*} is expanded to "/red/green/blue"
    *
-   * @param varName The name of the variable the value corresponds to. Eg: "d"
-   * @param iterator The iterator over list values. Eg: ["red", "green", "blue"]
-   * @param containsExplodeModifier Set to true if the template contains the explode modifier "*"
-   * @param compositeOutput An instance of CompositeOutput. Contains information on how the
+   * @param varName the name of the variable the value corresponds to. E.g. "d"
+   * @param iterator the iterator over list values. E.g. ["red", "green", "blue"]
+   * @param containsExplodeModifiersSet to true if the template contains the explode modifier "*"
+   * @param compositeOutput an instance of CompositeOutput. Contains information on how the
    *     expansion should be done
-   * @return The expanded list template
+   * @return the expanded list template
    * @throws IllegalArgumentException if the required list path parameter is empty
    */
   private static String getListPropertyValue(
@@ -378,12 +374,11 @@ private static String getListPropertyValue(
    * Expand the template of a composite map property. Eg: If d := [("semi", ";"),("dot",
    * "."),("comma", ",")] then {/d*} is expanded to "/semi=%3B/dot=./comma=%2C"
    *
-   * @param varName The name of the variable the value corresponds to. Eg: "d"
-   * @param map The map property value. Eg: [("semi", ";"),("dot", "."),("comma", ",")]
+   * @param varName the name of the variable the value corresponds to. Eg: "d"
+   * @param map the map property value. Eg: [("semi", ";"),("dot", "."),("comma", ",")]
    * @param containsExplodeModifier Set to true if the template contains the explode modifier "*"
-   * @param compositeOutput An instance of CompositeOutput. Contains information on how the
-   *     expansion should be done
-   * @return The expanded map template
+   * @param compositeOutput contains information on how the expansion should be done
+   * @return the expanded map template
    * @throws IllegalArgumentException if the required list path parameter is map
    */
   private static String getMapPropertyValue(

Diff for: google-http-client/src/main/java/com/google/api/client/util/escape/CharEscapers.java

@@ -29,6 +29,9 @@ public final class CharEscapers {
   private static final Escaper APPLICATION_X_WWW_FORM_URLENCODED =
       new PercentEscaper(PercentEscaper.SAFECHARS_URLENCODER, true);
 
+  private static final Escaper URI_ESCAPER =
+      new PercentEscaper(PercentEscaper.SAFECHARS_URLENCODER, false);
+
   private static final Escaper URI_PATH_ESCAPER =
       new PercentEscaper(PercentEscaper.SAFEPATHCHARS_URLENCODER);
 
@@ -42,8 +45,13 @@ public final class CharEscapers {
       new PercentEscaper(PercentEscaper.SAFEQUERYSTRINGCHARS_URLENCODER);
 
   /**
-   * Escapes the string value so it can be safely included in URIs. For details on escaping URIs,
-   * see <a href="http://tools.ietf.org/html/rfc3986#section-2.4">RFC 3986 - section 2.4</a>.
+   * Escapes the string value so it can be safely included in application/x-www-form-urlencoded
+   * data. This is not appropriate for generic URI escaping. In particular it encodes
+   * the space character as a plus #stead of percent escaping it, in 
+   * contravention of the URI specification.
+   * For details on application/x-www-form-urlencoded encoding see the 
+   * see <a href="https://www.w3.org/TR/html401/interact/forms.html#h-17.13.4.1">HTML 4
+   * specification, section 17.13.4.1</a>.
    *
    * <p>When encoding a String, the following rules apply:
    *
@@ -68,9 +76,36 @@ public final class CharEscapers {
    *   <li>{@link java.net.URLEncoder#encode(String, String)} with the encoding name "UTF-8"
    * </ul>
    */
+  @Deprecated
   public static String escapeUri(String value) {
     return APPLICATION_X_WWW_FORM_URLENCODED.escape(value);
   }
+  
+    /**
+   * Escapes the string value so it can be safely included in any part of a URI.
+   * For details on escaping URIs,
+   * see <a href="http://tools.ietf.org/html/rfc3986#section-2.4">RFC 3986 - section 2.4</a>.
+   *
+   * <p>When encoding a String, the following rules apply:
+   *
+   * <ul>
+   *   <li>The alphanumeric characters "a" through "z", "A" through "Z" and "0" through "9" remain
+   *       the same.
+   *   <li>The special characters ".", "-", "*", and "_" remain the same.
+   *   <li>The space character " " is converted into "%20".
+   *   <li>All other characters are converted into one or more bytes using UTF-8 encoding and each
+   *       byte is then represented by the 3-character string "%XY", where "XY" is the two-digit,
+   *       uppercase, hexadecimal representation of the byte value.
+   * </ul>
+   *
+   * <p><b>Note</b>: Unlike other escapers, URI escapers produce uppercase hexadecimal sequences.
+   * From <a href="http://tools.ietf.org/html/rfc3986">RFC 3986</a>:<br>
+   * <i>"URI producers and normalizers should use uppercase hexadecimal digits for all
+   * percent-encodings."</i>
+   */
+  public static String escapeUriConformant(String value) {
+    return URI_ESCAPER.escape(value);
+  }
 
   /**
    * Decodes application/x-www-form-urlencoded strings. The UTF-8 character set determines
@@ -144,7 +179,7 @@ public static String escapeUriPath(String value) {
 
   /**
    * Escapes a URI path but retains all reserved characters, including all general delimiters. That
-   * is the same as {@link #escapeUriPath(String)} except that it keeps '?', '+', and '/' unescaped.
+   * is the same as {@link #escapeUriPath(String)} except that it does not escape '?', '+', and '/'.
    */
   public static String escapeUriPathWithoutReserved(String value) {
     return URI_RESERVED_ESCAPER.escape(value);

Diff for: google-http-client/src/test/java/com/google/api/client/http/UriTemplateTest.java

@@ -58,13 +58,19 @@ public void testExpandTemplates_basic() {
     assertTrue(requestMap.containsKey("unused"));
   }
 
-  public void testExpanTemplates_basicEncodeValue() {
+  public void testExpandTemplates_basicEncodeValue() {
     SortedMap<String, Object> requestMap = Maps.newTreeMap();
     requestMap.put("abc", "xyz;def");
     assertEquals(";abc=xyz%3Bdef", UriTemplate.expand("{;abc}", requestMap, false));
     assertEquals("xyz;def", UriTemplate.expand("{+abc}", requestMap, false));
   }
 
+  public void testExpandTemplates_encodeSpace() {
+    SortedMap<String, Object> requestMap = Maps.newTreeMap();
+    requestMap.put("abc", "xyz def");
+    assertEquals(";abc=xyz%20def", UriTemplate.expand("{;abc}", requestMap, false));
+  }
+
   public void testExpandTemplates_noExpansionsWithQueryParams() {
     SortedMap<String, Object> requestMap = Maps.newTreeMap();
     requestMap.put("abc", "xyz");