HDFS-4171. WebHDFS and HttpFs should accept only valid Unix user names. (tucu)

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/branch-2@1410828 13f79535-47bb-0310-9956-ffa450edef68

hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/lib/wsrs/UserProvider.java

@@ -31,6 +31,7 @@ import javax.ws.rs.core.Context;
 import javax.ws.rs.ext.Provider;
 import java.lang.reflect.Type;
 import java.security.Principal;
+import java.text.MessageFormat;
 import java.util.regex.Pattern;
 
 @Provider
@@ -40,13 +41,26 @@ public class UserProvider extends AbstractHttpContextInjectable<Principal> imple
 
   public static final String USER_NAME_PARAM = "user.name";
 
-  public static final Pattern USER_PATTERN = Pattern.compile("[_a-zA-Z0-9]+");
+  public static final Pattern USER_PATTERN = Pattern.compile("^[A-Za-z_][A-Za-z0-9._-]*[$]?$");
 
-  private static class UserParam extends StringParam {
+  static class UserParam extends StringParam {
 
     public UserParam(String user) {
       super(USER_NAME_PARAM, user, USER_PATTERN);
     }
+
+    @Override
+    public String parseParam(String str) {
+      if (str != null) {
+        int len = str.length();
+        if (len < 1 || len > 31) {
+          throw new IllegalArgumentException(MessageFormat.format(
+            "Parameter [{0}], invalid value [{1}], it's length must be between 1 and 31",
+            getName(), str));
+        }
+      }
+      return super.parseParam(str);
+    }
   }
 
   @Override

hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/lib/wsrs/TestUserProvider.java

@@ -19,13 +19,18 @@
 package org.apache.hadoop.lib.wsrs;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 
 import java.security.Principal;
 
 import javax.ws.rs.core.MultivaluedMap;
 
+import org.apache.hadoop.test.TestException;
+import org.apache.hadoop.test.TestExceptionHelper;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.MethodRule;
 import org.mockito.Mockito;
 import org.slf4j.MDC;
 
@@ -35,6 +40,9 @@ import com.sun.jersey.core.spi.component.ComponentScope;
 
 public class TestUserProvider {
 
+  @Rule
+  public MethodRule exceptionHelper = new TestExceptionHelper();
+
   @Test
   @SuppressWarnings("unchecked")
   public void noUser() {
@@ -92,4 +100,51 @@ public class TestUserProvider {
     assertEquals(up.getInjectable(null, null, Principal.class), up);
     assertNull(up.getInjectable(null, null, String.class));
   }
+
+  @Test
+  @TestException(exception = IllegalArgumentException.class)
+  public void userNameEmpty() {
+    UserProvider.UserParam userParam = new UserProvider.UserParam("username");
+    userParam.parseParam("");
+  }
+
+  @Test
+  @TestException(exception = IllegalArgumentException.class)
+  public void userNameTooLong() {
+    UserProvider.UserParam userParam = new UserProvider.UserParam("username");
+    userParam.parseParam("a123456789012345678901234567890x");
+  }
+
+  @Test
+  @TestException(exception = IllegalArgumentException.class)
+  public void userNameInvalidStart() {
+    UserProvider.UserParam userParam = new UserProvider.UserParam("username");
+    userParam.parseParam("1x");
+  }
+
+  @Test
+  @TestException(exception = IllegalArgumentException.class)
+  public void userNameInvalidDollarSign() {
+    UserProvider.UserParam userParam = new UserProvider.UserParam("username");
+    userParam.parseParam("1$x");
+  }
+
+  @Test
+  public void userNameMinLength() {
+    UserProvider.UserParam userParam = new UserProvider.UserParam("username");
+    assertNotNull(userParam.parseParam("a"));
+  }
+
+  @Test
+  public void userNameMaxLength() {
+    UserProvider.UserParam userParam = new UserProvider.UserParam("username");
+    assertNotNull(userParam.parseParam("a123456789012345678901234567890"));
+  }
+
+  @Test
+  public void userNameValidDollarSign() {
+    UserProvider.UserParam userParam = new UserProvider.UserParam("username");
+    assertNotNull(userParam.parseParam("a$"));
+  }
+
 }

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

@@ -253,6 +253,9 @@ Release 2.0.3-alpha - Unreleased
     HDFS-4156. Seeking to a negative position should throw an IOE.
     (Eli Reisman via eli)
 
+    HDFS-4171. WebHDFS and HttpFs should accept only valid Unix user 
+    names. (tucu)
+
 Release 2.0.2-alpha - 2012-09-07 
 
   INCOMPATIBLE CHANGES

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

@@ -48,7 +48,7 @@ abstract class StringParam extends Param<String, StringParam.Domain> {
 
     @Override
     final String parse(final String str) {
-      if (pattern != null) {
+      if (str != null && pattern != null) {
         if (!pattern.matcher(str).matches()) {
           throw new IllegalArgumentException("Invalid value: \"" + str
               + "\" does not belong to the domain " + getDomain());

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

@@ -19,6 +19,9 @@ package org.apache.hadoop.hdfs.web.resources;
 
 import org.apache.hadoop.security.UserGroupInformation;
 
+import java.text.MessageFormat;
+import java.util.regex.Pattern;
+
 /** User parameter. */
 public class UserParam extends StringParam {
   /** Parameter name. */
@@ -26,14 +29,29 @@ public class UserParam extends StringParam {
   /** Default parameter value. */
   public static final String DEFAULT = "";
 
-  private static final Domain DOMAIN = new Domain(NAME, null);
+  private static final Domain DOMAIN = new Domain(NAME,
+    Pattern.compile("^[A-Za-z_][A-Za-z0-9._-]*[$]?$"));
+
+  private static String validateLength(String str) {
+    if (str == null) {
+      throw new IllegalArgumentException(
+        MessageFormat.format("Parameter [{0}], cannot be NULL", NAME));
+    }
+    int len = str.length();
+    if (len < 1 || len > 31) {
+      throw new IllegalArgumentException(MessageFormat.format(
+        "Parameter [{0}], invalid value [{1}], it's length must be between 1 and 31",
+        NAME, str));
+    }
+    return str;
+  }
 
   /**
    * Constructor.
    * @param str a string representation of the parameter value.
    */
   public UserParam(final String str) {
-    super(DOMAIN, str == null || str.equals(DEFAULT)? null: str);
+    super(DOMAIN, str == null || str.equals(DEFAULT)? null : validateLength(str));
   }
 
   /**

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

@@ -26,6 +26,9 @@ import org.apache.hadoop.hdfs.DFSConfigKeys;
 import org.junit.Assert;
 import org.junit.Test;
 
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+
 public class TestParam {
   public static final Log LOG = LogFactory.getLog(TestParam.class);
 
@@ -234,4 +237,43 @@ public class TestParam {
     final String actual = Param.toSortedString(sep, equalParam, ampParam);
     Assert.assertEquals(expected, actual);
   }
+
+  @Test
+  public void userNameEmpty() {
+    UserParam userParam = new UserParam("");
+    assertNull(userParam.getValue());
+  }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void userNameTooLong() {
+    new UserParam("a123456789012345678901234567890x");
+  }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void userNameInvalidStart() {
+    new UserParam("1x");
+  }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void userNameInvalidDollarSign() {
+    new UserParam("1$x");
+  }
+
+  @Test
+  public void userNameMinLength() {
+    UserParam userParam = new UserParam("a");
+    assertNotNull(userParam.getValue());
+  }
+
+  @Test
+  public void userNameMaxLength() {
+    UserParam userParam = new UserParam("a123456789012345678901234567890");
+    assertNotNull(userParam.getValue());
+  }
+
+  @Test
+  public void userNameValidDollarSign() {
+    UserParam userParam = new UserParam("a$");
+    assertNotNull(userParam.getValue());
+  }
 }