From 72d938fc5452be061a2ed650b9c0f57de1f5aef2 Mon Sep 17 00:00:00 2001 From: Jack Smith Date: Tue, 3 Sep 2024 18:37:33 -0400 Subject: [PATCH 1/5] Add feature to ignore parse errors when doing nested interpretation --- .../hubspot/jinjava/interpret/Context.java | 33 +++++++++++++++++++ .../interpret/ContextConfigurationIF.java | 8 +++++ .../jinjava/interpret/JinjavaInterpreter.java | 16 ++++++++- .../com/hubspot/jinjava/tree/TreeParser.java | 25 ++++++++------ 4 files changed, 71 insertions(+), 11 deletions(-) diff --git a/src/main/java/com/hubspot/jinjava/interpret/Context.java b/src/main/java/com/hubspot/jinjava/interpret/Context.java index 1ea7aaf71..4aef74aec 100644 --- a/src/main/java/com/hubspot/jinjava/interpret/Context.java +++ b/src/main/java/com/hubspot/jinjava/interpret/Context.java @@ -819,6 +819,23 @@ public TemporaryValueClosable withUnwrapRawOverride() { return temporaryValueClosable; } + public boolean isIgnoreParseErrors() { + return contextConfiguration.isIgnoreParseErrors(); + } + + private void setIgnoreParseErrors(boolean ignoreParseErrors) { + contextConfiguration = contextConfiguration.withIgnoreParseErrors(ignoreParseErrors); + } + + public TemporaryValueClosable withIgnoreParseErrors() { + TemporaryValueClosable temporaryValueClosable = new TemporaryValueClosable<>( + isIgnoreParseErrors(), + this::setIgnoreParseErrors + ); + setIgnoreParseErrors(true); + return temporaryValueClosable; + } + public static class TemporaryValueClosable implements AutoCloseable { private final T previousValue; @@ -829,10 +846,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..b224ca262 100644 --- a/src/main/java/com/hubspot/jinjava/interpret/ContextConfigurationIF.java +++ b/src/main/java/com/hubspot/jinjava/interpret/ContextConfigurationIF.java @@ -47,4 +47,12 @@ default boolean isPartialMacroEvaluation() { default boolean isUnwrapRawOverride() { return false; } + + /** + * When trying nested interpretation, parsing errors are likely. This flag avoids propogating to differentiate between static and dynamic parsing errors. + */ + @Default + default boolean isIgnoreParseErrors() { + return false; + } } diff --git a/src/main/java/com/hubspot/jinjava/interpret/JinjavaInterpreter.java b/src/main/java/com/hubspot/jinjava/interpret/JinjavaInterpreter.java index 9f5a99ce9..89d1b0c48 100644 --- a/src/main/java/com/hubspot/jinjava/interpret/JinjavaInterpreter.java +++ b/src/main/java/com/hubspot/jinjava/interpret/JinjavaInterpreter.java @@ -29,6 +29,7 @@ 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.TemplateError.ErrorItem; import com.hubspot.jinjava.interpret.TemplateError.ErrorReason; import com.hubspot.jinjava.interpret.TemplateError.ErrorType; @@ -83,6 +84,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<>(); @@ -259,7 +262,7 @@ public String renderFlat(String template) { public String renderFlat(String template, long renderLimit) { int depth = context.getRenderDepth(); - try { + try (TemporaryValueClosable c = ignoreParseErrorsIfActivated()) { if (depth > config.getMaxRenderDepth()) { ENGINE_LOG.warn("Max render depth exceeded: {}", Integer.toString(depth)); return template; @@ -272,6 +275,17 @@ public String renderFlat(String template, long renderLimit) { } } + private TemporaryValueClosable ignoreParseErrorsIfActivated() { + return config + .getFeatures() + .getActivationStrategy( + JinjavaInterpreter.IGNORE_NESTED_INTERPRETATION_PARSE_ERRORS + ) + .isActive(context) + ? context.withIgnoreParseErrors() + : TemporaryValueClosable.noOp(); + } + /** * Parse the given string into a root Node, and then renders it processing extend parents. * diff --git a/src/main/java/com/hubspot/jinjava/tree/TreeParser.java b/src/main/java/com/hubspot/jinjava/tree/TreeParser.java index 8bdc29653..76d65a9d0 100644 --- a/src/main/java/com/hubspot/jinjava/tree/TreeParser.java +++ b/src/main/java/com/hubspot/jinjava/tree/TreeParser.java @@ -81,7 +81,7 @@ public Node buildTree() { do { if (parent != root) { - interpreter.addError( + maybeAddError( TemplateError.fromException( new MissingEndTagException( ((TagNode) parent).getEndName(), @@ -113,7 +113,7 @@ private Node nextNode() { if (token.getType() == symbols.getFixed()) { if (token instanceof UnclosedToken) { - interpreter.addError( + maybeAddError( new TemplateError( ErrorType.WARNING, ErrorReason.SYNTAX_ERROR, @@ -134,7 +134,7 @@ private Node nextNode() { } else if (token.getType() == symbols.getNote()) { String commentClosed = symbols.getClosingComment(); if (!token.getImage().endsWith(commentClosed)) { - interpreter.addError( + maybeAddError( new TemplateError( ErrorType.WARNING, ErrorReason.SYNTAX_ERROR, @@ -148,7 +148,7 @@ private Node nextNode() { ); } } else { - interpreter.addError( + maybeAddError( TemplateError.fromException( new UnexpectedTokenException( token.getImage(), @@ -237,13 +237,11 @@ private Node tag(TagToken tagToken) { try { tag = interpreter.getContext().getTag(tagToken.getTagName()); if (tag == null) { - interpreter.addError( - TemplateError.fromException(new UnknownTagException(tagToken)) - ); + maybeAddError(TemplateError.fromException(new UnknownTagException(tagToken))); return null; } } catch (DisabledException e) { - interpreter.addError( + maybeAddError( new TemplateError( ErrorType.FATAL, ErrorReason.DISABLED, @@ -292,7 +290,7 @@ private void endTag(Tag tag, TagToken tagToken) { hasMatchingStartTag = true; break; } else { - interpreter.addError( + maybeAddError( TemplateError.fromException( new TemplateSyntaxException( tagToken.getImage(), @@ -305,7 +303,7 @@ private void endTag(Tag tag, TagToken tagToken) { } } if (!hasMatchingStartTag) { - interpreter.addError( + maybeAddError( new TemplateError( ErrorType.WARNING, ErrorReason.SYNTAX_ERROR, @@ -326,4 +324,11 @@ private boolean isTrimmingEnabledForToken(Token token, JinjavaConfig jinjavaConf } return jinjavaConfig.getLegacyOverrides().isUseTrimmingForNotesAndExpressions(); } + + private void maybeAddError(TemplateError templateError) { + if (interpreter.getContext().isIgnoreParseErrors()) { + return; + } + interpreter.addError(templateError); + } } From 09cbfbae15278269dc7a44f7d978c1a53cfb6243 Mon Sep 17 00:00:00 2001 From: Jack Smith Date: Tue, 3 Sep 2024 18:42:31 -0400 Subject: [PATCH 2/5] Test that errors aren't added when parse errors are ignored --- .../com/hubspot/jinjava/tree/TreeParserTest.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/test/java/com/hubspot/jinjava/tree/TreeParserTest.java b/src/test/java/com/hubspot/jinjava/tree/TreeParserTest.java index b4279523a..27e995e22 100644 --- a/src/test/java/com/hubspot/jinjava/tree/TreeParserTest.java +++ b/src/test/java/com/hubspot/jinjava/tree/TreeParserTest.java @@ -7,6 +7,7 @@ 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.TemplateError.ErrorType; import java.nio.charset.StandardCharsets; import org.junit.Test; @@ -361,6 +362,20 @@ public void itDoesNotMergeAdjacentTextNodesWhenLegacyOverrideIsApplied() { assertThat(interpreter.render(overriddenTree)).isEqualTo("A\nB"); } + @Test + public void itDoesNotAddErrorWhenParseErrorsAreIgnored() { + try ( + TemporaryValueClosable c = interpreter.getContext().withIgnoreParseErrors() + ) { + 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( From 4e144d86b6874e2236a0dbb575b6c2708ef6f71b Mon Sep 17 00:00:00 2001 From: Jack Smith Date: Tue, 3 Sep 2024 18:47:55 -0400 Subject: [PATCH 3/5] Test that parse errors are ignored when doing nested interpretation --- .../jinjava/tree/ExpressionNodeTest.java | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) 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(); } From f5975fa69bd24d9beb5343e1e469bfd882083d47 Mon Sep 17 00:00:00 2001 From: Jack Smith Date: Tue, 3 Sep 2024 19:36:04 -0400 Subject: [PATCH 4/5] Replace isIgnoreParseErrors and isThrowInterpreterErrors with ErrorHandlingStrategy --- .../hubspot/jinjava/interpret/Context.java | 73 +++++++++++---- .../interpret/ContextConfigurationIF.java | 46 +++++++--- .../jinjava/interpret/JinjavaInterpreter.java | 88 +++++++++++-------- .../expression/EagerExpressionStrategy.java | 33 +++---- .../com/hubspot/jinjava/tree/TreeParser.java | 25 +++--- .../jinjava/util/EagerExpressionResolver.java | 21 +++-- .../hubspot/jinjava/tree/TreeParserTest.java | 6 +- 7 files changed, 182 insertions(+), 110 deletions(-) diff --git a/src/main/java/com/hubspot/jinjava/interpret/Context.java b/src/main/java/com/hubspot/jinjava/interpret/Context.java index 4aef74aec..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() { @@ -819,23 +871,6 @@ public TemporaryValueClosable withUnwrapRawOverride() { return temporaryValueClosable; } - public boolean isIgnoreParseErrors() { - return contextConfiguration.isIgnoreParseErrors(); - } - - private void setIgnoreParseErrors(boolean ignoreParseErrors) { - contextConfiguration = contextConfiguration.withIgnoreParseErrors(ignoreParseErrors); - } - - public TemporaryValueClosable withIgnoreParseErrors() { - TemporaryValueClosable temporaryValueClosable = new TemporaryValueClosable<>( - isIgnoreParseErrors(), - this::setIgnoreParseErrors - ); - setIgnoreParseErrors(true); - return temporaryValueClosable; - } - public static class TemporaryValueClosable implements AutoCloseable { private final T previousValue; diff --git a/src/main/java/com/hubspot/jinjava/interpret/ContextConfigurationIF.java b/src/main/java/com/hubspot/jinjava/interpret/ContextConfigurationIF.java index b224ca262..daefd5981 100644 --- a/src/main/java/com/hubspot/jinjava/interpret/ContextConfigurationIF.java +++ b/src/main/java/com/hubspot/jinjava/interpret/ContextConfigurationIF.java @@ -33,11 +33,6 @@ default boolean isDeferLargeObjects() { return false; } - @Default - default boolean isThrowInterpreterErrors() { - return false; - } - @Default default boolean isPartialMacroEvaluation() { return false; @@ -48,11 +43,42 @@ default boolean isUnwrapRawOverride() { return false; } - /** - * When trying nested interpretation, parsing errors are likely. This flag avoids propogating to differentiate between static and dynamic parsing errors. - */ @Default - default boolean isIgnoreParseErrors() { - 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 89d1b0c48..ccfac1bdc 100644 --- a/src/main/java/com/hubspot/jinjava/interpret/JinjavaInterpreter.java +++ b/src/main/java/com/hubspot/jinjava/interpret/JinjavaInterpreter.java @@ -30,6 +30,8 @@ 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; @@ -262,27 +264,33 @@ public String renderFlat(String template) { public String renderFlat(String template, long renderLimit) { int depth = context.getRenderDepth(); - try (TemporaryValueClosable c = ignoreParseErrorsIfActivated()) { + try { if (depth > config.getMaxRenderDepth()) { ENGINE_LOG.warn("Max render depth exceeded: {}", Integer.toString(depth)); 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() { + private TemporaryValueClosable ignoreParseErrorsIfActivated() { return config .getFeatures() .getActivationStrategy( JinjavaInterpreter.IGNORE_NESTED_INTERPRETATION_PARSE_ERRORS ) .isActive(context) - ? context.withIgnoreParseErrors() + ? context.withErrorHandlingStrategy(ErrorHandlingStrategyIF.ignoreAll()) : TemporaryValueClosable.noOp(); } @@ -826,46 +834,50 @@ 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: + // 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/tree/TreeParser.java b/src/main/java/com/hubspot/jinjava/tree/TreeParser.java index 76d65a9d0..8bdc29653 100644 --- a/src/main/java/com/hubspot/jinjava/tree/TreeParser.java +++ b/src/main/java/com/hubspot/jinjava/tree/TreeParser.java @@ -81,7 +81,7 @@ public Node buildTree() { do { if (parent != root) { - maybeAddError( + interpreter.addError( TemplateError.fromException( new MissingEndTagException( ((TagNode) parent).getEndName(), @@ -113,7 +113,7 @@ private Node nextNode() { if (token.getType() == symbols.getFixed()) { if (token instanceof UnclosedToken) { - maybeAddError( + interpreter.addError( new TemplateError( ErrorType.WARNING, ErrorReason.SYNTAX_ERROR, @@ -134,7 +134,7 @@ private Node nextNode() { } else if (token.getType() == symbols.getNote()) { String commentClosed = symbols.getClosingComment(); if (!token.getImage().endsWith(commentClosed)) { - maybeAddError( + interpreter.addError( new TemplateError( ErrorType.WARNING, ErrorReason.SYNTAX_ERROR, @@ -148,7 +148,7 @@ private Node nextNode() { ); } } else { - maybeAddError( + interpreter.addError( TemplateError.fromException( new UnexpectedTokenException( token.getImage(), @@ -237,11 +237,13 @@ private Node tag(TagToken tagToken) { try { tag = interpreter.getContext().getTag(tagToken.getTagName()); if (tag == null) { - maybeAddError(TemplateError.fromException(new UnknownTagException(tagToken))); + interpreter.addError( + TemplateError.fromException(new UnknownTagException(tagToken)) + ); return null; } } catch (DisabledException e) { - maybeAddError( + interpreter.addError( new TemplateError( ErrorType.FATAL, ErrorReason.DISABLED, @@ -290,7 +292,7 @@ private void endTag(Tag tag, TagToken tagToken) { hasMatchingStartTag = true; break; } else { - maybeAddError( + interpreter.addError( TemplateError.fromException( new TemplateSyntaxException( tagToken.getImage(), @@ -303,7 +305,7 @@ private void endTag(Tag tag, TagToken tagToken) { } } if (!hasMatchingStartTag) { - maybeAddError( + interpreter.addError( new TemplateError( ErrorType.WARNING, ErrorReason.SYNTAX_ERROR, @@ -324,11 +326,4 @@ private boolean isTrimmingEnabledForToken(Token token, JinjavaConfig jinjavaConf } return jinjavaConfig.getLegacyOverrides().isUseTrimmingForNotesAndExpressions(); } - - private void maybeAddError(TemplateError templateError) { - if (interpreter.getContext().isIgnoreParseErrors()) { - return; - } - interpreter.addError(templateError); - } } 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/TreeParserTest.java b/src/test/java/com/hubspot/jinjava/tree/TreeParserTest.java index 27e995e22..d1522b23b 100644 --- a/src/test/java/com/hubspot/jinjava/tree/TreeParserTest.java +++ b/src/test/java/com/hubspot/jinjava/tree/TreeParserTest.java @@ -8,6 +8,8 @@ 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; @@ -365,7 +367,9 @@ public void itDoesNotMergeAdjacentTextNodesWhenLegacyOverrideIsApplied() { @Test public void itDoesNotAddErrorWhenParseErrorsAreIgnored() { try ( - TemporaryValueClosable c = interpreter.getContext().withIgnoreParseErrors() + TemporaryValueClosable c = interpreter + .getContext() + .withErrorHandlingStrategy(ErrorHandlingStrategyIF.ignoreAll()) ) { String expression = "{% if "; final Node tree = new TreeParser(interpreter, expression).buildTree(); From 447fcfddb1d494d822c8af9d693dee4b7361557b Mon Sep 17 00:00:00 2001 From: Jack Smith Date: Tue, 3 Sep 2024 19:38:09 -0400 Subject: [PATCH 5/5] Add default branch for checkstyle --- .../java/com/hubspot/jinjava/interpret/JinjavaInterpreter.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/com/hubspot/jinjava/interpret/JinjavaInterpreter.java b/src/main/java/com/hubspot/jinjava/interpret/JinjavaInterpreter.java index ccfac1bdc..843eb0c49 100644 --- a/src/main/java/com/hubspot/jinjava/interpret/JinjavaInterpreter.java +++ b/src/main/java/com/hubspot/jinjava/interpret/JinjavaInterpreter.java @@ -849,6 +849,7 @@ public void addError(TemplateError templateError) { templateError.getMessage() ); case ADD_ERROR: + default: // Checkstyle // fix line numbers not matching up with source template if (!context.getCurrentPathStack().isEmpty()) { if (