Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/changelog/134936.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 134936
summary: Fixing conditional processor mutability bugs
area: Ingest Node
type: bug
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,8 @@ private static Object wrapUnmodifiable(Object raw) {
return new UnmodifiableIngestData((Map<String, Object>) raw);
} else if (raw instanceof List) {
return new UnmodifiableIngestList((List<Object>) raw);
} else if (raw instanceof Set<?> rawSet) {
return new UnmodifiableIngestSet((Set<Object>) rawSet);
} else if (raw instanceof byte[] bytes) {
return bytes.clone();
}
Expand Down Expand Up @@ -287,23 +289,7 @@ public boolean contains(final Object o) {

@Override
public Iterator<Object> iterator() {
Iterator<Object> wrapped = data.iterator();
return new Iterator<Object>() {
@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
Expand Down Expand Up @@ -465,4 +451,100 @@ public void add(final Object o) {
}
}
}

private static final class UnmodifiableIngestSet implements Set<Object> {
private final Set<Object> data;

UnmodifiableIngestSet(Set<Object> 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<Object> iterator() {
return new UnmodifiableIterator(data.iterator());
}

@Override
public Object[] toArray() {
return data.toArray();
}

@Override
public <T> 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<Object> {
private final Iterator<Object> it;

UnmodifiableIterator(Iterator<Object> 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();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -141,6 +143,14 @@ public void testActsOnImmutableData() throws Exception {
assertMutatingCtxThrows(ctx -> ctx.put("foo", "bar"));
assertMutatingCtxThrows(ctx -> ((List<Object>) ctx.get("listField")).add("bar"));
assertMutatingCtxThrows(ctx -> ((List<Object>) ctx.get("listField")).remove("bar"));
assertMutatingCtxThrows(ctx -> ((Map<String, Object>) ctx.get("mapField")).put("bar", "baz"));
assertMutatingCtxThrows(ctx -> ((Map<?, ?>) ctx.get("mapField")).remove("bar"));
assertMutatingCtxThrows(ctx -> ((Set<Object>) ctx.get("setField")).add("bar"));
assertMutatingCtxThrows(ctx -> ((Set<Object>) ctx.get("setField")).remove("bar"));
assertMutatingCtxThrows(ctx -> ((List<Object>) ((Set<Object>) ctx.get("setField")).iterator().next()).add("bar"));
assertMutatingCtxThrows(
ctx -> ((List<Object>) ((List<Object>) ((Set<Object>) ctx.get("setField")).iterator().next()).iterator().next()).add("bar")
);
}

public void testPrecompiledError() {
Expand Down Expand Up @@ -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<Map<String, Object>> mutation) throws Exception {
String scriptName = "conditionalScript";
PlainActionFuture<Exception> expectedException = new PlainActionFuture<>();
Expand Down Expand Up @@ -221,6 +232,11 @@ private static void assertMutatingCtxThrows(Consumer<Map<String, Object>> mutati
);
IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document);
ingestDocument.setFieldValue("listField", new ArrayList<>());
ingestDocument.setFieldValue("mapField", new HashMap<>());
ingestDocument.setFieldValue("setField", new HashSet<>());
List<Object> 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));
Expand Down