diff --git a/src/main/java/com/hubspot/jinjava/interpret/Context.java b/src/main/java/com/hubspot/jinjava/interpret/Context.java index 1ea7aaf71..e99b8a84a 100644 --- a/src/main/java/com/hubspot/jinjava/interpret/Context.java +++ b/src/main/java/com/hubspot/jinjava/interpret/Context.java @@ -21,6 +21,7 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.SetMultimap; import com.google.common.collect.Sets; +import com.hubspot.jinjava.interpret.ContextConfigurationIF.ErrorHandlingStrategyIF.TemplateErrorTypeHandlingStrategy; import com.hubspot.jinjava.lib.Importable; import com.hubspot.jinjava.lib.expression.ExpressionStrategy; import com.hubspot.jinjava.lib.exptest.ExpTest; @@ -769,13 +770,64 @@ public TemporaryValueClosable withDeferLargeObjects( return temporaryValueClosable; } + @Deprecated public boolean getThrowInterpreterErrors() { - return contextConfiguration.isThrowInterpreterErrors(); + ErrorHandlingStrategy errorHandlingStrategy = getErrorHandlingStrategy(); + return ( + errorHandlingStrategy.getFatalErrorStrategy() == + TemplateErrorTypeHandlingStrategy.THROW_EXCEPTION + ); } + @Deprecated public void setThrowInterpreterErrors(boolean throwInterpreterErrors) { contextConfiguration = - contextConfiguration.withThrowInterpreterErrors(throwInterpreterErrors); + contextConfiguration.withErrorHandlingStrategy( + ErrorHandlingStrategy + .builder() + .setFatalErrorStrategy( + throwInterpreterErrors + ? TemplateErrorTypeHandlingStrategy.THROW_EXCEPTION + : TemplateErrorTypeHandlingStrategy.ADD_ERROR + ) + .setNonFatalErrorStrategy( + throwInterpreterErrors + ? TemplateErrorTypeHandlingStrategy.IGNORE // Deprecated, warnings are ignored when doing eager expression resolving + : TemplateErrorTypeHandlingStrategy.ADD_ERROR + ) + .build() + ); + } + + @Deprecated + public TemporaryValueClosable withThrowInterpreterErrors() { + TemporaryValueClosable temporaryValueClosable = new TemporaryValueClosable<>( + getThrowInterpreterErrors(), + this::setThrowInterpreterErrors + ); + setThrowInterpreterErrors(true); + return temporaryValueClosable; + } + + public ErrorHandlingStrategy getErrorHandlingStrategy() { + return contextConfiguration.getErrorHandlingStrategy(); + } + + public void setErrorHandlingStrategy(ErrorHandlingStrategy errorHandlingStrategy) { + contextConfiguration = + contextConfiguration.withErrorHandlingStrategy(errorHandlingStrategy); + } + + public TemporaryValueClosable withErrorHandlingStrategy( + ErrorHandlingStrategy errorHandlingStrategy + ) { + TemporaryValueClosable temporaryValueClosable = + new TemporaryValueClosable<>( + getErrorHandlingStrategy(), + this::setErrorHandlingStrategy + ); + setErrorHandlingStrategy(errorHandlingStrategy); + return temporaryValueClosable; } public boolean isPartialMacroEvaluation() { @@ -829,10 +881,26 @@ private TemporaryValueClosable(T previousValue, Consumer resetValueConsumer) this.resetValueConsumer = resetValueConsumer; } + public static TemporaryValueClosable noOp() { + return new NoOpTemporaryValueClosable<>(); + } + @Override public void close() { resetValueConsumer.accept(previousValue); } + + private static class NoOpTemporaryValueClosable extends TemporaryValueClosable { + + private NoOpTemporaryValueClosable() { + super(null, null); + } + + @Override + public void close() { + // No-op + } + } } public Node getCurrentNode() { diff --git a/src/main/java/com/hubspot/jinjava/interpret/ContextConfigurationIF.java b/src/main/java/com/hubspot/jinjava/interpret/ContextConfigurationIF.java index 5aac82fa8..daefd5981 100644 --- a/src/main/java/com/hubspot/jinjava/interpret/ContextConfigurationIF.java +++ b/src/main/java/com/hubspot/jinjava/interpret/ContextConfigurationIF.java @@ -34,17 +34,51 @@ default boolean isDeferLargeObjects() { } @Default - default boolean isThrowInterpreterErrors() { + default boolean isPartialMacroEvaluation() { return false; } @Default - default boolean isPartialMacroEvaluation() { + default boolean isUnwrapRawOverride() { return false; } @Default - default boolean isUnwrapRawOverride() { - return false; + default ErrorHandlingStrategy getErrorHandlingStrategy() { + return ErrorHandlingStrategy.of(); + } + + @Immutable(singleton = true) + @HubSpotImmutableStyle + interface ErrorHandlingStrategyIF { + @Default + default TemplateErrorTypeHandlingStrategy getFatalErrorStrategy() { + return TemplateErrorTypeHandlingStrategy.ADD_ERROR; + } + + @Default + default TemplateErrorTypeHandlingStrategy getNonFatalErrorStrategy() { + return TemplateErrorTypeHandlingStrategy.ADD_ERROR; + } + + enum TemplateErrorTypeHandlingStrategy { + IGNORE, + ADD_ERROR, + THROW_EXCEPTION, + } + + static ErrorHandlingStrategy throwAll() { + return ErrorHandlingStrategy + .of() + .withFatalErrorStrategy(TemplateErrorTypeHandlingStrategy.THROW_EXCEPTION) + .withNonFatalErrorStrategy(TemplateErrorTypeHandlingStrategy.THROW_EXCEPTION); + } + + static ErrorHandlingStrategy ignoreAll() { + return ErrorHandlingStrategy + .of() + .withFatalErrorStrategy(TemplateErrorTypeHandlingStrategy.IGNORE) + .withNonFatalErrorStrategy(TemplateErrorTypeHandlingStrategy.IGNORE); + } } } diff --git a/src/main/java/com/hubspot/jinjava/interpret/JinjavaInterpreter.java b/src/main/java/com/hubspot/jinjava/interpret/JinjavaInterpreter.java index 9f5a99ce9..843eb0c49 100644 --- a/src/main/java/com/hubspot/jinjava/interpret/JinjavaInterpreter.java +++ b/src/main/java/com/hubspot/jinjava/interpret/JinjavaInterpreter.java @@ -29,6 +29,9 @@ import com.hubspot.jinjava.el.ExpressionResolver; import com.hubspot.jinjava.el.ext.DeferredParsingException; import com.hubspot.jinjava.el.ext.ExtendedParser; +import com.hubspot.jinjava.interpret.Context.TemporaryValueClosable; +import com.hubspot.jinjava.interpret.ContextConfigurationIF.ErrorHandlingStrategyIF; +import com.hubspot.jinjava.interpret.ContextConfigurationIF.ErrorHandlingStrategyIF.TemplateErrorTypeHandlingStrategy; import com.hubspot.jinjava.interpret.TemplateError.ErrorItem; import com.hubspot.jinjava.interpret.TemplateError.ErrorReason; import com.hubspot.jinjava.interpret.TemplateError.ErrorType; @@ -83,6 +86,8 @@ public class JinjavaInterpreter implements PyishSerializable { public static final String OUTPUT_UNDEFINED_VARIABLES_ERROR = "OUTPUT_UNDEFINED_VARIABLES_ERROR"; + public static final String IGNORE_NESTED_INTERPRETATION_PARSE_ERRORS = + "IGNORE_NESTED_INTERPRETATION_PARSE_ERRORS"; private final Multimap blocks = ArrayListMultimap.create(); private final LinkedList extendParentRoots = new LinkedList<>(); private final Map revertibleObjects = new HashMap<>(); @@ -265,13 +270,30 @@ public String renderFlat(String template, long renderLimit) { return template; } else { context.setRenderDepth(depth + 1); - return render(parse(template), false, renderLimit); + Node parsedNode; + try ( + TemporaryValueClosable c = ignoreParseErrorsIfActivated() + ) { + parsedNode = parse(template); + } + return render(parsedNode, false, renderLimit); } } finally { context.setRenderDepth(depth); } } + private TemporaryValueClosable ignoreParseErrorsIfActivated() { + return config + .getFeatures() + .getActivationStrategy( + JinjavaInterpreter.IGNORE_NESTED_INTERPRETATION_PARSE_ERRORS + ) + .isActive(context) + ? context.withErrorHandlingStrategy(ErrorHandlingStrategyIF.ignoreAll()) + : TemporaryValueClosable.noOp(); + } + /** * Parse the given string into a root Node, and then renders it processing extend parents. * @@ -812,46 +834,51 @@ public void addError(TemplateError templateError) { if (templateError == null) { return; } - - if (context.getThrowInterpreterErrors()) { - if (templateError.getSeverity() == ErrorType.FATAL) { - // Throw fatal errors when locating deferred words. + ErrorHandlingStrategy errorHandlingStrategy = context.getErrorHandlingStrategy(); + TemplateErrorTypeHandlingStrategy errorTypeHandlingStrategy = + templateError.getSeverity() == ErrorType.FATAL + ? errorHandlingStrategy.getFatalErrorStrategy() + : errorHandlingStrategy.getNonFatalErrorStrategy(); + switch (errorTypeHandlingStrategy) { + case IGNORE: + return; + case THROW_EXCEPTION: throw new TemplateSyntaxException( this, templateError.getFieldName(), templateError.getMessage() ); - } else { - // Hide warning errors when locating deferred words. - return; - } - } - // fix line numbers not matching up with source template - if (!context.getCurrentPathStack().isEmpty()) { - if ( - !templateError.getSourceTemplate().isPresent() && - context.getCurrentPathStack().peek().isPresent() - ) { - templateError.setMessage( - getWrappedErrorMessage( - context.getCurrentPathStack().peek().get(), - templateError - ) - ); - templateError.setSourceTemplate(context.getCurrentPathStack().peek().get()); - } - templateError.setStartPosition(context.getCurrentPathStack().getTopStartPosition()); - templateError.setLineno(context.getCurrentPathStack().getTopLineNumber()); - } + case ADD_ERROR: + default: // Checkstyle + // fix line numbers not matching up with source template + if (!context.getCurrentPathStack().isEmpty()) { + if ( + !templateError.getSourceTemplate().isPresent() && + context.getCurrentPathStack().peek().isPresent() + ) { + templateError.setMessage( + getWrappedErrorMessage( + context.getCurrentPathStack().peek().get(), + templateError + ) + ); + templateError.setSourceTemplate(context.getCurrentPathStack().peek().get()); + } + templateError.setStartPosition( + context.getCurrentPathStack().getTopStartPosition() + ); + templateError.setLineno(context.getCurrentPathStack().getTopLineNumber()); + } - // Limit the number of errors and filter duplicates - if (errors.size() < MAX_ERROR_SIZE) { - templateError = templateError.withScopeDepth(scopeDepth); - int errorCode = templateError.hashCode(); - if (!errorSet.contains(errorCode)) { - this.errors.add(templateError); - this.errorSet.add(errorCode); - } + // Limit the number of errors and filter duplicates + if (errors.size() < MAX_ERROR_SIZE) { + templateError = templateError.withScopeDepth(scopeDepth); + int errorCode = templateError.hashCode(); + if (!errorSet.contains(errorCode)) { + this.errors.add(templateError); + this.errorSet.add(errorCode); + } + } } } diff --git a/src/main/java/com/hubspot/jinjava/lib/expression/EagerExpressionStrategy.java b/src/main/java/com/hubspot/jinjava/lib/expression/EagerExpressionStrategy.java index 4ba01ac6d..c1f6d15b0 100644 --- a/src/main/java/com/hubspot/jinjava/lib/expression/EagerExpressionStrategy.java +++ b/src/main/java/com/hubspot/jinjava/lib/expression/EagerExpressionStrategy.java @@ -2,8 +2,11 @@ import com.google.common.annotations.Beta; import com.hubspot.jinjava.JinjavaConfig; +import com.hubspot.jinjava.interpret.Context.TemporaryValueClosable; +import com.hubspot.jinjava.interpret.ContextConfigurationIF.ErrorHandlingStrategyIF; +import com.hubspot.jinjava.interpret.ErrorHandlingStrategy; import com.hubspot.jinjava.interpret.JinjavaInterpreter; -import com.hubspot.jinjava.interpret.TemplateError.ErrorReason; +import com.hubspot.jinjava.interpret.TemplateSyntaxException; import com.hubspot.jinjava.lib.filter.EscapeFilter; import com.hubspot.jinjava.lib.tag.eager.DeferredToken; import com.hubspot.jinjava.lib.tag.eager.EagerExecutionResult; @@ -14,7 +17,6 @@ import com.hubspot.jinjava.util.EagerReconstructionUtils; import com.hubspot.jinjava.util.Logging; import com.hubspot.jinjava.util.PrefixToPreserveState; -import java.util.Objects; import org.apache.commons.lang3.StringUtils; @Beta @@ -103,17 +105,20 @@ public static String postProcessResult( StringUtils.contains(result, master.getSymbols().getExpressionStartWithTag())) ) { if (interpreter.getConfig().isNestedInterpretationEnabled()) { - long errorSizeStart = getParsingErrorsCount(interpreter); - - interpreter.parse(result); - - if (getParsingErrorsCount(interpreter) == errorSizeStart) { + try { + try ( + TemporaryValueClosable c = interpreter + .getContext() + .withErrorHandlingStrategy(ErrorHandlingStrategyIF.throwAll()) + ) { + interpreter.parse(result); + } try { result = interpreter.renderFlat(result); } catch (Exception e) { Logging.ENGINE_LOG.warn("Error rendering variable node result", e); } - } + } catch (TemplateSyntaxException ignored) {} } else { result = EagerReconstructionUtils.wrapInRawIfNeeded(result, interpreter); } @@ -125,18 +130,6 @@ public static String postProcessResult( return result; } - private static long getParsingErrorsCount(JinjavaInterpreter interpreter) { - return interpreter - .getErrors() - .stream() - .filter(Objects::nonNull) - .filter(error -> - "Unclosed comment".equals(error.getMessage()) || - error.getReason() == ErrorReason.DISABLED - ) - .count(); - } - private static String wrapInExpression(String output, JinjavaInterpreter interpreter) { JinjavaConfig config = interpreter.getConfig(); return String.format( diff --git a/src/main/java/com/hubspot/jinjava/util/EagerExpressionResolver.java b/src/main/java/com/hubspot/jinjava/util/EagerExpressionResolver.java index 29278252f..cc97811ff 100644 --- a/src/main/java/com/hubspot/jinjava/util/EagerExpressionResolver.java +++ b/src/main/java/com/hubspot/jinjava/util/EagerExpressionResolver.java @@ -5,7 +5,10 @@ import com.google.common.primitives.Primitives; import com.hubspot.jinjava.el.ext.DeferredParsingException; import com.hubspot.jinjava.el.ext.ExtendedParser; +import com.hubspot.jinjava.interpret.Context.TemporaryValueClosable; +import com.hubspot.jinjava.interpret.ContextConfigurationIF.ErrorHandlingStrategyIF.TemplateErrorTypeHandlingStrategy; import com.hubspot.jinjava.interpret.DeferredValueException; +import com.hubspot.jinjava.interpret.ErrorHandlingStrategy; import com.hubspot.jinjava.interpret.JinjavaInterpreter; import com.hubspot.jinjava.interpret.OutputTooBigException; import com.hubspot.jinjava.interpret.PartiallyDeferredValue; @@ -122,12 +125,18 @@ public static Set findDeferredWords( boolean nestedInterpretationEnabled = interpreter .getConfig() .isNestedInterpretationEnabled(); - boolean throwInterpreterErrorsStart = interpreter - .getContext() - .getThrowInterpreterErrors(); FoundQuotedExpressionTags foundQuotedExpressionTags = new FoundQuotedExpressionTags(); - try { - interpreter.getContext().setThrowInterpreterErrors(true); + try ( + TemporaryValueClosable closable = interpreter + .getContext() + .withErrorHandlingStrategy( + ErrorHandlingStrategy + .builder() + .setFatalErrorStrategy(TemplateErrorTypeHandlingStrategy.THROW_EXCEPTION) + .setNonFatalErrorStrategy(TemplateErrorTypeHandlingStrategy.IGNORE) + .build() + ) + ) { Set words = new HashSet<>(); char[] value = partiallyResolved.toCharArray(); int prevQuotePos = -1; @@ -184,8 +193,6 @@ public static Set findDeferredWords( ); } return words; - } finally { - interpreter.getContext().setThrowInterpreterErrors(throwInterpreterErrorsStart); } } diff --git a/src/test/java/com/hubspot/jinjava/tree/ExpressionNodeTest.java b/src/test/java/com/hubspot/jinjava/tree/ExpressionNodeTest.java index 12a24c04c..23b482228 100644 --- a/src/test/java/com/hubspot/jinjava/tree/ExpressionNodeTest.java +++ b/src/test/java/com/hubspot/jinjava/tree/ExpressionNodeTest.java @@ -236,6 +236,28 @@ public void itEscapesValueWhenContextSet() throws Exception { assertThat(val("{{ a }}")).isEqualTo("foo < bar"); } + @Test + public void itIgnoresParseErrorsWhenFeatureIsEnabled() { + final JinjavaConfig config = JinjavaConfig + .newBuilder() + .withFeatureConfig( + FeatureConfig + .newBuilder() + .add(JinjavaInterpreter.IGNORE_NESTED_INTERPRETATION_PARSE_ERRORS, c -> true) + .build() + ) + .build(); + JinjavaInterpreter interpreter = new Jinjava(config).newInterpreter(); + Context context = interpreter.getContext(); + context.put("myvar", "hello {% if"); + context.put("place", "world"); + + ExpressionNode node = fixture("simplevar"); + + assertThat(node.render(interpreter).toString()).isEqualTo("hello {% if"); + assertThat(interpreter.getErrors()).isEmpty(); + } + private String val(String jinja) { return parse(jinja).render(interpreter).getValue(); } diff --git a/src/test/java/com/hubspot/jinjava/tree/TreeParserTest.java b/src/test/java/com/hubspot/jinjava/tree/TreeParserTest.java index b4279523a..d1522b23b 100644 --- a/src/test/java/com/hubspot/jinjava/tree/TreeParserTest.java +++ b/src/test/java/com/hubspot/jinjava/tree/TreeParserTest.java @@ -7,6 +7,9 @@ import com.hubspot.jinjava.Jinjava; import com.hubspot.jinjava.JinjavaConfig; import com.hubspot.jinjava.LegacyOverrides; +import com.hubspot.jinjava.interpret.Context.TemporaryValueClosable; +import com.hubspot.jinjava.interpret.ContextConfigurationIF.ErrorHandlingStrategyIF; +import com.hubspot.jinjava.interpret.ErrorHandlingStrategy; import com.hubspot.jinjava.interpret.TemplateError.ErrorType; import java.nio.charset.StandardCharsets; import org.junit.Test; @@ -361,6 +364,22 @@ public void itDoesNotMergeAdjacentTextNodesWhenLegacyOverrideIsApplied() { assertThat(interpreter.render(overriddenTree)).isEqualTo("A\nB"); } + @Test + public void itDoesNotAddErrorWhenParseErrorsAreIgnored() { + try ( + TemporaryValueClosable c = interpreter + .getContext() + .withErrorHandlingStrategy(ErrorHandlingStrategyIF.ignoreAll()) + ) { + String expression = "{% if "; + final Node tree = new TreeParser(interpreter, expression).buildTree(); + assertThat(tree.getChildren()).hasSize(1); + assertThat(tree.getChildren().get(0).toTreeString()) + .isEqualToIgnoringWhitespace(" {~ {% if ~}"); + } + assertThat(interpreter.getErrors()).isEmpty(); + } + Node parse(String fixture) { try { return new TreeParser(