Skip to content

Commit b419c3a

Browse files
virajjasanisteveloughran
virajjasani
authored and
steveloughran
committed
HADOOP-18980. Invalid inputs for getTrimmedStringCollectionSplitByEquals (ADDENDUM) (#6546)
This is a followup to #6406: HADOOP-18980. S3A credential provider remapping: make extensible It adds extra validation of key-value pairs in a configuration option, with tests. Contributed by Viraj Jasani
1 parent 66b0edf commit b419c3a

File tree

3 files changed

+155
-8
lines changed

3 files changed

+155
-8
lines changed

‎hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/StringUtils.java

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import org.apache.commons.lang3.time.FastDateFormat;
4141
import org.apache.hadoop.classification.InterfaceAudience;
4242
import org.apache.hadoop.classification.InterfaceStability;
43+
import org.apache.hadoop.classification.VisibleForTesting;
4344
import org.apache.hadoop.fs.Path;
4445
import org.apache.hadoop.net.NetUtils;
4546
import org.apache.log4j.LogManager;
@@ -79,6 +80,18 @@ public class StringUtils {
7980
public static final Pattern ENV_VAR_PATTERN = Shell.WINDOWS ?
8081
WIN_ENV_VAR_PATTERN : SHELL_ENV_VAR_PATTERN;
8182

83+
/**
84+
* {@link #getTrimmedStringCollectionSplitByEquals(String)} throws
85+
* {@link IllegalArgumentException} with error message starting with this string
86+
* if the argument provided is not valid representation of non-empty key-value
87+
* pairs.
88+
* Value = {@value}
89+
*/
90+
@VisibleForTesting
91+
public static final String STRING_COLLECTION_SPLIT_EQUALS_INVALID_ARG =
92+
"Trimmed string split by equals does not correctly represent "
93+
+ "non-empty key-value pairs.";
94+
8295
/**
8396
* Make a string representation of the exception.
8497
* @param e The exception to stringify
@@ -494,10 +507,19 @@ public static Map<String, String> getTrimmedStringCollectionSplitByEquals(
494507
String[] trimmedList = getTrimmedStrings(str);
495508
Map<String, String> pairs = new HashMap<>();
496509
for (String s : trimmedList) {
497-
String[] splitByKeyVal = getTrimmedStringsSplitByEquals(s);
498-
if (splitByKeyVal.length == 2) {
499-
pairs.put(splitByKeyVal[0], splitByKeyVal[1]);
510+
if (s.isEmpty()) {
511+
continue;
500512
}
513+
String[] splitByKeyVal = getTrimmedStringsSplitByEquals(s);
514+
Preconditions.checkArgument(
515+
splitByKeyVal.length == 2,
516+
STRING_COLLECTION_SPLIT_EQUALS_INVALID_ARG + " Input: " + str);
517+
boolean emptyKey = org.apache.commons.lang3.StringUtils.isEmpty(splitByKeyVal[0]);
518+
boolean emptyVal = org.apache.commons.lang3.StringUtils.isEmpty(splitByKeyVal[1]);
519+
Preconditions.checkArgument(
520+
!emptyKey && !emptyVal,
521+
STRING_COLLECTION_SPLIT_EQUALS_INVALID_ARG + " Input: " + str);
522+
pairs.put(splitByKeyVal[0], splitByKeyVal[1]);
501523
}
502524
return pairs;
503525
}

‎hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestStringUtils.java

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@
1919
package org.apache.hadoop.util;
2020

2121
import java.util.Locale;
22+
23+
import static org.apache.hadoop.test.LambdaTestUtils.intercept;
24+
import static org.apache.hadoop.util.StringUtils.STRING_COLLECTION_SPLIT_EQUALS_INVALID_ARG;
2225
import static org.apache.hadoop.util.StringUtils.TraditionalBinaryPrefix.long2String;
2326
import static org.apache.hadoop.util.StringUtils.TraditionalBinaryPrefix.string2long;
2427
import static org.junit.Assert.assertArrayEquals;
@@ -515,7 +518,7 @@ public void testCreateStartupShutdownMessage() {
515518
}
516519

517520
@Test
518-
public void testStringCollectionSplitByEquals() {
521+
public void testStringCollectionSplitByEqualsSuccess() {
519522
Map<String, String> splitMap =
520523
StringUtils.getTrimmedStringCollectionSplitByEquals("");
521524
Assertions
@@ -566,6 +569,59 @@ public void testStringCollectionSplitByEquals() {
566569
.containsEntry("element.xyz.key5", "element.abc.val5")
567570
.containsEntry("element.xyz.key6", "element.abc.val6")
568571
.containsEntry("element.xyz.key7", "element.abc.val7");
572+
573+
splitMap = StringUtils.getTrimmedStringCollectionSplitByEquals(
574+
"element.first.key1 = element.first.val2 ,element.first.key1 =element.first.val1");
575+
Assertions
576+
.assertThat(splitMap)
577+
.describedAs("Map of key value pairs split by equals(=) and comma(,)")
578+
.hasSize(1)
579+
.containsEntry("element.first.key1", "element.first.val1");
580+
581+
splitMap = StringUtils.getTrimmedStringCollectionSplitByEquals(
582+
",,, , ,, ,element.first.key1 = element.first.val2 ,"
583+
+ "element.first.key1 = element.first.val1 , ,,, ,");
584+
Assertions
585+
.assertThat(splitMap)
586+
.describedAs("Map of key value pairs split by equals(=) and comma(,)")
587+
.hasSize(1)
588+
.containsEntry("element.first.key1", "element.first.val1");
589+
590+
splitMap = StringUtils.getTrimmedStringCollectionSplitByEquals(
591+
",, , , ,, ,");
592+
Assertions
593+
.assertThat(splitMap)
594+
.describedAs("Map of key value pairs split by equals(=) and comma(,)")
595+
.hasSize(0);
596+
597+
}
598+
599+
@Test
600+
public void testStringCollectionSplitByEqualsFailure() throws Exception {
601+
intercept(
602+
IllegalArgumentException.class,
603+
STRING_COLLECTION_SPLIT_EQUALS_INVALID_ARG,
604+
() -> StringUtils.getTrimmedStringCollectionSplitByEquals(" = element.abc.val1"));
605+
606+
intercept(
607+
IllegalArgumentException.class,
608+
STRING_COLLECTION_SPLIT_EQUALS_INVALID_ARG,
609+
() -> StringUtils.getTrimmedStringCollectionSplitByEquals("element.abc.key1="));
610+
611+
intercept(
612+
IllegalArgumentException.class,
613+
STRING_COLLECTION_SPLIT_EQUALS_INVALID_ARG,
614+
() -> StringUtils.getTrimmedStringCollectionSplitByEquals("="));
615+
616+
intercept(
617+
IllegalArgumentException.class,
618+
STRING_COLLECTION_SPLIT_EQUALS_INVALID_ARG,
619+
() -> StringUtils.getTrimmedStringCollectionSplitByEquals("== = = ="));
620+
621+
intercept(
622+
IllegalArgumentException.class,
623+
STRING_COLLECTION_SPLIT_EQUALS_INVALID_ARG,
624+
() -> StringUtils.getTrimmedStringCollectionSplitByEquals(",="));
569625
}
570626

571627
// Benchmark for StringUtils split

‎hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AAWSCredentialsProvider.java

Lines changed: 73 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@
7070
import static org.apache.hadoop.fs.s3a.test.PublicDatasetTestUtils.getExternalData;
7171
import static org.apache.hadoop.test.LambdaTestUtils.intercept;
7272
import static org.apache.hadoop.test.LambdaTestUtils.interceptFuture;
73+
import static org.apache.hadoop.util.StringUtils.STRING_COLLECTION_SPLIT_EQUALS_INVALID_ARG;
7374

7475
/**
7576
* Unit tests for {@link Constants#AWS_CREDENTIALS_PROVIDER} logic.
@@ -722,8 +723,8 @@ public void testV2ClassNotFound() throws Throwable {
722723
* Tests for the string utility that will be used by S3A credentials provider.
723724
*/
724725
@Test
725-
public void testStringCollectionSplitByEquals() {
726-
final Configuration configuration = new Configuration();
726+
public void testStringCollectionSplitByEqualsSuccess() {
727+
final Configuration configuration = new Configuration(false);
727728
configuration.set("custom_key", "");
728729
Map<String, String> splitMap =
729730
S3AUtils.getTrimmedStringCollectionSplitByEquals(
@@ -775,8 +776,7 @@ public void testStringCollectionSplitByEquals() {
775776
+ "element.abc.val5 ,\n \n \n "
776777
+ " element.xyz.key6 = element.abc.val6 \n , \n"
777778
+ "element.xyz.key7=element.abc.val7,\n");
778-
splitMap = S3AUtils.getTrimmedStringCollectionSplitByEquals(
779-
configuration, "custom_key");
779+
splitMap = S3AUtils.getTrimmedStringCollectionSplitByEquals(configuration, "custom_key");
780780

781781
Assertions
782782
.assertThat(splitMap)
@@ -790,6 +790,75 @@ public void testStringCollectionSplitByEquals() {
790790
.containsEntry("element.xyz.key5", "element.abc.val5")
791791
.containsEntry("element.xyz.key6", "element.abc.val6")
792792
.containsEntry("element.xyz.key7", "element.abc.val7");
793+
794+
configuration.set("custom_key",
795+
"element.first.key1 = element.first.val2 ,element.first.key1 =element.first.val1");
796+
splitMap =
797+
S3AUtils.getTrimmedStringCollectionSplitByEquals(
798+
configuration, "custom_key");
799+
Assertions
800+
.assertThat(splitMap)
801+
.describedAs("Map of key value pairs split by equals(=) and comma(,)")
802+
.hasSize(1)
803+
.containsEntry("element.first.key1", "element.first.val1");
804+
805+
configuration.set("custom_key",
806+
",,, , ,, ,element.first.key1 = element.first.val2 ,"
807+
+ "element.first.key1 = element.first.val1 , ,,, ,");
808+
splitMap = S3AUtils.getTrimmedStringCollectionSplitByEquals(
809+
configuration, "custom_key");
810+
Assertions
811+
.assertThat(splitMap)
812+
.describedAs("Map of key value pairs split by equals(=) and comma(,)")
813+
.hasSize(1)
814+
.containsEntry("element.first.key1", "element.first.val1");
815+
816+
configuration.set("custom_key", ",, , , ,, ,");
817+
splitMap = S3AUtils.getTrimmedStringCollectionSplitByEquals(
818+
configuration, "custom_key");
819+
Assertions
820+
.assertThat(splitMap)
821+
.describedAs("Map of key value pairs split by equals(=) and comma(,)")
822+
.hasSize(0);
823+
}
824+
825+
/**
826+
* Validates that the argument provided is invalid by intercepting the expected
827+
* Exception.
828+
*
829+
* @param propKey The property key to validate.
830+
* @throws Exception If any error occurs.
831+
*/
832+
private static void expectInvalidArgument(final String propKey) throws Exception {
833+
final Configuration configuration = new Configuration(false);
834+
configuration.set("custom_key", propKey);
835+
836+
intercept(
837+
IllegalArgumentException.class,
838+
STRING_COLLECTION_SPLIT_EQUALS_INVALID_ARG,
839+
() -> S3AUtils.getTrimmedStringCollectionSplitByEquals(
840+
configuration, "custom_key"));
841+
}
842+
843+
/**
844+
* Tests for the string utility that will be used by S3A credentials provider.
845+
*/
846+
@Test
847+
public void testStringCollectionSplitByEqualsFailure() throws Exception {
848+
expectInvalidArgument(" = element.abc.val1");
849+
expectInvalidArgument("=element.abc.val1");
850+
expectInvalidArgument("= element.abc.val1");
851+
expectInvalidArgument(" =element.abc.val1");
852+
expectInvalidArgument("element.abc.key1=");
853+
expectInvalidArgument("element.abc.key1= ");
854+
expectInvalidArgument("element.abc.key1 =");
855+
expectInvalidArgument("element.abc.key1 = ");
856+
expectInvalidArgument("=");
857+
expectInvalidArgument(" =");
858+
expectInvalidArgument("= ");
859+
expectInvalidArgument(" = ");
860+
expectInvalidArgument("== = = =");
861+
expectInvalidArgument(", = ");
793862
}
794863

795864
/**

0 commit comments

Comments
 (0)