From 105c365c4dfe52cce1d0506dd51f6ec46acabe94 Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Tue, 16 Sep 2025 09:41:06 -0500 Subject: [PATCH 1/5] Exposing ConditionalProcessors unmodifiable map for use by other classes in the package --- .../ingest/ConditionalProcessor.java | 17 ++++++++++++++--- .../ingest/ConditionalProcessorTests.java | 4 ++++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/ingest/ConditionalProcessor.java b/server/src/main/java/org/elasticsearch/ingest/ConditionalProcessor.java index 39c3882c4ee49..0214929c1c571 100644 --- a/server/src/main/java/org/elasticsearch/ingest/ConditionalProcessor.java +++ b/server/src/main/java/org/elasticsearch/ingest/ConditionalProcessor.java @@ -146,7 +146,7 @@ boolean evaluate(IngestDocument ingestDocument) { } return factory.newInstance( condition.getParams(), - new UnmodifiableIngestData(new DynamicMap(ingestDocument.getSourceAndMetadata(), FUNCTIONS)) + wrapUnmodifiableMap(new DynamicMap(ingestDocument.getSourceAndMetadata(), FUNCTIONS)) ).execute(); } @@ -171,8 +171,8 @@ public String getCondition() { private static Object wrapUnmodifiable(Object raw) { // Wraps all mutable types that the JSON parser can create by immutable wrappers. // Any inputs not wrapped are assumed to be immutable - if (raw instanceof Map) { - return new UnmodifiableIngestData((Map) raw); + if (raw instanceof Map rawMap) { + return wrapUnmodifiableMap((Map) rawMap); } else if (raw instanceof List) { return new UnmodifiableIngestList((List) raw); } else if (raw instanceof byte[] bytes) { @@ -185,6 +185,17 @@ private static UnsupportedOperationException unmodifiableException() { return new UnsupportedOperationException("Mutating ingest documents in conditionals is not supported"); } + /** + * This wraps the input map, returning an unmodifiable version of it. Any attempts to modify the returned map will throw an + * UnsupportedOperationException. This includes attempts to modify objects nested within the map. This map is assumed to be safe to + * execute painless scripts against, knowing that the script cannot accidentally change underlying data + * @param map The map to be wrapped + * @return An unmodifiable map with the same values as the input map + */ + static Map wrapUnmodifiableMap(Map map) { + return new UnmodifiableIngestData(map); + } + private static final class UnmodifiableIngestData implements Map { private final Map data; diff --git a/server/src/test/java/org/elasticsearch/ingest/ConditionalProcessorTests.java b/server/src/test/java/org/elasticsearch/ingest/ConditionalProcessorTests.java index efcdff2f86469..4c932e3fd9226 100644 --- a/server/src/test/java/org/elasticsearch/ingest/ConditionalProcessorTests.java +++ b/server/src/test/java/org/elasticsearch/ingest/ConditionalProcessorTests.java @@ -275,6 +275,10 @@ private static void assertMutatingCtxThrows(Consumer> mutati assertThat(e, instanceOf(UnsupportedOperationException.class)); assertEquals("Mutating ingest documents in conditionals is not supported", e.getMessage()); assertStats(processor, 0, 0, 0); + + Map unmodifiableDocument = ConditionalProcessor.wrapUnmodifiableMap(document); + assertThrows(UnsupportedOperationException.class, () -> mutation.accept(unmodifiableDocument)); + mutation.accept(document); // no exception expected } private static void assertStats(ConditionalProcessor conditionalProcessor, long count, long failed, long time) { From 1ef6bf51430673d6fea2388988115b48be3a9f12 Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Tue, 16 Sep 2025 15:37:52 -0500 Subject: [PATCH 2/5] Moving ConditionalProcessor unmodifiable map logic into IngestDocument --- .../ingest/ConditionalProcessor.java | 355 +----------------- .../elasticsearch/ingest/IngestDocument.java | 336 +++++++++++++++++ .../ingest/ConditionalProcessorTests.java | 4 - .../ingest/IngestDocumentTests.java | 32 ++ 4 files changed, 369 insertions(+), 358 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/ingest/ConditionalProcessor.java b/server/src/main/java/org/elasticsearch/ingest/ConditionalProcessor.java index 0214929c1c571..3a4e9e7926d25 100644 --- a/server/src/main/java/org/elasticsearch/ingest/ConditionalProcessor.java +++ b/server/src/main/java/org/elasticsearch/ingest/ConditionalProcessor.java @@ -9,28 +9,14 @@ package org.elasticsearch.ingest; -import org.elasticsearch.common.logging.DeprecationCategory; -import org.elasticsearch.common.logging.DeprecationLogger; -import org.elasticsearch.script.DynamicMap; import org.elasticsearch.script.IngestConditionalScript; import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptException; import org.elasticsearch.script.ScriptService; import org.elasticsearch.script.ScriptType; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; -import java.util.Iterator; -import java.util.List; -import java.util.ListIterator; -import java.util.Map; -import java.util.Set; import java.util.function.BiConsumer; -import java.util.function.Function; import java.util.function.LongSupplier; -import java.util.stream.Collectors; import static org.elasticsearch.ingest.ConfigurationUtils.newConfigurationException; @@ -39,16 +25,6 @@ */ public class ConditionalProcessor extends AbstractProcessor implements WrappingProcessor { - private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(DynamicMap.class); - private static final Map> FUNCTIONS = Map.of("_type", value -> { - deprecationLogger.warn( - DeprecationCategory.INDICES, - "conditional-processor__type", - "[types removal] Looking up doc types [_type] in scripts is deprecated." - ); - return value; - }); - static final String TYPE = "conditional"; private final Script condition; @@ -144,10 +120,7 @@ boolean evaluate(IngestDocument ingestDocument) { if (factory == null) { factory = scriptService.compile(condition, IngestConditionalScript.CONTEXT); } - return factory.newInstance( - condition.getParams(), - wrapUnmodifiableMap(new DynamicMap(ingestDocument.getSourceAndMetadata(), FUNCTIONS)) - ).execute(); + return factory.newInstance(condition.getParams(), ingestDocument.getUnmodifiableSourceAndMetadata()).execute(); } public Processor getInnerProcessor() { @@ -167,330 +140,4 @@ public String getCondition() { return condition.getIdOrCode(); } - @SuppressWarnings("unchecked") - private static Object wrapUnmodifiable(Object raw) { - // Wraps all mutable types that the JSON parser can create by immutable wrappers. - // Any inputs not wrapped are assumed to be immutable - if (raw instanceof Map rawMap) { - return wrapUnmodifiableMap((Map) rawMap); - } else if (raw instanceof List) { - return new UnmodifiableIngestList((List) raw); - } else if (raw instanceof byte[] bytes) { - return bytes.clone(); - } - return raw; - } - - private static UnsupportedOperationException unmodifiableException() { - return new UnsupportedOperationException("Mutating ingest documents in conditionals is not supported"); - } - - /** - * This wraps the input map, returning an unmodifiable version of it. Any attempts to modify the returned map will throw an - * UnsupportedOperationException. This includes attempts to modify objects nested within the map. This map is assumed to be safe to - * execute painless scripts against, knowing that the script cannot accidentally change underlying data - * @param map The map to be wrapped - * @return An unmodifiable map with the same values as the input map - */ - static Map wrapUnmodifiableMap(Map map) { - return new UnmodifiableIngestData(map); - } - - private static final class UnmodifiableIngestData implements Map { - - private final Map data; - - UnmodifiableIngestData(Map data) { - this.data = data; - } - - @Override - public int size() { - return data.size(); - } - - @Override - public boolean isEmpty() { - return data.isEmpty(); - } - - @Override - public boolean containsKey(final Object key) { - return data.containsKey(key); - } - - @Override - public boolean containsValue(final Object value) { - return data.containsValue(value); - } - - @Override - public Object get(final Object key) { - return wrapUnmodifiable(data.get(key)); - } - - @Override - public Object put(final String key, final Object value) { - throw unmodifiableException(); - } - - @Override - public Object remove(final Object key) { - throw unmodifiableException(); - } - - @Override - public void putAll(final Map m) { - throw unmodifiableException(); - } - - @Override - public void clear() { - throw unmodifiableException(); - } - - @Override - public Set keySet() { - return Collections.unmodifiableSet(data.keySet()); - } - - @Override - public Collection values() { - return new UnmodifiableIngestList(new ArrayList<>(data.values())); - } - - @Override - public Set> entrySet() { - return data.entrySet().stream().map(entry -> new Entry() { - @Override - public String getKey() { - return entry.getKey(); - } - - @Override - public Object getValue() { - return wrapUnmodifiable(entry.getValue()); - } - - @Override - public Object setValue(final Object value) { - throw unmodifiableException(); - } - - @Override - public boolean equals(final Object o) { - return entry.equals(o); - } - - @Override - public int hashCode() { - return entry.hashCode(); - } - }).collect(Collectors.toSet()); - } - } - - private static final class UnmodifiableIngestList implements List { - - private final List data; - - UnmodifiableIngestList(List data) { - this.data = data; - } - - @Override - public int size() { - return data.size(); - } - - @Override - public boolean isEmpty() { - return data.isEmpty(); - } - - @Override - public boolean contains(final Object o) { - return data.contains(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(); - } - }; - } - - @Override - public Object[] toArray() { - Object[] wrapped = data.toArray(new Object[0]); - for (int i = 0; i < wrapped.length; i++) { - wrapped[i] = wrapUnmodifiable(wrapped[i]); - } - return wrapped; - } - - @Override - @SuppressWarnings("unchecked") - public T[] toArray(final T[] a) { - Object[] raw = data.toArray(new Object[0]); - T[] wrapped = (T[]) Arrays.copyOf(raw, a.length, a.getClass()); - for (int i = 0; i < wrapped.length; i++) { - wrapped[i] = (T) wrapUnmodifiable(wrapped[i]); - } - return wrapped; - } - - @Override - public boolean add(final Object o) { - throw unmodifiableException(); - } - - @Override - public boolean remove(final Object o) { - throw unmodifiableException(); - } - - @Override - public boolean containsAll(final Collection c) { - return data.contains(c); - } - - @Override - public boolean addAll(final Collection c) { - throw unmodifiableException(); - } - - @Override - public boolean addAll(final int index, final Collection c) { - throw unmodifiableException(); - } - - @Override - public boolean removeAll(final Collection c) { - throw unmodifiableException(); - } - - @Override - public boolean retainAll(final Collection c) { - throw unmodifiableException(); - } - - @Override - public void clear() { - throw unmodifiableException(); - } - - @Override - public Object get(final int index) { - return wrapUnmodifiable(data.get(index)); - } - - @Override - public Object set(final int index, final Object element) { - throw unmodifiableException(); - } - - @Override - public void add(final int index, final Object element) { - throw unmodifiableException(); - } - - @Override - public Object remove(final int index) { - throw unmodifiableException(); - } - - @Override - public int indexOf(final Object o) { - return data.indexOf(o); - } - - @Override - public int lastIndexOf(final Object o) { - return data.lastIndexOf(o); - } - - @Override - public ListIterator listIterator() { - return new UnmodifiableListIterator(data.listIterator()); - } - - @Override - public ListIterator listIterator(final int index) { - return new UnmodifiableListIterator(data.listIterator(index)); - } - - @Override - public List subList(final int fromIndex, final int toIndex) { - return new UnmodifiableIngestList(data.subList(fromIndex, toIndex)); - } - - private static final class UnmodifiableListIterator implements ListIterator { - - private final ListIterator data; - - UnmodifiableListIterator(ListIterator data) { - this.data = data; - } - - @Override - public boolean hasNext() { - return data.hasNext(); - } - - @Override - public Object next() { - return wrapUnmodifiable(data.next()); - } - - @Override - public boolean hasPrevious() { - return data.hasPrevious(); - } - - @Override - public Object previous() { - return wrapUnmodifiable(data.previous()); - } - - @Override - public int nextIndex() { - return data.nextIndex(); - } - - @Override - public int previousIndex() { - return data.previousIndex(); - } - - @Override - public void remove() { - throw unmodifiableException(); - } - - @Override - public void set(final Object o) { - throw unmodifiableException(); - } - - @Override - public void add(final Object o) { - throw unmodifiableException(); - } - } - } } diff --git a/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java b/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java index f1ff163721004..f9d9ad3ec9b4a 100644 --- a/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java +++ b/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java @@ -10,6 +10,8 @@ package org.elasticsearch.ingest; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.logging.DeprecationCategory; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.util.CollectionUtils; import org.elasticsearch.common.util.Maps; import org.elasticsearch.common.util.concurrent.ConcurrentCollections; @@ -22,6 +24,7 @@ import org.elasticsearch.index.mapper.SourceFieldMapper; import org.elasticsearch.index.mapper.VersionFieldMapper; import org.elasticsearch.script.CtxMap; +import org.elasticsearch.script.DynamicMap; import org.elasticsearch.script.ScriptService; import org.elasticsearch.script.TemplateScript; @@ -36,13 +39,16 @@ import java.util.Date; import java.util.Deque; import java.util.HashMap; +import java.util.Iterator; import java.util.LinkedHashSet; import java.util.List; +import java.util.ListIterator; import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.function.BiConsumer; +import java.util.function.Function; import java.util.stream.Collectors; /** @@ -63,6 +69,16 @@ public final class IngestDocument { // a 'not found' sentinel value for use in getOrDefault calls in order to avoid containsKey-and-then-get private static final Object NOT_FOUND = new Object(); + private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(DynamicMap.class); + private static final Map> DEPRECATION_FUNCTIONS = Map.of("_type", value -> { + deprecationLogger.warn( + DeprecationCategory.INDICES, + "conditional-processor__type", + "[types removal] Looking up doc types [_type] in scripts is deprecated." + ); + return value; + }); + private final IngestCtxMap ctxMap; private final Map ingestMetadata; @@ -1006,6 +1022,10 @@ public Map getSourceAndMetadata() { return ctxMap; } + public Map getUnmodifiableSourceAndMetadata() { + return new UnmodifiableIngestData(new DynamicMap(ctxMap, DEPRECATION_FUNCTIONS)); + } + /** * Get the CtxMap */ @@ -1557,4 +1577,320 @@ private static String invalidPath(String fullPath) { return "path [" + fullPath + "] is not valid"; } } + + @SuppressWarnings("unchecked") + private static Object wrapUnmodifiable(Object raw) { + // Wraps all mutable types that the JSON parser can create by immutable wrappers. + // Any inputs not wrapped are assumed to be immutable + if (raw instanceof Map rawMap) { + return new UnmodifiableIngestData((Map) rawMap); + } else if (raw instanceof List) { + return new UnmodifiableIngestList((List) raw); + } else if (raw instanceof byte[] bytes) { + return bytes.clone(); + } + return raw; + } + + private static UnsupportedOperationException unmodifiableException() { + return new UnsupportedOperationException("Mutating ingest documents in conditionals is not supported"); + } + + private static final class UnmodifiableIngestData implements Map { + + private final Map data; + + UnmodifiableIngestData(Map data) { + this.data = data; + } + + @Override + public int size() { + return data.size(); + } + + @Override + public boolean isEmpty() { + return data.isEmpty(); + } + + @Override + public boolean containsKey(final Object key) { + return data.containsKey(key); + } + + @Override + public boolean containsValue(final Object value) { + return data.containsValue(value); + } + + @Override + public Object get(final Object key) { + return wrapUnmodifiable(data.get(key)); + } + + @Override + public Object put(final String key, final Object value) { + throw unmodifiableException(); + } + + @Override + public Object remove(final Object key) { + throw unmodifiableException(); + } + + @Override + public void putAll(final Map m) { + throw unmodifiableException(); + } + + @Override + public void clear() { + throw unmodifiableException(); + } + + @Override + public Set keySet() { + return Collections.unmodifiableSet(data.keySet()); + } + + @Override + public Collection values() { + return new UnmodifiableIngestList(new ArrayList<>(data.values())); + } + + @Override + public Set> entrySet() { + return data.entrySet().stream().map(entry -> new Entry() { + @Override + public String getKey() { + return entry.getKey(); + } + + @Override + public Object getValue() { + return wrapUnmodifiable(entry.getValue()); + } + + @Override + public Object setValue(final Object value) { + throw unmodifiableException(); + } + + @Override + public boolean equals(final Object o) { + return entry.equals(o); + } + + @Override + public int hashCode() { + return entry.hashCode(); + } + }).collect(Collectors.toSet()); + } + } + + private static final class UnmodifiableIngestList implements List { + + private final List data; + + UnmodifiableIngestList(List data) { + this.data = data; + } + + @Override + public int size() { + return data.size(); + } + + @Override + public boolean isEmpty() { + return data.isEmpty(); + } + + @Override + public boolean contains(final Object o) { + return data.contains(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(); + } + }; + } + + @Override + public Object[] toArray() { + Object[] wrapped = data.toArray(new Object[0]); + for (int i = 0; i < wrapped.length; i++) { + wrapped[i] = wrapUnmodifiable(wrapped[i]); + } + return wrapped; + } + + @Override + @SuppressWarnings("unchecked") + public T[] toArray(final T[] a) { + Object[] raw = data.toArray(new Object[0]); + T[] wrapped = (T[]) Arrays.copyOf(raw, a.length, a.getClass()); + for (int i = 0; i < wrapped.length; i++) { + wrapped[i] = (T) wrapUnmodifiable(wrapped[i]); + } + return wrapped; + } + + @Override + public boolean add(final Object o) { + throw unmodifiableException(); + } + + @Override + public boolean remove(final Object o) { + throw unmodifiableException(); + } + + @Override + public boolean containsAll(final Collection c) { + return data.contains(c); + } + + @Override + public boolean addAll(final Collection c) { + throw unmodifiableException(); + } + + @Override + public boolean addAll(final int index, final Collection c) { + throw unmodifiableException(); + } + + @Override + public boolean removeAll(final Collection c) { + throw unmodifiableException(); + } + + @Override + public boolean retainAll(final Collection c) { + throw unmodifiableException(); + } + + @Override + public void clear() { + throw unmodifiableException(); + } + + @Override + public Object get(final int index) { + return wrapUnmodifiable(data.get(index)); + } + + @Override + public Object set(final int index, final Object element) { + throw unmodifiableException(); + } + + @Override + public void add(final int index, final Object element) { + throw unmodifiableException(); + } + + @Override + public Object remove(final int index) { + throw unmodifiableException(); + } + + @Override + public int indexOf(final Object o) { + return data.indexOf(o); + } + + @Override + public int lastIndexOf(final Object o) { + return data.lastIndexOf(o); + } + + @Override + public ListIterator listIterator() { + return new UnmodifiableListIterator(data.listIterator()); + } + + @Override + public ListIterator listIterator(final int index) { + return new UnmodifiableListIterator(data.listIterator(index)); + } + + @Override + public List subList(final int fromIndex, final int toIndex) { + return new UnmodifiableIngestList(data.subList(fromIndex, toIndex)); + } + + private static final class UnmodifiableListIterator implements ListIterator { + + private final ListIterator data; + + UnmodifiableListIterator(ListIterator data) { + this.data = data; + } + + @Override + public boolean hasNext() { + return data.hasNext(); + } + + @Override + public Object next() { + return wrapUnmodifiable(data.next()); + } + + @Override + public boolean hasPrevious() { + return data.hasPrevious(); + } + + @Override + public Object previous() { + return wrapUnmodifiable(data.previous()); + } + + @Override + public int nextIndex() { + return data.nextIndex(); + } + + @Override + public int previousIndex() { + return data.previousIndex(); + } + + @Override + public void remove() { + throw unmodifiableException(); + } + + @Override + public void set(final Object o) { + throw unmodifiableException(); + } + + @Override + public void add(final Object o) { + 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 4c932e3fd9226..efcdff2f86469 100644 --- a/server/src/test/java/org/elasticsearch/ingest/ConditionalProcessorTests.java +++ b/server/src/test/java/org/elasticsearch/ingest/ConditionalProcessorTests.java @@ -275,10 +275,6 @@ private static void assertMutatingCtxThrows(Consumer> mutati assertThat(e, instanceOf(UnsupportedOperationException.class)); assertEquals("Mutating ingest documents in conditionals is not supported", e.getMessage()); assertStats(processor, 0, 0, 0); - - Map unmodifiableDocument = ConditionalProcessor.wrapUnmodifiableMap(document); - assertThrows(UnsupportedOperationException.class, () -> mutation.accept(unmodifiableDocument)); - mutation.accept(document); // no exception expected } private static void assertStats(ConditionalProcessor conditionalProcessor, long count, long failed, long time) { diff --git a/server/src/test/java/org/elasticsearch/ingest/IngestDocumentTests.java b/server/src/test/java/org/elasticsearch/ingest/IngestDocumentTests.java index f8b6e85bd08f7..c8a60b95e3424 100644 --- a/server/src/test/java/org/elasticsearch/ingest/IngestDocumentTests.java +++ b/server/src/test/java/org/elasticsearch/ingest/IngestDocumentTests.java @@ -2162,4 +2162,36 @@ void doTestNestedAccessPatternPropagation(int level, int maxCallDepth, IngestDoc } logger.debug("LEVEL {}/{}: COMPLETE", level, maxCallDepth); } + + @SuppressWarnings("unchecked") + public void testGetUnmodifiableSourceAndMetadata() { + assertMutatingThrows(ctx -> ctx.remove("foo")); + assertMutatingThrows(ctx -> ctx.put("foo", "bar")); + assertMutatingThrows(ctx -> ((List) ctx.get("listField")).add("bar")); + assertMutatingThrows(ctx -> ((List) ctx.get("listField")).remove("bar")); + assertMutatingThrows(ctx -> ((Map) ctx.get("mapField")).put("bar", "baz")); + assertMutatingThrows(ctx -> ((Map) ctx.get("mapField")).remove("bar")); + + /* + * The source can also have a byte array. But we do not throw an UnsupportedOperationException when a byte array is changed -- + * we just ignore the change. + */ + Map document = new HashMap<>(); + IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document); + ingestDocument.setFieldValue("byteArrayField", randomByteArrayOfLength(10)); + Map unmodifiableDocument = ingestDocument.getUnmodifiableSourceAndMetadata(); + byte originalByteValue = ((byte[]) unmodifiableDocument.get("byteArrayField"))[0]; + ((byte[]) unmodifiableDocument.get("byteArrayField"))[0] = (byte) (originalByteValue + 1); + assertThat(((byte[]) unmodifiableDocument.get("byteArrayField"))[0], equalTo(originalByteValue)); + } + + public void assertMutatingThrows(Consumer> mutation) { + Map document = new HashMap<>(); + IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document); + ingestDocument.setFieldValue("listField", new ArrayList<>()); + ingestDocument.setFieldValue("mapField", new HashMap<>()); + Map unmodifiableDocument = ingestDocument.getUnmodifiableSourceAndMetadata(); + assertThrows(UnsupportedOperationException.class, () -> mutation.accept(unmodifiableDocument)); + mutation.accept(ingestDocument.getSourceAndMetadata()); // no exception expected + } } From 82abbb77a4981cda6e5b64e84c0baa78c7cad427 Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Tue, 16 Sep 2025 15:46:41 -0500 Subject: [PATCH 3/5] removing deprecation logging about _type --- .../elasticsearch/ingest/IngestDocument.java | 22 +++------ .../ingest/ConditionalProcessorTests.java | 49 ------------------- 2 files changed, 7 insertions(+), 64 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java b/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java index f9d9ad3ec9b4a..8fd901d71b0a1 100644 --- a/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java +++ b/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java @@ -10,8 +10,6 @@ package org.elasticsearch.ingest; import org.elasticsearch.common.Strings; -import org.elasticsearch.common.logging.DeprecationCategory; -import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.util.CollectionUtils; import org.elasticsearch.common.util.Maps; import org.elasticsearch.common.util.concurrent.ConcurrentCollections; @@ -24,7 +22,6 @@ import org.elasticsearch.index.mapper.SourceFieldMapper; import org.elasticsearch.index.mapper.VersionFieldMapper; import org.elasticsearch.script.CtxMap; -import org.elasticsearch.script.DynamicMap; import org.elasticsearch.script.ScriptService; import org.elasticsearch.script.TemplateScript; @@ -48,7 +45,6 @@ import java.util.Optional; import java.util.Set; import java.util.function.BiConsumer; -import java.util.function.Function; import java.util.stream.Collectors; /** @@ -69,16 +65,6 @@ public final class IngestDocument { // a 'not found' sentinel value for use in getOrDefault calls in order to avoid containsKey-and-then-get private static final Object NOT_FOUND = new Object(); - private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(DynamicMap.class); - private static final Map> DEPRECATION_FUNCTIONS = Map.of("_type", value -> { - deprecationLogger.warn( - DeprecationCategory.INDICES, - "conditional-processor__type", - "[types removal] Looking up doc types [_type] in scripts is deprecated." - ); - return value; - }); - private final IngestCtxMap ctxMap; private final Map ingestMetadata; @@ -1022,8 +1008,14 @@ public Map getSourceAndMetadata() { return ctxMap; } + /* + * This returns the same information as getSourceAndMetadata(), but in an unmodifiable map that is safe to send into a script that is + * not supposed to be modifying the data. If an attempt is made to modify this Map, or a Map or List nested within it, an + * UnsupportedOperationException is thrown. If an attempt is made to modify a byte[] within this Map, the attempt succeeds, but the + * results are not reflected on this IngestDocument. + */ public Map getUnmodifiableSourceAndMetadata() { - return new UnmodifiableIngestData(new DynamicMap(ctxMap, DEPRECATION_FUNCTIONS)); + return new UnmodifiableIngestData(ctxMap); } /** diff --git a/server/src/test/java/org/elasticsearch/ingest/ConditionalProcessorTests.java b/server/src/test/java/org/elasticsearch/ingest/ConditionalProcessorTests.java index efcdff2f86469..1317233d3107a 100644 --- a/server/src/test/java/org/elasticsearch/ingest/ConditionalProcessorTests.java +++ b/server/src/test/java/org/elasticsearch/ingest/ConditionalProcessorTests.java @@ -143,55 +143,6 @@ public void testActsOnImmutableData() throws Exception { assertMutatingCtxThrows(ctx -> ((List) ctx.get("listField")).remove("bar")); } - public void testTypeDeprecation() throws Exception { - - ScriptService scriptService = new ScriptService( - Settings.builder().build(), - Map.of(Script.DEFAULT_SCRIPT_LANG, new MockScriptEngine(Script.DEFAULT_SCRIPT_LANG, Map.of(scriptName, ctx -> { - ctx.get("_type"); - return true; - }), Map.of())), - new HashMap<>(ScriptModule.CORE_CONTEXTS), - () -> 1L, - TestProjectResolvers.singleProject(randomProjectIdOrDefault()) - ); - - LongSupplier relativeTimeProvider = mock(LongSupplier.class); - when(relativeTimeProvider.getAsLong()).thenReturn(0L, TimeUnit.MILLISECONDS.toNanos(1), 0L, TimeUnit.MILLISECONDS.toNanos(2)); - ConditionalProcessor processor = new ConditionalProcessor( - randomAlphaOfLength(10), - "description", - new Script(ScriptType.INLINE, Script.DEFAULT_SCRIPT_LANG, scriptName, Map.of()), - scriptService, - new Processor() { - @Override - public IngestDocument execute(final IngestDocument ingestDocument) { - return ingestDocument; - } - - @Override - public String getType() { - return null; - } - - @Override - public String getTag() { - return null; - } - - @Override - public String getDescription() { - return null; - } - }, - relativeTimeProvider - ); - - IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), Map.of()); - execProcessor(processor, ingestDocument, (result, e) -> {}); - assertWarnings("[types removal] Looking up doc types [_type] in scripts is deprecated."); - } - public void testPrecompiledError() { ScriptService scriptService = MockScriptService.singleContext(IngestConditionalScript.CONTEXT, code -> { throw new ScriptException("bad script", new ParseException("error", 0), List.of(), "", "lang", null); From 1a7960bf312d97f54ff094f105bc9a0a0ec7461f Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Tue, 16 Sep 2025 17:10:33 -0500 Subject: [PATCH 4/5] Adding support for sets as well --- .../elasticsearch/ingest/IngestDocument.java | 116 +++++++++++++++--- .../ingest/IngestDocumentTests.java | 12 ++ 2 files changed, 111 insertions(+), 17 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java b/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java index 8fd901d71b0a1..2868b5ac9c4e0 100644 --- a/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java +++ b/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java @@ -1578,6 +1578,8 @@ private static Object wrapUnmodifiable(Object raw) { return new UnmodifiableIngestData((Map) rawMap); } 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(); } @@ -1707,23 +1709,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 @@ -1885,4 +1871,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/IngestDocumentTests.java b/server/src/test/java/org/elasticsearch/ingest/IngestDocumentTests.java index c8a60b95e3424..95c0fc5b72c24 100644 --- a/server/src/test/java/org/elasticsearch/ingest/IngestDocumentTests.java +++ b/server/src/test/java/org/elasticsearch/ingest/IngestDocumentTests.java @@ -23,6 +23,7 @@ import java.time.ZonedDateTime; import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -2169,8 +2170,14 @@ public void testGetUnmodifiableSourceAndMetadata() { assertMutatingThrows(ctx -> ctx.put("foo", "bar")); assertMutatingThrows(ctx -> ((List) ctx.get("listField")).add("bar")); assertMutatingThrows(ctx -> ((List) ctx.get("listField")).remove("bar")); + assertMutatingThrows(ctx -> ((Set) ctx.get("setField")).add("bar")); + assertMutatingThrows(ctx -> ((Set) ctx.get("setField")).remove("bar")); assertMutatingThrows(ctx -> ((Map) ctx.get("mapField")).put("bar", "baz")); assertMutatingThrows(ctx -> ((Map) ctx.get("mapField")).remove("bar")); + assertMutatingThrows(ctx -> ((List) ((Set) ctx.get("setField")).iterator().next()).add("bar")); + assertMutatingThrows( + ctx -> ((List) ((List) ((Set) ctx.get("setField")).iterator().next()).iterator().next()).add("bar") + ); /* * The source can also have a byte array. But we do not throw an UnsupportedOperationException when a byte array is changed -- @@ -2185,11 +2192,16 @@ public void testGetUnmodifiableSourceAndMetadata() { assertThat(((byte[]) unmodifiableDocument.get("byteArrayField"))[0], equalTo(originalByteValue)); } + @SuppressWarnings("unchecked") public void assertMutatingThrows(Consumer> mutation) { Map document = new HashMap<>(); 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); Map unmodifiableDocument = ingestDocument.getUnmodifiableSourceAndMetadata(); assertThrows(UnsupportedOperationException.class, () -> mutation.accept(unmodifiableDocument)); mutation.accept(ingestDocument.getSourceAndMetadata()); // no exception expected From 4425f8f45c92cb690a5c1fbc88691239341edcb0 Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Thu, 18 Sep 2025 09:47:26 -0500 Subject: [PATCH 5/5] updating comments --- .../org/elasticsearch/ingest/IngestDocument.java | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java b/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java index 2868b5ac9c4e0..55c767e80cd0a 100644 --- a/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java +++ b/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java @@ -1010,9 +1010,11 @@ public Map getSourceAndMetadata() { /* * This returns the same information as getSourceAndMetadata(), but in an unmodifiable map that is safe to send into a script that is - * not supposed to be modifying the data. If an attempt is made to modify this Map, or a Map or List nested within it, an + * not supposed to be modifying the data. If an attempt is made to modify this Map, or a Map, List, or Set nested within it, an * UnsupportedOperationException is thrown. If an attempt is made to modify a byte[] within this Map, the attempt succeeds, but the - * results are not reflected on this IngestDocument. + * results are not reflected on this IngestDocument. If a user has put any other mutable Object into the IngestDocument, this method + * makes no attempt to make it immutable. This method just protects users against accidentally modifying the most common types of + * Objects found in IngestDocuments. */ public Map getUnmodifiableSourceAndMetadata() { return new UnmodifiableIngestData(ctxMap); @@ -1572,8 +1574,13 @@ private static String invalidPath(String fullPath) { @SuppressWarnings("unchecked") private static Object wrapUnmodifiable(Object raw) { - // Wraps all mutable types that the JSON parser can create by immutable wrappers. - // Any inputs not wrapped are assumed to be immutable + /* + * This method makes an attempt to make the raw Object and its children immutable, if it is one of a known set of classes. If raw + * is a Map, List, or Set, an immutable version will be returned, and an UnsupportedOperationException will be thrown if an attempt + * to modify it is made. All the Objects in those collections are also made unmodifiable by this method. If raw is a byte[], a copy + * of the byte[] will be returned so that changes to it will not be reflected in the original data. No exception will be thrown if + * a user modifies it though. + */ if (raw instanceof Map rawMap) { return new UnmodifiableIngestData((Map) rawMap); } else if (raw instanceof List) {