Trim header values and update unit tests

Author: Aditya Deshpande

hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/impl/AWSClientConfig.java

@@ -26,6 +26,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.TimeUnit;
+import java.util.stream.Collectors;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -442,8 +443,15 @@ private static void initRequestHeaders(Configuration conf,
       Map<String, String> awsClientCustomHeadersMap =
               S3AUtils.getTrimmedStringCollectionSplitByEquals(conf, configKey);
       awsClientCustomHeadersMap.forEach((header, valueString) -> {
-        List<String> headerValues = Arrays.asList(valueString.split(";"));
-        clientConfig.putHeader(header, headerValues);
+        List<String> headerValues = Arrays.stream(valueString.split(";"))
+                        .map(String::trim)
+                        .filter(v -> !v.isEmpty())
+                        .collect(Collectors.toList());
+        if (!headerValues.isEmpty()) {
+          clientConfig.putHeader(header, headerValues);
+        } else {
+          LOG.warn("Ignoring header '{}' for {} client because no values were provided", header, awsServiceIdentifier);
+        }
       });
       LOG.debug("headers for {} client = {}", awsServiceIdentifier, clientConfig.headers());
     }

hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/impl/TestAwsClientConfig.java

@@ -216,7 +216,8 @@ private void setOptionsToValue(String value, Configuration conf, String... keys)
   @Test
   public void testInitRequestHeadersForSTS() throws IOException {
     final Configuration conf = new Configuration();
-    conf.set(CUSTOM_HEADERS_STS, "foo=bar;baz,qux=quux");
+    conf.set(CUSTOM_HEADERS_STS, "header1=value1;value2,header2=value3");
+
     Assertions.assertThat(conf.get(CUSTOM_HEADERS_S3))
             .describedAs("Custom client headers for s3 %s", CUSTOM_HEADERS_S3)
             .isNull();
@@ -225,18 +226,21 @@ public void testInitRequestHeadersForSTS() throws IOException {
             .headers().size())
         .describedAs("Count of S3 client headers")
         .isEqualTo(0);
+
     Assertions.assertThat(createClientConfigBuilder(conf, AWS_SERVICE_IDENTIFIER_STS)
             .headers().size())
         .describedAs("Count of STS client headers")
         .isEqualTo(2);
+
     Assertions.assertThat(createClientConfigBuilder(conf, AWS_SERVICE_IDENTIFIER_STS)
-            .headers().get("foo"))
-        .describedAs("STS client 'foo' header value")
-        .isEqualTo(Lists.newArrayList("bar", "baz"));
+            .headers().get("header1"))
+        .describedAs("STS client 'header1' header value")
+        .isEqualTo(Lists.newArrayList("value1", "value2"));
+
     Assertions.assertThat(createClientConfigBuilder(conf, AWS_SERVICE_IDENTIFIER_STS)
-            .headers().get("qux"))
-        .describedAs("STS client 'qux' header value")
-        .isEqualTo(Lists.newArrayList("quux"));
+            .headers().get("header2"))
+        .describedAs("STS client 'header2' header value")
+        .isEqualTo(Lists.newArrayList("value3"));
   }
 
   /**
@@ -246,7 +250,8 @@ public void testInitRequestHeadersForSTS() throws IOException {
   @Test
   public void testInitRequestHeadersForS3() throws IOException {
     final Configuration conf = new Configuration();
-    conf.set(CUSTOM_HEADERS_S3, "foo=bar;baz,qux=quux");
+    conf.set(CUSTOM_HEADERS_S3, "header1=value1;value2,header2=value3");
+
     Assertions.assertThat(conf.get(CUSTOM_HEADERS_STS))
             .describedAs("Custom client headers for STS %s", CUSTOM_HEADERS_STS)
             .isNull();
@@ -255,17 +260,69 @@ public void testInitRequestHeadersForS3() throws IOException {
             .headers().size())
         .describedAs("Count of STS client headers")
         .isEqualTo(0);
+
     Assertions.assertThat(createClientConfigBuilder(conf, AWS_SERVICE_IDENTIFIER_S3)
             .headers().size())
         .describedAs("Count of S3 client headers")
         .isEqualTo(2);
+
+    Assertions.assertThat(createClientConfigBuilder(conf, AWS_SERVICE_IDENTIFIER_S3)
+            .headers().get("header1"))
+        .describedAs("S3 client 'header1' header value")
+        .isEqualTo(Lists.newArrayList("value1", "value2"));
+
+    Assertions.assertThat(createClientConfigBuilder(conf, AWS_SERVICE_IDENTIFIER_S3)
+            .headers().get("header2"))
+        .describedAs("S3 client 'header2' header value")
+        .isEqualTo(Lists.newArrayList("value3"));
+  }
+
+  /**
+   * if {@link org.apache.hadoop.fs.s3a.Constants#CUSTOM_HEADERS_S3} is set,
+   * verify that returned client configuration has desired headers set with whitespaces trimmed for headers and values.
+   */
+  @Test
+  public void testInitRequestHeadersForS3WithWhitespace() throws IOException {
+    final Configuration conf = new Configuration();
+    conf.set(CUSTOM_HEADERS_S3, "  header1 =  value1 ;  value2 ,   header2= value3  ");
+
+    Assertions.assertThat(conf.get(CUSTOM_HEADERS_STS))
+            .describedAs("Custom client headers for STS %s", CUSTOM_HEADERS_STS)
+            .isNull();
+
+    Assertions.assertThat(createClientConfigBuilder(conf, AWS_SERVICE_IDENTIFIER_STS)
+                    .headers().size())
+            .describedAs("Count of STS client headers")
+            .isEqualTo(0);
+
+    Assertions.assertThat(createClientConfigBuilder(conf, AWS_SERVICE_IDENTIFIER_S3)
+                    .headers().size())
+            .describedAs("Count of S3 client headers")
+            .isEqualTo(2);
+
     Assertions.assertThat(createClientConfigBuilder(conf, AWS_SERVICE_IDENTIFIER_S3)
-            .headers().get("foo"))
-        .describedAs("S3 client 'foo' header value")
-        .isEqualTo(Lists.newArrayList("bar", "baz"));
+                    .headers().get("header1"))
+            .describedAs("S3 client 'header1' header value")
+            .isEqualTo(Lists.newArrayList("value1", "value2"));
+
+    Assertions.assertThat(createClientConfigBuilder(conf, AWS_SERVICE_IDENTIFIER_S3)
+                    .headers().get("header2"))
+            .describedAs("S3 client 'header2' header value")
+            .isEqualTo(Lists.newArrayList("value3"));
+  }
+
+  /**
+   * if {@link org.apache.hadoop.fs.s3a.Constants#CUSTOM_HEADERS_S3} is set with duplicate values,
+   * verify that returned client configuration has desired headers with both values.
+   */
+  @Test
+  public void testInitRequestHeadersForS3WithDuplicateValues() throws IOException {
+    Configuration conf = new Configuration();
+    conf.set(CUSTOM_HEADERS_S3, "header1=duplicate;duplicate");
+
     Assertions.assertThat(createClientConfigBuilder(conf, AWS_SERVICE_IDENTIFIER_S3)
-            .headers().get("qux"))
-        .describedAs("S3 client 'qux' header value")
-        .isEqualTo(Lists.newArrayList("quux"));
+                    .headers().get("header1"))
+            .describedAs("S3 client 'header1' header value")
+            .isEqualTo(Lists.newArrayList("duplicate", "duplicate"));
   }
 }