You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by wa...@apache.org on 2014/08/12 19:46:31 UTC
svn commit: r1617542 - in
/hadoop/common/trunk/hadoop-common-project/hadoop-common: CHANGES.txt
src/main/java/org/apache/hadoop/util/GenericOptionsParser.java
src/test/java/org/apache/hadoop/util/TestGenericOptionsParser.java
Author: wang
Date: Tue Aug 12 17:46:31 2014
New Revision: 1617542
URL: http://svn.apache.org/r1617542
Log:
HADOOP-10820. Throw an exception in GenericOptionsParser when passed an empty Path. Contributed by Alex Holmes and Zhihai Xu.
Modified:
hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/GenericOptionsParser.java
hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestGenericOptionsParser.java
Modified: hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt?rev=1617542&r1=1617541&r2=1617542&view=diff
==============================================================================
--- hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt (original)
+++ hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt Tue Aug 12 17:46:31 2014
@@ -502,6 +502,9 @@ Release 2.6.0 - UNRELEASED
HADOOP-10835. Implement HTTP proxyuser support in HTTP authentication
client/server libraries. (tucu)
+ HADOOP-10820. Throw an exception in GenericOptionsParser when passed
+ an empty Path. (Alex Holmes and Zhihai Xu via wang)
+
OPTIMIZATIONS
BUG FIXES
Modified: hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/GenericOptionsParser.java
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/GenericOptionsParser.java?rev=1617542&r1=1617541&r2=1617542&view=diff
==============================================================================
--- hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/GenericOptionsParser.java (original)
+++ hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/GenericOptionsParser.java Tue Aug 12 17:46:31 2014
@@ -378,9 +378,15 @@ public class GenericOptionsParser {
if (files == null)
return null;
String[] fileArr = files.split(",");
+ if (fileArr.length == 0) {
+ throw new IllegalArgumentException("File name can't be empty string");
+ }
String[] finalArr = new String[fileArr.length];
for (int i =0; i < fileArr.length; i++) {
String tmp = fileArr[i];
+ if (tmp.isEmpty()) {
+ throw new IllegalArgumentException("File name can't be empty string");
+ }
String finalPath;
URI pathURI;
try {
Modified: hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestGenericOptionsParser.java
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestGenericOptionsParser.java?rev=1617542&r1=1617541&r2=1617542&view=diff
==============================================================================
--- hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestGenericOptionsParser.java (original)
+++ hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestGenericOptionsParser.java Tue Aug 12 17:46:31 2014
@@ -21,11 +21,14 @@ import java.io.File;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.net.URI;
+import java.util.ArrayList;
import java.util.Arrays;
+import java.util.List;
import java.util.Map;
import junit.framework.TestCase;
+import org.apache.commons.math3.util.Pair;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
@@ -34,12 +37,14 @@ import org.apache.hadoop.security.Creden
import org.apache.hadoop.security.UserGroupInformation;
import org.apache.hadoop.security.token.Token;
import org.apache.hadoop.security.token.delegation.AbstractDelegationTokenIdentifier;
+import org.apache.hadoop.test.GenericTestUtils;
import org.apache.commons.cli.Option;
import org.apache.commons.cli.OptionBuilder;
import org.apache.commons.cli.Options;
import org.junit.Assert;
import com.google.common.collect.Maps;
+import static org.junit.Assert.fail;
public class TestGenericOptionsParser extends TestCase {
File testDir;
@@ -93,6 +98,67 @@ public class TestGenericOptionsParser ex
}
/**
+ * Test the case where the libjars, files and archives arguments
+ * contains an empty token, which should create an IllegalArgumentException.
+ */
+ public void testEmptyFilenames() throws Exception {
+ List<Pair<String, String>> argsAndConfNames = new ArrayList<Pair<String, String>>();
+ argsAndConfNames.add(new Pair<String, String>("-libjars", "tmpjars"));
+ argsAndConfNames.add(new Pair<String, String>("-files", "tmpfiles"));
+ argsAndConfNames.add(new Pair<String, String>("-archives", "tmparchives"));
+ for (Pair<String, String> argAndConfName : argsAndConfNames) {
+ String arg = argAndConfName.getFirst();
+ String configName = argAndConfName.getSecond();
+
+ File tmpFileOne = new File(testDir, "tmpfile1");
+ Path tmpPathOne = new Path(tmpFileOne.toString());
+ File tmpFileTwo = new File(testDir, "tmpfile2");
+ Path tmpPathTwo = new Path(tmpFileTwo.toString());
+ localFs.create(tmpPathOne);
+ localFs.create(tmpPathTwo);
+ String[] args = new String[2];
+ args[0] = arg;
+ // create an empty path in between two valid files,
+ // which prior to HADOOP-10820 used to result in the
+ // working directory being added to "tmpjars" (or equivalent)
+ args[1] = String.format("%s,,%s",
+ tmpFileOne.toURI().toString(), tmpFileTwo.toURI().toString());
+ try {
+ new GenericOptionsParser(conf, args);
+ fail("Expected exception for empty filename");
+ } catch (IllegalArgumentException e) {
+ // expect to receive an IllegalArgumentException
+ GenericTestUtils.assertExceptionContains("File name can't be"
+ + " empty string", e);
+ }
+
+ // test zero file list length - it should create an exception
+ args[1] = ",,";
+ try {
+ new GenericOptionsParser(conf, args);
+ fail("Expected exception for zero file list length");
+ } catch (IllegalArgumentException e) {
+ // expect to receive an IllegalArgumentException
+ GenericTestUtils.assertExceptionContains("File name can't be"
+ + " empty string", e);
+ }
+
+ // test filename with space character
+ // it should create exception from parser in URI class
+ // due to URI syntax error
+ args[1] = String.format("%s, ,%s",
+ tmpFileOne.toURI().toString(), tmpFileTwo.toURI().toString());
+ try {
+ new GenericOptionsParser(conf, args);
+ fail("Expected exception for filename with space character");
+ } catch (IllegalArgumentException e) {
+ // expect to receive an IllegalArgumentException
+ GenericTestUtils.assertExceptionContains("URISyntaxException", e);
+ }
+ }
+ }
+
+ /**
* Test that options passed to the constructor are used.
*/
@SuppressWarnings("static-access")