HDFS-4172. namenode does not URI-encode parameters when building URI for datanode request (Derek Dagit via bobby)

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1408401 13f79535-47bb-0310-9956-ffa450edef68

hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt

@@ -1985,6 +1985,9 @@ Release 0.23.5 - UNRELEASED
     HDFS-4090. getFileChecksum() result incompatible when called against
     zero-byte files. (Kihwal Lee via daryn)
 
+    HDFS-4172. namenode does not URI-encode parameters when building URI for
+    datanode request (Derek Dagit via bobby)
+
 Release 0.23.4 - UNRELEASED
 
   INCOMPATIBLE CHANGES

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/resources/BooleanParam.java

@@ -22,6 +22,12 @@ abstract class BooleanParam extends Param<Boolean, BooleanParam.Domain> {
   static final String TRUE = "true";
   static final String FALSE = "false";
 
+  /** @return the parameter value as a string */
+  @Override
+  public String getValueString() {
+    return value.toString();
+  }
+
   BooleanParam(final Domain domain, final Boolean value) {
     super(domain, value);
   }

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/resources/EnumSetParam.java

@@ -53,6 +53,11 @@ abstract class EnumSetParam<E extends Enum<E>> extends Param<EnumSet<E>, EnumSet
     return getName() + "=" + toString(value);
   }
 
+  /** @return the parameter value as a string */
+  @Override
+  public String getValueString() {
+    return toString(value);
+  }
   
   /** The domain of the parameter. */
   static final class Domain<E extends Enum<E>> extends Param.Domain<EnumSet<E>> {

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/resources/HttpOpParam.java

@@ -114,6 +114,12 @@ public abstract class HttpOpParam<E extends Enum<E> & HttpOpParam.Op>
     }
   }
 
+  /** @return the parameter value as a string */
+  @Override
+  public String getValueString() {
+    return value.toString();
+  }
+
   HttpOpParam(final Domain<E> domain, final E value) {
     super(domain, value);
   }

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/resources/InetSocketAddressParam.java

@@ -31,6 +31,12 @@ abstract class InetSocketAddressParam
     return getName() + "=" + Domain.toString(getValue());
   }
 
+  /** @return the parameter value as a string */
+  @Override
+  public String getValueString() {
+    return Domain.toString(getValue());
+  }
+
   /** The domain of the parameter. */
   static final class Domain extends Param.Domain<InetSocketAddress> {
     Domain(final String paramName) {

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/resources/IntegerParam.java

@@ -44,6 +44,12 @@ abstract class IntegerParam extends Param<Integer, IntegerParam.Domain> {
     return getName() + "=" + domain.toString(getValue());
   }
 
+  /** @return the parameter value as a string */
+  @Override
+  public String getValueString() {
+    return domain.toString(getValue());
+  }
+
   /** The domain of the parameter. */
   static final class Domain extends Param.Domain<Integer> {
     /** The radix of the number. */

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/resources/LongParam.java

@@ -43,6 +43,12 @@ abstract class LongParam extends Param<Long, LongParam.Domain> {
     return getName() + "=" + domain.toString(getValue());
   }
 
+  /** @return the parameter value as a string */
+  @Override
+  public String getValueString() {
+    return domain.toString(getValue());
+  }
+
   /** The domain of the parameter. */
   static final class Domain extends Param.Domain<Long> {
     /** The radix of the number. */

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/resources/Param.java

@@ -17,6 +17,8 @@
  */
 package org.apache.hadoop.hdfs.web.resources;
 
+import java.io.UnsupportedEncodingException;
+import java.net.URLEncoder;
 import java.util.Arrays;
 import java.util.Comparator;
 
@@ -32,16 +34,29 @@ public abstract class Param<T, D extends Param.Domain<T>> {
     }
   };
 
-  /** Convert the parameters to a sorted String. */
+  /** Convert the parameters to a sorted String.
+   *
+   * @param separator URI parameter separator character
+   * @param parameters parameters to encode into a string
+   * @return the encoded URI string
+   */
   public static String toSortedString(final String separator,
       final Param<?, ?>... parameters) {
     Arrays.sort(parameters, NAME_CMP);
     final StringBuilder b = new StringBuilder();
-    for(Param<?, ?> p : parameters) {
-      if (p.getValue() != null) {
-        b.append(separator).append(p);
+    try {
+      for(Param<?, ?> p : parameters) {
+        if (p.getValue() != null) {
+          b.append(separator).append(
+              URLEncoder.encode(p.getName(), "UTF-8")
+              + "="
+              + URLEncoder.encode(p.getValueString(), "UTF-8"));
+        }
       }
-    }
+  } catch (UnsupportedEncodingException e) {
+    // Sane systems know about UTF-8, so this should never happen.
+    throw new RuntimeException(e);
+  }
     return b.toString();
   }
 
@@ -60,6 +75,9 @@ public abstract class Param<T, D extends Param.Domain<T>> {
     return value;
   }
 
+  /** @return the parameter value as a string */
+  public abstract String getValueString();
+
   /** @return the parameter name. */
   public abstract String getName();
 
@@ -101,4 +119,4 @@ public abstract class Param<T, D extends Param.Domain<T>> {
       }
     }
   }
-}
+}

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/resources/ShortParam.java

@@ -44,6 +44,12 @@ abstract class ShortParam extends Param<Short, ShortParam.Domain> {
     return getName() + "=" + domain.toString(getValue());
   }
 
+  /** @return the parameter value as a string */
+  @Override
+  public final String getValueString() {
+    return domain.toString(getValue());
+  }
+
   /** The domain of the parameter. */
   static final class Domain extends Param.Domain<Short> {
     /** The radix of the number. */

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/resources/StringParam.java

@@ -25,6 +25,12 @@ abstract class StringParam extends Param<String, StringParam.Domain> {
     super(domain, domain.parse(str));
   }
 
+  /** @return the parameter value as a string */
+  @Override
+  public String getValueString() {
+    return value;
+  }
+
   /** The domain of the parameter. */
   static final class Domain extends Param.Domain<String> {
     /** The pattern defining the domain; null . */

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/resources/TestParam.java

@@ -224,4 +224,14 @@ public class TestParam {
       LOG.info("EXPECTED: " + e);
     }
   }
+
+  @Test
+  public void testToSortedStringEscapesURICharacters() {
+    final String sep = "&";
+    Param<?, ?> ampParam = new TokenArgumentParam("token&ampersand");
+    Param<?, ?> equalParam = new RenewerParam("renewer=equal");
+    final String expected = "&renewer=renewer%3Dequal&token=token%26ampersand";
+    final String actual = Param.toSortedString(sep, equalParam, ampParam);
+    Assert.assertEquals(expected, actual);
+  }
 }