From 99c50ee511c0898b22495f9dc2fb2e710747cf76 Mon Sep 17 00:00:00 2001 From: Dan Hermann Date: Wed, 3 Mar 2021 08:09:32 -0600 Subject: [PATCH] Set processor's copy_from should deep copy non-primitive mutable types (#69349) (#69871) --- .../ingest/common/SetProcessor.java | 2 +- .../ingest/common/SetProcessorTests.java | 63 +++++++++++++++++++ .../elasticsearch/ingest/IngestDocument.java | 2 +- 3 files changed, 65 insertions(+), 2 deletions(-) diff --git a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/SetProcessor.java b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/SetProcessor.java index 76a5e872e1f1d..02d0f923ec1fa 100644 --- a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/SetProcessor.java +++ b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/SetProcessor.java @@ -73,7 +73,7 @@ public IngestDocument execute(IngestDocument document) { if (overrideEnabled || document.hasField(field) == false || document.getFieldValue(field, Object.class) == null) { if (copyFrom != null) { Object fieldValue = document.getFieldValue(copyFrom, Object.class, ignoreEmptyValue); - document.setFieldValue(field, fieldValue, ignoreEmptyValue); + document.setFieldValue(field, IngestDocument.deepCopy(fieldValue), ignoreEmptyValue); } else { document.setFieldValue(field, value, ignoreEmptyValue); } diff --git a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/SetProcessorTests.java b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/SetProcessorTests.java index b59179b2de0bc..4228ce93eaf14 100644 --- a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/SetProcessorTests.java +++ b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/SetProcessorTests.java @@ -17,8 +17,13 @@ import org.elasticsearch.test.ESTestCase; import org.hamcrest.Matchers; +import java.util.ArrayList; +import java.util.Date; import java.util.HashMap; +import java.util.HashSet; +import java.util.List; import java.util.Map; +import java.util.Set; import static org.hamcrest.Matchers.equalTo; @@ -144,6 +149,64 @@ public void testCopyFromOtherField() throws Exception { assertThat(ingestDocument.getFieldValue(fieldName, Object.class), equalTo(fieldValue)); } + public void testCopyFromDeepCopiesNonPrimitiveMutableTypes() throws Exception { + final String originalField = "originalField"; + final String targetField = "targetField"; + Processor processor = createSetProcessor(targetField, null, originalField, true, false); + + // map types + Map document = new HashMap<>(); + Map originalMap = new HashMap<>(); + originalMap.put("foo", "bar"); + document.put(originalField, originalMap); + IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document); + IngestDocument output = processor.execute(ingestDocument); + originalMap.put("foo", "not-bar"); + Map outputMap = output.getFieldValue(targetField, Map.class); + assertThat(outputMap.get("foo"), equalTo("bar")); + + // set types + document = new HashMap<>(); + Set originalSet = randomUnique(() -> randomAlphaOfLength(5), 5); + Set preservedSet = new HashSet<>(originalSet); + document.put(originalField, originalSet); + ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document); + processor.execute(ingestDocument); + originalSet.add(randomValueOtherThanMany(originalSet::contains, () -> randomAlphaOfLength(5))); + assertThat(ingestDocument.getFieldValue(targetField, Object.class), equalTo(preservedSet)); + + // list types + document = new HashMap<>(); + List originalList = randomList(1, 5, () -> randomAlphaOfLength(5)); + List preservedList = new ArrayList<>(originalList); + document.put(originalField, originalList); + ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document); + processor.execute(ingestDocument); + originalList.add(randomValueOtherThanMany(originalList::contains, () -> randomAlphaOfLength(5))); + assertThat(ingestDocument.getFieldValue(targetField, Object.class), equalTo(preservedList)); + + // byte[] types + document = new HashMap<>(); + byte[] originalBytes = randomByteArrayOfLength(10); + byte[] preservedBytes = new byte[originalBytes.length]; + System.arraycopy(originalBytes, 0, preservedBytes, 0, originalBytes.length); + document.put(originalField, originalBytes); + ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document); + processor.execute(ingestDocument); + originalBytes[0] = originalBytes[0] == 0 ? (byte) 1 : (byte) 0; + assertThat(ingestDocument.getFieldValue(targetField, Object.class), equalTo(preservedBytes)); + + // Date types + document = new HashMap<>(); + Date originalDate = new Date(); + Date preservedDate = new Date(originalDate.getTime()); + document.put(originalField, originalDate); + ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document); + processor.execute(ingestDocument); + originalDate.setTime(originalDate.getTime() + 1); + assertThat(ingestDocument.getFieldValue(targetField, Object.class), equalTo(preservedDate)); + } + private static Processor createSetProcessor(String fieldName, Object fieldValue, String copyFrom, boolean overrideEnabled, boolean ignoreEmptyValue) { return new SetProcessor(randomAlphaOfLength(10), null, new TestTemplateService.MockTemplateScript.Factory(fieldName), diff --git a/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java b/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java index 5e820cf129023..fca79b7f73cb8 100644 --- a/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java +++ b/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java @@ -716,7 +716,7 @@ public static Map deepCopyMap(Map source) { return (Map) deepCopy(source); } - private static Object deepCopy(Object value) { + public static Object deepCopy(Object value) { if (value instanceof Map) { Map mapValue = (Map) value; Map copy = new HashMap<>(mapValue.size());