Fixed a bug where null values are not handled properly when S3EventNotification.fromJson is invoked (#5770)

zoewangg · web-flow · commit 5043265abf3d · 2025-01-04T02:13:54.000Z
diff --git a/.changes/next-release/bugfix-S3EventNotifications-86f3901.json b/.changes/next-release/bugfix-S3EventNotifications-86f3901.json
@@ -0,0 +1,6 @@
+{
+    "type": "bugfix",
+    "category": "S3 Event Notifications",
+    "contributor": "",
+    "description": "Fixed a bug where null values are not handled properly when `S3EventNotification#fromJson` is invoked, throwing `IllegalArgumentException`. See [#5730](https://github.com/aws/aws-sdk-java-v2/issues/5730)"
+}
diff --git a/services-custom/s3-event-notifications/src/main/java/software/amazon/awssdk/eventnotifications/s3/internal/DefaultS3EventNotificationReader.java b/services-custom/s3-event-notifications/src/main/java/software/amazon/awssdk/eventnotifications/s3/internal/DefaultS3EventNotificationReader.java
@@ -70,7 +70,7 @@ private S3EventNotification readEvent(JsonNode jsonNode) {
     }
 
     private List<S3EventNotificationRecord> readRecords(JsonNode node) {
-        if (node == null || node.isNull()) {
+        if (isNull(node)) {
             return null;
         }
         List<JsonNode> recordArray = expectArrayOrNull(node, "Records");
@@ -274,23 +274,27 @@ private String expectStringOrNull(Map<String, JsonNode> node, String name) {
     }
 
     private String expectStringOrNull(JsonNode node, String name) {
-        if (node == null) {
+        if (isNull(node)) {
             return null;
         }
         Validate.isTrue(node.isString(), "'%s' was not a string", name);
         return node.asString();
     }
 
+    private static boolean isNull(JsonNode node) {
+        return node == null || node.isNull();
+    }
+
     private List<JsonNode> expectArrayOrNull(JsonNode node, String name) {
-        if (node == null) {
+        if (isNull(node)) {
             return null;
         }
         Validate.isTrue(node.isArray(), "expected '%s' to be an array, but was not.", name);
         return node.asArray();
     }
 
     private Map<String, JsonNode> expectObjectOrNull(JsonNode node, String name) {
-        if (node == null) {
+        if (isNull(node)) {
             return null;
         }
         return expectObject(node, name);
@@ -302,7 +306,7 @@ private Map<String, JsonNode> expectObject(JsonNode node, String name) {
     }
 
     private Long expectLong(JsonNode node, String name) {
-        if (node == null) {
+        if (isNull(node)) {
             return null;
         }
         Validate.isTrue(node.isNumber(), "expected '%s' to be numeric, but was not", name);
diff --git a/services-custom/s3-event-notifications/src/test/java/software/amazon/awssdk/eventnotifications/s3/model/S3EventNotificationReaderTest.java b/services-custom/s3-event-notifications/src/test/java/software/amazon/awssdk/eventnotifications/s3/model/S3EventNotificationReaderTest.java
@@ -40,6 +40,75 @@
 
 class S3EventNotificationReaderTest {
 
+    @Test
+    void fromJson_containsNullValues_shouldSucceed() {
+        S3EventNotification eventNotification = new S3EventNotification(
+            Collections.singletonList(new S3EventNotificationRecord(
+                "us-west-2",
+                "ObjectCreated:Put",
+                "aws:s3",
+                "1970-01-01T00:00:00.000Z",
+                "2.1",
+                null,
+                new ResponseElements(
+                    null, null),
+                new S3(
+                    "testConfigRule",
+                    new S3Bucket(
+                        "mybucket",
+                        new UserIdentity("A3NL1KOZZKExample"),
+                        "arn:aws:s3:::mybucket"),
+                    new S3Object(
+                        "HappyFace.jpg",
+                        null,
+                        "d41d8cd98f00b204e9800998ecf8427e",
+                        null,
+                        "0055AED6DCD90281E5"),
+                    "1.0"
+                ),
+                new UserIdentity("AIDAJDPLRKLG7UEXAMPLE"),
+                null, null, null, null)
+            ));
+        String json = eventNotification.toJsonPretty();
+        assertThat(json).isEqualTo("{\n"
+                                   + "  \"Records\" : [ {\n"
+                                   + "    \"eventVersion\" : \"2.1\",\n"
+                                   + "    \"eventSource\" : \"aws:s3\",\n"
+                                   + "    \"awsRegion\" : \"us-west-2\",\n"
+                                   + "    \"eventTime\" : \"1970-01-01T00:00:00Z\",\n"
+                                   + "    \"eventName\" : \"ObjectCreated:Put\",\n"
+                                   + "    \"userIdentity\" : {\n"
+                                   + "      \"principalId\" : \"AIDAJDPLRKLG7UEXAMPLE\"\n"
+                                   + "    },\n"
+                                   + "    \"requestParameters\" : null,\n"
+                                   + "    \"responseElements\" : {\n"
+                                   + "      \"x-amz-request-id\" : null,\n"
+                                   + "      \"x-amz-id-2\" : null\n"
+                                   + "    },\n"
+                                   + "    \"s3\" : {\n"
+                                   + "      \"s3SchemaVersion\" : \"1.0\",\n"
+                                   + "      \"configurationId\" : \"testConfigRule\",\n"
+                                   + "      \"bucket\" : {\n"
+                                   + "        \"name\" : \"mybucket\",\n"
+                                   + "        \"ownerIdentity\" : {\n"
+                                   + "          \"principalId\" : \"A3NL1KOZZKExample\"\n"
+                                   + "        },\n"
+                                   + "        \"arn\" : \"arn:aws:s3:::mybucket\"\n"
+                                   + "      },\n"
+                                   + "      \"object\" : {\n"
+                                   + "        \"key\" : \"HappyFace.jpg\",\n"
+                                   + "        \"size\" : null,\n"
+                                   + "        \"eTag\" : \"d41d8cd98f00b204e9800998ecf8427e\",\n"
+                                   + "        \"versionId\" : null,\n"
+                                   + "        \"sequencer\" : \"0055AED6DCD90281E5\"\n"
+                                   + "      }\n"
+                                   + "    }\n"
+                                   + "  } ]\n"
+                                   + "}");
+        S3EventNotification actual = S3EventNotification.fromJson(json);
+        assertThat(actual).isEqualTo(eventNotification);
+    }
+
     @Test
     void givenEventWithoutOptionalFields_whenReadingJson_expectOnlyRequiredFields() {
         String eventJson = "{  "

Original file line number	Diff line number	Diff line change
`@@ -70,7 +70,7 @@ private S3EventNotification readEvent(JsonNode jsonNode) {`
`70`	`70`	`}`
`71`	`71`
`72`	`72`	`private List<S3EventNotificationRecord> readRecords(JsonNode node) {`
`73`		`- if (node == null \|\| node.isNull()) {`
	`73`	`+ if (isNull(node)) {`
`74`	`74`	`return null;`
`75`	`75`	`}`
`76`	`76`	`List<JsonNode> recordArray = expectArrayOrNull(node, "Records");`
`@@ -274,23 +274,27 @@ private String expectStringOrNull(Map<String, JsonNode> node, String name) {`
`274`	`274`	`}`
`275`	`275`
`276`	`276`	`private String expectStringOrNull(JsonNode node, String name) {`
`277`		`- if (node == null) {`
	`277`	`+ if (isNull(node)) {`
`278`	`278`	`return null;`
`279`	`279`	`}`
`280`	`280`	`Validate.isTrue(node.isString(), "'%s' was not a string", name);`
`281`	`281`	`return node.asString();`
`282`	`282`	`}`
`283`	`283`
	`284`	`+ private static boolean isNull(JsonNode node) {`
	`285`	`+ return node == null \|\| node.isNull();`
	`286`	`+ }`
	`287`	`+`
`284`	`288`	`private List<JsonNode> expectArrayOrNull(JsonNode node, String name) {`
`285`		`- if (node == null) {`
	`289`	`+ if (isNull(node)) {`
`286`	`290`	`return null;`
`287`	`291`	`}`
`288`	`292`	`Validate.isTrue(node.isArray(), "expected '%s' to be an array, but was not.", name);`
`289`	`293`	`return node.asArray();`
`290`	`294`	`}`
`291`	`295`
`292`	`296`	`private Map<String, JsonNode> expectObjectOrNull(JsonNode node, String name) {`
`293`		`- if (node == null) {`
	`297`	`+ if (isNull(node)) {`
`294`	`298`	`return null;`
`295`	`299`	`}`
`296`	`300`	`return expectObject(node, name);`
`@@ -302,7 +306,7 @@ private Map<String, JsonNode> expectObject(JsonNode node, String name) {`
`302`	`306`	`}`
`303`	`307`
`304`	`308`	`private Long expectLong(JsonNode node, String name) {`
`305`		`- if (node == null) {`
	`309`	`+ if (isNull(node)) {`
`306`	`310`	`return null;`
`307`	`311`	`}`
`308`	`312`	`Validate.isTrue(node.isNumber(), "expected '%s' to be numeric, but was not", name);`