Skip to content

Commit 6938ed0

Browse files
masseykegmjehovich
authored andcommitted
Fixing conditional processor mutability bugs (elastic#134936)
1 parent 653b034 commit 6938ed0

File tree

3 files changed

+120
-17
lines changed

3 files changed

+120
-17
lines changed

docs/changelog/134936.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 134936
2+
summary: Fixing conditional processor mutability bugs
3+
area: Ingest Node
4+
type: bug
5+
issues: []

server/src/main/java/org/elasticsearch/ingest/ConditionalProcessor.java

Lines changed: 99 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,8 @@ private static Object wrapUnmodifiable(Object raw) {
158158
return new UnmodifiableIngestData((Map<String, Object>) raw);
159159
} else if (raw instanceof List) {
160160
return new UnmodifiableIngestList((List<Object>) raw);
161+
} else if (raw instanceof Set<?> rawSet) {
162+
return new UnmodifiableIngestSet((Set<Object>) rawSet);
161163
} else if (raw instanceof byte[] bytes) {
162164
return bytes.clone();
163165
}
@@ -287,23 +289,7 @@ public boolean contains(final Object o) {
287289

288290
@Override
289291
public Iterator<Object> iterator() {
290-
Iterator<Object> wrapped = data.iterator();
291-
return new Iterator<Object>() {
292-
@Override
293-
public boolean hasNext() {
294-
return wrapped.hasNext();
295-
}
296-
297-
@Override
298-
public Object next() {
299-
return wrapped.next();
300-
}
301-
302-
@Override
303-
public void remove() {
304-
throw unmodifiableException();
305-
}
306-
};
292+
return new UnmodifiableIterator(data.iterator());
307293
}
308294

309295
@Override
@@ -465,4 +451,100 @@ public void add(final Object o) {
465451
}
466452
}
467453
}
454+
455+
private static final class UnmodifiableIngestSet implements Set<Object> {
456+
private final Set<Object> data;
457+
458+
UnmodifiableIngestSet(Set<Object> data) {
459+
this.data = data;
460+
}
461+
462+
@Override
463+
public int size() {
464+
return data.size();
465+
}
466+
467+
@Override
468+
public boolean isEmpty() {
469+
return data.isEmpty();
470+
}
471+
472+
@Override
473+
public boolean contains(Object o) {
474+
return data.contains(o);
475+
}
476+
477+
@Override
478+
public Iterator<Object> iterator() {
479+
return new UnmodifiableIterator(data.iterator());
480+
}
481+
482+
@Override
483+
public Object[] toArray() {
484+
return data.toArray();
485+
}
486+
487+
@Override
488+
public <T> T[] toArray(T[] a) {
489+
return data.toArray(a);
490+
}
491+
492+
@Override
493+
public boolean add(Object o) {
494+
throw unmodifiableException();
495+
}
496+
497+
@Override
498+
public boolean remove(Object o) {
499+
throw unmodifiableException();
500+
}
501+
502+
@Override
503+
public boolean containsAll(Collection<?> c) {
504+
return data.containsAll(c);
505+
}
506+
507+
@Override
508+
public boolean addAll(Collection<?> c) {
509+
throw unmodifiableException();
510+
}
511+
512+
@Override
513+
public boolean retainAll(Collection<?> c) {
514+
throw unmodifiableException();
515+
}
516+
517+
@Override
518+
public boolean removeAll(Collection<?> c) {
519+
throw unmodifiableException();
520+
}
521+
522+
@Override
523+
public void clear() {
524+
throw unmodifiableException();
525+
}
526+
}
527+
528+
private static final class UnmodifiableIterator implements Iterator<Object> {
529+
private final Iterator<Object> it;
530+
531+
UnmodifiableIterator(Iterator<Object> it) {
532+
this.it = it;
533+
}
534+
535+
@Override
536+
public boolean hasNext() {
537+
return it.hasNext();
538+
}
539+
540+
@Override
541+
public Object next() {
542+
return wrapUnmodifiable(it.next());
543+
}
544+
545+
@Override
546+
public void remove() {
547+
throw unmodifiableException();
548+
}
549+
}
468550
}

server/src/test/java/org/elasticsearch/ingest/ConditionalProcessorTests.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,10 @@
2626
import java.text.ParseException;
2727
import java.util.ArrayList;
2828
import java.util.HashMap;
29+
import java.util.HashSet;
2930
import java.util.List;
3031
import java.util.Map;
32+
import java.util.Set;
3133
import java.util.concurrent.TimeUnit;
3234
import java.util.concurrent.atomic.AtomicBoolean;
3335
import java.util.function.BiConsumer;
@@ -141,6 +143,14 @@ public void testActsOnImmutableData() throws Exception {
141143
assertMutatingCtxThrows(ctx -> ctx.put("foo", "bar"));
142144
assertMutatingCtxThrows(ctx -> ((List<Object>) ctx.get("listField")).add("bar"));
143145
assertMutatingCtxThrows(ctx -> ((List<Object>) ctx.get("listField")).remove("bar"));
146+
assertMutatingCtxThrows(ctx -> ((Map<String, Object>) ctx.get("mapField")).put("bar", "baz"));
147+
assertMutatingCtxThrows(ctx -> ((Map<?, ?>) ctx.get("mapField")).remove("bar"));
148+
assertMutatingCtxThrows(ctx -> ((Set<Object>) ctx.get("setField")).add("bar"));
149+
assertMutatingCtxThrows(ctx -> ((Set<Object>) ctx.get("setField")).remove("bar"));
150+
assertMutatingCtxThrows(ctx -> ((List<Object>) ((Set<Object>) ctx.get("setField")).iterator().next()).add("bar"));
151+
assertMutatingCtxThrows(
152+
ctx -> ((List<Object>) ((List<Object>) ((Set<Object>) ctx.get("setField")).iterator().next()).iterator().next()).add("bar")
153+
);
144154
}
145155

146156
public void testPrecompiledError() {
@@ -194,6 +204,7 @@ public boolean execute() {
194204
execProcessor(processor, ingestDoc, (doc, e) -> { assertThat(e.getMessage(), equalTo("runtime problem")); });
195205
}
196206

207+
@SuppressWarnings("unchecked")
197208
private static void assertMutatingCtxThrows(Consumer<Map<String, Object>> mutation) throws Exception {
198209
String scriptName = "conditionalScript";
199210
PlainActionFuture<Exception> expectedException = new PlainActionFuture<>();
@@ -221,6 +232,11 @@ private static void assertMutatingCtxThrows(Consumer<Map<String, Object>> mutati
221232
);
222233
IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document);
223234
ingestDocument.setFieldValue("listField", new ArrayList<>());
235+
ingestDocument.setFieldValue("mapField", new HashMap<>());
236+
ingestDocument.setFieldValue("setField", new HashSet<>());
237+
List<Object> listWithinSet = new ArrayList<>();
238+
listWithinSet.add(new ArrayList<>());
239+
ingestDocument.getFieldValue("setField", Set.class).add(listWithinSet);
224240
execProcessor(processor, ingestDocument, (result, e) -> {});
225241
Exception e = safeGet(expectedException);
226242
assertThat(e, instanceOf(UnsupportedOperationException.class));

0 commit comments

Comments
 (0)