Skip to content

Commit

Permalink
Qute: type pollution fixes
Browse files Browse the repository at this point in the history
- turn ResultNode in an abstract class
- restrict the support of CompletionStage by default; i.e. only consider
  CompletableFuture and CompletedStage
- in some cases intentionally use "instanceof" to test interfaces as the last resort in order to mitigate the type pollution
- also make Qute's version of LazyValue Loom-friendly
  • Loading branch information
mkouba committed Sep 13, 2023
1 parent a45d2e5 commit 2d33b47
Show file tree
Hide file tree
Showing 9 changed files with 79 additions and 35 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package io.quarkus.qute;

import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CompletionStage;

class CompletionStageSupport {

static final String SYSTEM_PROPERTY = "quarkus.qute.unrestricted-completion-stage-support";

/**
* {@code true} if any {@link CompletionStage} implementation is supported. {@code false} if only {@link CompletableFuture}
* and {@link CompletedStage} are considered in the API.
*/
static final boolean UNRESTRICTED = Boolean.getBoolean(SYSTEM_PROPERTY);
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ class EvaluatorImpl implements Evaluator {
private final Map<String, NamespaceResolver[]> namespaceResolvers;
private final boolean strictRendering;
private final ErrorInitializer initializer;
private final boolean unrestrictedCompletionStages;

EvaluatorImpl(List<ValueResolver> valueResolvers, List<NamespaceResolver> namespaceResolvers, boolean strictRendering,
ErrorInitializer errorInitializer) {
Expand All @@ -52,6 +53,7 @@ class EvaluatorImpl implements Evaluator {
this.namespaceResolvers = namespaceResolversMap;
this.strictRendering = strictRendering;
this.initializer = errorInitializer;
this.unrestrictedCompletionStages = CompletionStageSupport.UNRESTRICTED;
}

@Override
Expand Down Expand Up @@ -218,16 +220,16 @@ private CompletionStage<Object> resolve(EvalContextImpl evalContext, Iterator<Va
}

@SuppressWarnings("unchecked")
private static CompletionStage<Object> toCompletionStage(Object result) {
// Note that we intentionally avoid "result instanceof Uni"; see https://github.com/RedHatPerf/type-pollution-agent
// Unfortunatelly, we can't get rid of "result instanceof CompletionStage" so we at least try to test CompletableFuture and CompletedStage first
private CompletionStage<Object> toCompletionStage(Object result) {
// Note that we intentionally use "instanceof" to test interfaces as the last resort in order to mitigate the "type pollution"
// See https://github.com/RedHatPerf/type-pollution-agent for more information
if (result instanceof CompletableFuture) {
return (CompletableFuture<Object>) result;
} else if (result instanceof CompletedStage) {
return (CompletedStage<Object>) result;
} else if (result instanceof AbstractUni) {
return ((AbstractUni<Object>) result).subscribeAsCompletionStage();
} else if (result instanceof CompletionStage) {
} else if (unrestrictedCompletionStages && result instanceof CompletionStage) {
return (CompletionStage<Object>) result;
}
return CompletedStage.of(result);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,47 +2,52 @@

import java.util.Collections;
import java.util.List;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CompletionStage;
import java.util.function.Function;

import org.jboss.logging.Logger;

/**
* This node holds a single expression such as {@code foo.bar}.
*/
class ExpressionNode implements TemplateNode, Function<Object, CompletionStage<ResultNode>> {
class ExpressionNode implements TemplateNode {

private static final Logger LOG = Logger.getLogger("io.quarkus.qute.nodeResolve");

final ExpressionImpl expression;
private final Engine engine;
private final boolean traceLevel;
private final boolean hasEngineResultMappers;
private final boolean unrestrictedCompletionStages;

ExpressionNode(ExpressionImpl expression, Engine engine) {
this.expression = expression;
this.engine = engine;
this.traceLevel = LOG.isTraceEnabled();
this.hasEngineResultMappers = !engine.getResultMappers().isEmpty();
this.unrestrictedCompletionStages = CompletionStageSupport.UNRESTRICTED;
}

@Override
public CompletionStage<ResultNode> resolve(ResolutionContext context) {
if (traceLevel) {
LOG.tracef("Resolve {%s} started:%s", expression.toOriginalString(), expression.getOrigin());
}
return context.evaluate(expression).thenCompose(this);
return context.evaluate(expression).thenCompose(this::toResultNode);
}

@Override
public CompletionStage<ResultNode> apply(Object result) {
CompletionStage<ResultNode> toResultNode(Object result) {
if (traceLevel) {
LOG.tracef("Resolve {%s} completed:%s", expression.toOriginalString(), expression.getOrigin());
}
if (result instanceof ResultNode) {
return CompletedStage.of((ResultNode) result);
} else if (result instanceof CompletionStage) {
return ((CompletionStage<?>) result).thenCompose(this);
} else if (result instanceof CompletableFuture) {
return (CompletableFuture<ResultNode>) ((CompletionStage<?>) result).thenCompose(this::toResultNode);
} else if (result instanceof CompletedStage) {
return (CompletableFuture<ResultNode>) ((CompletionStage<?>) result).thenCompose(this::toResultNode);
} else if (unrestrictedCompletionStages && result instanceof CompletionStage) {
return ((CompletionStage<?>) result).thenCompose(this::toResultNode);
} else {
return CompletedStage.of(new SingleResultNode(result, this));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
package io.quarkus.qute;

import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;
import java.util.function.Supplier;

final class LazyValue<T> {
public class LazyValue<T> {

private final Supplier<T> supplier;

private final Lock lock = new ReentrantLock();

private transient volatile T value;

public LazyValue(Supplier<T> supplier) {
Expand All @@ -17,11 +21,15 @@ public T get() {
if (valueCopy != null) {
return valueCopy;
}
synchronized (this) {

lock.lock();
try {
if (value == null) {
value = supplier.get();
}
return value;
} finally {
lock.unlock();
}
}

Expand All @@ -30,13 +38,13 @@ public T getIfPresent() {
}

public void clear() {
synchronized (this) {
value = null;
}
lock.lock();
value = null;
lock.unlock();
}

public boolean isSet() {
return value != null;
}

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
import static io.quarkus.qute.Parameter.EMPTY;

import java.lang.reflect.Array;
import java.util.AbstractCollection;
import java.util.AbstractMap;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
Expand Down Expand Up @@ -71,27 +73,31 @@ public CompletionStage<ResultNode> resolve(SectionResolutionContext context) {
}

private static int extractSize(Object it) {
if (it instanceof Collection) {
return ((Collection<?>) it).size();
} else if (it instanceof Map) {
return ((Map<?, ?>) it).size();
// Note that we intentionally use "instanceof" to test interfaces as the last resort in order to mitigate the "type pollution"
// See https://github.com/RedHatPerf/type-pollution-agent for more information
if (it instanceof AbstractCollection) {
return ((AbstractCollection<?>) it).size();
} else if (it instanceof AbstractMap) {
return ((AbstractMap<?, ?>) it).size();
} else if (it.getClass().isArray()) {
return Array.getLength(it);
} else if (it instanceof Integer) {
return ((Integer) it);
} else if (it instanceof Collection) {
return ((Collection<?>) it).size();
} else if (it instanceof Map) {
return ((Map<?, ?>) it).size();
}
return 10;
}

private Iterator<?> extractIterator(Object it) {
if (it instanceof Iterable) {
return ((Iterable<?>) it).iterator();
} else if (it instanceof Iterator) {
return (Iterator<?>) it;
} else if (it instanceof Map) {
return ((Map<?, ?>) it).entrySet().iterator();
} else if (it instanceof Stream) {
return ((Stream<?>) it).sequential().iterator();
// Note that we intentionally use "instanceof" to test interfaces as the last resort in order to mitigate the "type pollution"
// See https://github.com/RedHatPerf/type-pollution-agent for more information
if (it instanceof AbstractCollection) {
return ((AbstractCollection<?>) it).iterator();
} else if (it instanceof AbstractMap) {
return ((AbstractMap<?, ?>) it).entrySet().iterator();
} else if (it instanceof Integer) {
return IntStream.rangeClosed(1, (Integer) it).iterator();
} else if (it.getClass().isArray()) {
Expand All @@ -102,6 +108,14 @@ private Iterator<?> extractIterator(Object it) {
elements.add(Array.get(it, i));
}
return elements.iterator();
} else if (it instanceof Iterable) {
return ((Iterable<?>) it).iterator();
} else if (it instanceof Iterator) {
return (Iterator<?>) it;
} else if (it instanceof Map) {
return ((Map<?, ?>) it).entrySet().iterator();
} else if (it instanceof Stream) {
return ((Stream<?>) it).sequential().iterator();
} else {
TemplateException.Builder builder;
if (Results.isNotFound(it)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
/**
* A result node backed by an array of result nodes.
*/
public final class MultiResultNode implements ResultNode {
public final class MultiResultNode extends ResultNode {

private final Supplier<ResultNode>[] results;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
/**
* Node of a result tree.
*/
public interface ResultNode {
public abstract class ResultNode {

static CompletionStage<ResultNode> NOOP = CompletedStage.of(new ResultNode() {
public static CompletionStage<ResultNode> NOOP = CompletedStage.of(new ResultNode() {
@Override
public void process(Consumer<String> resultConsumer) {
}
Expand All @@ -18,6 +18,6 @@ public void process(Consumer<String> resultConsumer) {
*
* @param resultConsumer
*/
void process(Consumer<String> resultConsumer);
public abstract void process(Consumer<String> resultConsumer);

}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
/**
* A result node backed by a single object value.
*/
public final class SingleResultNode implements ResultNode {
public final class SingleResultNode extends ResultNode {

private final Object value;
private final ExpressionNode node;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
/**
* Static text.
*/
public class TextNode implements TemplateNode, ResultNode {
public class TextNode extends ResultNode implements TemplateNode {

private final CompletedStage<ResultNode> result;
private final String value;
Expand Down

0 comments on commit 2d33b47

Please sign in to comment.