Skip to content

Commit 569ae3a

Browse files
Correct JacksonEvent.merge() null-key handling and close InputSt… (opensearch-project#6775)
Fix JacksonEvent.merge() null-key bug and InputStream leak Harden XML parser in LastReleasedVersionProvider against XXE attacks. Signed-off-by: Siqi Ding <dingdd@amazon.com>
1 parent 8ea59d9 commit 569ae3a

3 files changed

Lines changed: 20 additions & 6 deletions

File tree

buildSrc/src/main/java/org/opensearch/dataprepper/gradle/compatibility/LastReleasedVersionProvider.java

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,16 +32,20 @@ public static String getLastReleasedMajorVersion(final Project project) {
3232
final String groupPath = group.replace('.', '/');
3333
final String metadataUrl = String.format("https://repo1.maven.org/maven2/%s/%s/maven-metadata.xml", groupPath, artifactId);
3434

35-
try {
36-
return getLastReleasedMajorVersion(new URL(metadataUrl).openStream(), currentVersion);
35+
try (InputStream stream = new URL(metadataUrl).openStream()) {
36+
return getLastReleasedMajorVersion(stream, currentVersion);
3737
} catch (final Exception e) {
3838
return null;
3939
}
4040
}
4141

4242
static String getLastReleasedMajorVersion(final InputStream metadataStream, final String currentVersion) throws Exception {
4343
final String majorVersion = currentVersion.split("\\.")[0];
44-
final Document doc = DocumentBuilderFactory.newInstance()
44+
DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
45+
dbf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
46+
dbf.setXIncludeAware(false);
47+
dbf.setExpandEntityReferences(false);
48+
final Document doc = dbf
4549
.newDocumentBuilder()
4650
.parse(metadataStream);
4751

data-prepper-api/src/main/java/org/opensearch/dataprepper/model/event/JacksonEvent.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -481,9 +481,8 @@ public void merge(final Event other, final Collection<String> keys) {
481481
}
482482

483483
for (final String key : keys) {
484-
final Object value = other.get(key, Object.class);
485-
if (value != null) {
486-
put(key, value);
484+
if (other.containsKey(key)) {
485+
put(key, other.get(key, Object.class));
487486
}
488487
}
489488
}

data-prepper-api/src/test/java/org/opensearch/dataprepper/model/event/JacksonEventTest.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -717,6 +717,17 @@ void merge_with_keys_overwrites_existing_values() {
717717
assertThat(event.containsKey("b"), equalTo(false));
718718
}
719719

720+
@Test
721+
void merge_with_keys_overwrites_existing_value_with_null() {
722+
event.put("a", "original");
723+
final String jsonString = "{\"a\": null}";
724+
Event otherEvent = JacksonEvent.builder().withEventType(EventType.DOCUMENT.toString()).withData(jsonString).build();
725+
event.merge(otherEvent, List.of("a"));
726+
727+
assertThat(event.containsKey("a"), equalTo(true));
728+
assertThat(event.get("a", Object.class), equalTo(null));
729+
}
730+
720731
@Test
721732
void merge_with_keys_skips_missing_keys_in_other() {
722733
event.put("existing", "value");

0 commit comments

Comments
 (0)