diff --git a/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/lib/wsrs/UserProvider.java b/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/lib/wsrs/UserProvider.java index 8310b00d9d4..4db42c21ac8 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/lib/wsrs/UserProvider.java +++ b/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 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 diff --git a/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/lib/wsrs/TestUserProvider.java b/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/lib/wsrs/TestUserProvider.java index 2e5c646f374..2bba4f090d0 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/lib/wsrs/TestUserProvider.java +++ b/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$")); + } + } diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt index df8262d81ff..7e832b59fa3 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/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 diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/resources/StringParam.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/resources/StringParam.java index 1c9fbe401da..f0631203278 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/resources/StringParam.java +++ b/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 { @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()); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/resources/UserParam.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/resources/UserParam.java index e50460306ae..ead8e54882b 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/resources/UserParam.java +++ b/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)); } /** diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/resources/TestParam.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/resources/TestParam.java index a6825f3ec83..0e06de319ad 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/resources/TestParam.java +++ b/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()); + } }