Skip to content

Commit

Permalink
Fix 4990, allow null values in S3 object metadata
Browse files Browse the repository at this point in the history
Change tests to check keys in any order instead of relying on keySets
implicit ordering.
  • Loading branch information
dropofwill committed May 16, 2024
1 parent 7499be3 commit 2182a44
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 8 deletions.
6 changes: 6 additions & 0 deletions .changes/next-release/bugfix-AmazonS3-d82f8b1.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"type": "bugfix",
"category": "Amazon S3",
"contributor": "dropofwill",
"description": "Fixes issue 4990 allowing null values in S3 object metadata"
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@

package software.amazon.awssdk.services.s3.internal.handlers;

import java.util.HashMap;
import java.util.Map;
import java.util.stream.Collectors;
import software.amazon.awssdk.annotations.SdkInternalApi;
import software.amazon.awssdk.core.SdkRequest;
import software.amazon.awssdk.core.interceptor.Context;
Expand Down Expand Up @@ -69,6 +69,6 @@ private CreateMultipartUploadRequest trimMetadataNames(CreateMultipartUploadRequ

private Map<String, String> trimKeys(Map<String, String> map) {
return map.entrySet().stream()
.collect(Collectors.toMap(e -> StringUtils.trim(e.getKey()), Map.Entry::getValue));
.collect(HashMap::new, (hm, entry) -> hm.put(StringUtils.trim(entry.getKey()), entry.getValue()), HashMap::putAll);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
Expand All @@ -37,15 +38,20 @@
public class ObjectMetadataInterceptorTest {
private static final ObjectMetadataInterceptor INTERCEPTOR = new ObjectMetadataInterceptor();



public static List<TestCase> testCases() {
List<String> listWithNulls = new ArrayList<>();
listWithNulls.add("\tc\r\n");
listWithNulls.add(null);

List<String> listWithNullsTrimmed = new ArrayList<>();
listWithNullsTrimmed.add("c");
listWithNullsTrimmed.add(null);
return asList(
tc(asList("a", "b", "c"), asList("a", "b", "c")),
tc(asList(" a ", "b", "c"), asList("a", "b", "c")),
tc(asList(" a", "\tb", "\tc"), asList("a", "b", "c")),
tc(asList("a\n", "\tb", "\tc\r\n"), asList("a", "b", "c"))

tc(asList("a\n", "\tb", "\tc\r\n"), asList("a", "b", "c")),
tc(listWithNulls, listWithNullsTrimmed)
);
}

Expand All @@ -68,7 +74,7 @@ public void modifyRequest_putObject_metadataKeysAreTrimmed(TestCase tc) {

PutObjectRequest modified = (PutObjectRequest) INTERCEPTOR.modifyRequest(ctx, attrs);

assertThat(modified.metadata().keySet()).containsExactlyElementsOf(tc.expectedKeys);
assertThat(modified.metadata().keySet()).containsOnlyOnceElementsOf(tc.expectedKeys);
}

@ParameterizedTest
Expand All @@ -90,7 +96,7 @@ public void modifyRequest_creatMultipartUpload_metadataKeysAreTrimmed(TestCase t

CreateMultipartUploadRequest modified = (CreateMultipartUploadRequest) INTERCEPTOR.modifyRequest(ctx, attrs);

assertThat(modified.metadata().keySet()).containsExactlyElementsOf(tc.expectedKeys);
assertThat(modified.metadata().keySet()).containsOnlyOnceElementsOf(tc.expectedKeys);
}

@Test
Expand Down

0 comments on commit 2182a44

Please sign in to comment.