diff --git a/docs/changelog/134936.yaml b/docs/changelog/134936.yaml new file mode 100644 index 0000000000000..d9ff3aef5c031 --- /dev/null +++ b/docs/changelog/134936.yaml @@ -0,0 +1,5 @@ +pr: 134936 +summary: Fixing conditional processor mutability bugs +area: Ingest Node +type: bug +issues: [] diff --git a/server/src/main/java/org/elasticsearch/ingest/ConditionalProcessor.java b/server/src/main/java/org/elasticsearch/ingest/ConditionalProcessor.java index 24e093d5e1883..9582279bfbe93 100644 --- a/server/src/main/java/org/elasticsearch/ingest/ConditionalProcessor.java +++ b/server/src/main/java/org/elasticsearch/ingest/ConditionalProcessor.java @@ -158,6 +158,8 @@ private static Object wrapUnmodifiable(Object raw) { return new UnmodifiableIngestData((Map) raw); } else if (raw instanceof List) { return new UnmodifiableIngestList((List) raw); + } else if (raw instanceof Set rawSet) { + return new UnmodifiableIngestSet((Set) rawSet); } else if (raw instanceof byte[] bytes) { return bytes.clone(); } @@ -287,23 +289,7 @@ public boolean contains(final Object o) { @Override public Iterator iterator() { - Iterator wrapped = data.iterator(); - return new Iterator() { - @Override - public boolean hasNext() { - return wrapped.hasNext(); - } - - @Override - public Object next() { - return wrapped.next(); - } - - @Override - public void remove() { - throw unmodifiableException(); - } - }; + return new UnmodifiableIterator(data.iterator()); } @Override @@ -465,4 +451,100 @@ public void add(final Object o) { } } } + + private static final class UnmodifiableIngestSet implements Set { + private final Set data; + + UnmodifiableIngestSet(Set data) { + this.data = data; + } + + @Override + public int size() { + return data.size(); + } + + @Override + public boolean isEmpty() { + return data.isEmpty(); + } + + @Override + public boolean contains(Object o) { + return data.contains(o); + } + + @Override + public Iterator iterator() { + return new UnmodifiableIterator(data.iterator()); + } + + @Override + public Object[] toArray() { + return data.toArray(); + } + + @Override + public T[] toArray(T[] a) { + return data.toArray(a); + } + + @Override + public boolean add(Object o) { + throw unmodifiableException(); + } + + @Override + public boolean remove(Object o) { + throw unmodifiableException(); + } + + @Override + public boolean containsAll(Collection c) { + return data.containsAll(c); + } + + @Override + public boolean addAll(Collection c) { + throw unmodifiableException(); + } + + @Override + public boolean retainAll(Collection c) { + throw unmodifiableException(); + } + + @Override + public boolean removeAll(Collection c) { + throw unmodifiableException(); + } + + @Override + public void clear() { + throw unmodifiableException(); + } + } + + private static final class UnmodifiableIterator implements Iterator { + private final Iterator it; + + UnmodifiableIterator(Iterator it) { + this.it = it; + } + + @Override + public boolean hasNext() { + return it.hasNext(); + } + + @Override + public Object next() { + return wrapUnmodifiable(it.next()); + } + + @Override + public void remove() { + throw unmodifiableException(); + } + } } diff --git a/server/src/test/java/org/elasticsearch/ingest/ConditionalProcessorTests.java b/server/src/test/java/org/elasticsearch/ingest/ConditionalProcessorTests.java index 1317233d3107a..144036ce80eda 100644 --- a/server/src/test/java/org/elasticsearch/ingest/ConditionalProcessorTests.java +++ b/server/src/test/java/org/elasticsearch/ingest/ConditionalProcessorTests.java @@ -26,8 +26,10 @@ import java.text.ParseException; import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.BiConsumer; @@ -141,6 +143,14 @@ public void testActsOnImmutableData() throws Exception { assertMutatingCtxThrows(ctx -> ctx.put("foo", "bar")); assertMutatingCtxThrows(ctx -> ((List) ctx.get("listField")).add("bar")); assertMutatingCtxThrows(ctx -> ((List) ctx.get("listField")).remove("bar")); + assertMutatingCtxThrows(ctx -> ((Map) ctx.get("mapField")).put("bar", "baz")); + assertMutatingCtxThrows(ctx -> ((Map) ctx.get("mapField")).remove("bar")); + assertMutatingCtxThrows(ctx -> ((Set) ctx.get("setField")).add("bar")); + assertMutatingCtxThrows(ctx -> ((Set) ctx.get("setField")).remove("bar")); + assertMutatingCtxThrows(ctx -> ((List) ((Set) ctx.get("setField")).iterator().next()).add("bar")); + assertMutatingCtxThrows( + ctx -> ((List) ((List) ((Set) ctx.get("setField")).iterator().next()).iterator().next()).add("bar") + ); } public void testPrecompiledError() { @@ -194,6 +204,7 @@ public boolean execute() { execProcessor(processor, ingestDoc, (doc, e) -> { assertThat(e.getMessage(), equalTo("runtime problem")); }); } + @SuppressWarnings("unchecked") private static void assertMutatingCtxThrows(Consumer> mutation) throws Exception { String scriptName = "conditionalScript"; PlainActionFuture expectedException = new PlainActionFuture<>(); @@ -221,6 +232,11 @@ private static void assertMutatingCtxThrows(Consumer> mutati ); IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document); ingestDocument.setFieldValue("listField", new ArrayList<>()); + ingestDocument.setFieldValue("mapField", new HashMap<>()); + ingestDocument.setFieldValue("setField", new HashSet<>()); + List listWithinSet = new ArrayList<>(); + listWithinSet.add(new ArrayList<>()); + ingestDocument.getFieldValue("setField", Set.class).add(listWithinSet); execProcessor(processor, ingestDocument, (result, e) -> {}); Exception e = safeGet(expectedException); assertThat(e, instanceOf(UnsupportedOperationException.class));