From e3d1669be6392d6e8aa44f26de90e04f64a37de0 Mon Sep 17 00:00:00 2001 From: jasmith-hs Date: Wed, 4 Mar 2026 14:49:27 -0500 Subject: [PATCH 1/8] Begin making 3.0 adjustments Update some tests Implement allowlisting for methods and classes Extract separate MethodValidator Add result validating Use string-based config. Make the allowlists easier to work with Wrap classes before validating result Split MethodValidator from ReturnTypeValidator Don't create new JinjavaBeanELResolver instances per Jinjava or JinjavaConfig for bean cache performance Validate ReturnTypes at the top-level so that accessing values of arrays, lists, maps have their return values validated Wrap at a higher level and don't resolve `____int3rpr3t3r____` Use BeanMethods and cache allowed methods and return types Don't expect ____int3rpr3t3r____ and don't use arrays in ReverseFilter and add method and return type validator to test classes Helper for constructing JinjavaConfig in tests Extract test objects to separate classes. Allow arrays. Add AnnotationIntrospector Don't need annotation introspector All tests passing Fix build Describe a couple more changes Don't allow jinjava constructs outside of the AllowlistGroup classes from being allowlisted Add tests that certain classes and packages CANNOT be allowlisted Allow BigInteger Add annotation to MethodValidator Test that certain constructs are fully banned Remove unused method from ReturnTypeValidator Use Map and Set instead of ImmutableMap and ImmutableSet for less churn Apply spotless formatting Fix test after merge conflicts Remove 3.0-changes file --- .../com/hubspot/jinjava/JinjavaConfig.java | 14 +- .../jinjava/el/ExpressionResolver.java | 18 +- .../hubspot/jinjava/el/HasInterpreter.java | 7 + .../hubspot/jinjava/el/JinjavaELContext.java | 8 +- .../el/JinjavaInterpreterResolver.java | 10 +- .../hubspot/jinjava/el/NoInvokeELContext.java | 10 +- ...eValidatingJinjavaInterpreterResolver.java | 73 +++++ .../jinjava/el/ext/AllowlistGroup.java | 206 ++++++++++++++ .../el/ext/AllowlistMethodValidator.java | 76 +++++ .../el/ext/AllowlistReturnTypeValidator.java | 73 +++++ .../com/hubspot/jinjava/el/ext/AstDict.java | 6 +- .../jinjava/el/ext/AstFilterChain.java | 5 +- .../com/hubspot/jinjava/el/ext/AstList.java | 5 +- .../jinjava/el/ext/AstMacroFunction.java | 5 +- .../jinjava/el/ext/AstRangeBracket.java | 5 +- .../el/ext/BannedAllowlistOptions.java | 66 +++++ .../jinjava/el/ext/BeanELResolver.java | 8 +- .../jinjava/el/ext/ExtendedParser.java | 2 +- .../jinjava/el/ext/JinjavaBeanELResolver.java | 260 +++++++++--------- .../jinjava/el/ext/MethodValidator.java | 10 + .../jinjava/el/ext/MethodValidatorConfig.java | 77 ++++++ .../jinjava/el/ext/ReturnTypeValidator.java | 8 + .../el/ext/ReturnTypeValidatorConfig.java | 76 +++++ .../jinjava/el/ext/eager/EagerAstDict.java | 6 +- .../el/ext/eager/EagerAstMacroFunction.java | 8 +- .../el/ext/eager/EagerAstParameters.java | 8 +- .../el/ext/eager/EvalResultHolder.java | 11 +- .../hubspot/jinjava/interpret/Context.java | 24 +- .../jinjava/lib/filter/RenderFilter.java | 6 +- .../jinjava/lib/filter/ReverseFilter.java | 54 ++-- .../jinjava/lib/tag/eager/EagerCycleTag.java | 9 +- .../hubspot/jinjava/objects/DummyObject.java | 15 +- .../objects/collections/ArrayBacked.java | 5 + .../jinjava/objects/collections/PySet.java | 38 +++ .../collections/SizeLimitingPySet.java | 68 +++++ .../jinjava/util/EagerExpressionResolver.java | 4 + .../util/EagerReconstructionUtils.java | 44 ++- .../com/hubspot/jinjava/BaseJinjavaTest.java | 41 ++- .../java/com/hubspot/jinjava/EagerTest.java | 12 +- .../hubspot/jinjava/FilterOverrideTest.java | 2 +- .../jinjava/el/ExpressionResolverTest.java | 44 +-- .../hubspot/jinjava/el/ext/AstDictTest.java | 104 ++++--- .../jinjava/el/ext/ExtendedParserTest.java | 21 +- .../el/ext/JinjavaBeanELResolverTest.java | 255 +++++++---------- .../ValidatorConfigBannedConstructsTest.java | 197 +++++++++++++ .../jinjava/el/ext/eager/EagerAstDotTest.java | 21 +- .../el/ext/eager/EagerAstMethodTest.java | 4 +- .../interpret/JinjavaInterpreterTest.java | 131 +++++---- .../lib/filter/time/FormatDateFilterTest.java | 2 +- .../hubspot/jinjava/lib/tag/IfTagTest.java | 1 - .../jinjava/lib/tag/ImportTagTest.java | 4 +- .../jinjava/lib/tag/ValidationModeTest.java | 3 +- .../lib/tag/eager/EagerImportTagTest.java | 8 +- .../lib/tag/eager/EagerSetTagTest.java | 8 +- .../objects/collections/PyMapTest.java | 21 +- .../hubspot/jinjava/tree/TreeParserTest.java | 24 +- .../util/EagerExpressionResolverTest.java | 7 +- .../test.expected.jinja | 4 +- .../defers-macro-in-for/test.expected.jinja | 2 +- .../defers-macro-in-if/test.expected.jinja | 2 +- .../test.expected.jinja | 14 +- .../test.expected.jinja | 4 +- .../test.expected.jinja | 12 +- .../test.expected.jinja | 6 +- .../test.expected.jinja | 12 +- .../test.expected.jinja | 4 +- .../test.expected.jinja | 2 +- .../test.expected.jinja | 2 +- .../test.expected.jinja | 2 +- .../test.expected.jinja | 4 +- 70 files changed, 1680 insertions(+), 628 deletions(-) create mode 100644 src/main/java/com/hubspot/jinjava/el/HasInterpreter.java create mode 100644 src/main/java/com/hubspot/jinjava/el/ReturnTypeValidatingJinjavaInterpreterResolver.java create mode 100644 src/main/java/com/hubspot/jinjava/el/ext/AllowlistGroup.java create mode 100644 src/main/java/com/hubspot/jinjava/el/ext/AllowlistMethodValidator.java create mode 100644 src/main/java/com/hubspot/jinjava/el/ext/AllowlistReturnTypeValidator.java create mode 100644 src/main/java/com/hubspot/jinjava/el/ext/BannedAllowlistOptions.java create mode 100644 src/main/java/com/hubspot/jinjava/el/ext/MethodValidator.java create mode 100644 src/main/java/com/hubspot/jinjava/el/ext/MethodValidatorConfig.java create mode 100644 src/main/java/com/hubspot/jinjava/el/ext/ReturnTypeValidator.java create mode 100644 src/main/java/com/hubspot/jinjava/el/ext/ReturnTypeValidatorConfig.java create mode 100644 src/main/java/com/hubspot/jinjava/objects/collections/ArrayBacked.java create mode 100644 src/main/java/com/hubspot/jinjava/objects/collections/PySet.java create mode 100644 src/main/java/com/hubspot/jinjava/objects/collections/SizeLimitingPySet.java create mode 100644 src/test/java/com/hubspot/jinjava/el/ext/ValidatorConfigBannedConstructsTest.java diff --git a/src/main/java/com/hubspot/jinjava/JinjavaConfig.java b/src/main/java/com/hubspot/jinjava/JinjavaConfig.java index 654960ade..ccfaf7ca5 100644 --- a/src/main/java/com/hubspot/jinjava/JinjavaConfig.java +++ b/src/main/java/com/hubspot/jinjava/JinjavaConfig.java @@ -24,6 +24,8 @@ import com.hubspot.jinjava.el.JinjavaObjectUnwrapper; import com.hubspot.jinjava.el.JinjavaProcessors; import com.hubspot.jinjava.el.ObjectUnwrapper; +import com.hubspot.jinjava.el.ext.AllowlistMethodValidator; +import com.hubspot.jinjava.el.ext.AllowlistReturnTypeValidator; import com.hubspot.jinjava.features.FeatureConfig; import com.hubspot.jinjava.features.Features; import com.hubspot.jinjava.interpret.Context.Library; @@ -98,8 +100,6 @@ default int getMaxMacroRecursionDepth() { } Map> getDisabled(); - Set getRestrictedMethods(); - Set getRestrictedProperties(); @Value.Default default boolean isFailOnUnknownTokens() { @@ -161,6 +161,16 @@ default TokenScannerSymbols getTokenScannerSymbols() { return new DefaultTokenScannerSymbols(); } + @Value.Default + default AllowlistMethodValidator getMethodValidator() { + return AllowlistMethodValidator.DEFAULT; + } + + @Value.Default + default AllowlistReturnTypeValidator getReturnTypeValidator() { + return AllowlistReturnTypeValidator.DEFAULT; + } + @Value.Default default ELResolver getElResolver() { return isDefaultReadOnlyResolver() diff --git a/src/main/java/com/hubspot/jinjava/el/ExpressionResolver.java b/src/main/java/com/hubspot/jinjava/el/ExpressionResolver.java index d96ea0ea2..60befb6e4 100644 --- a/src/main/java/com/hubspot/jinjava/el/ExpressionResolver.java +++ b/src/main/java/com/hubspot/jinjava/el/ExpressionResolver.java @@ -22,10 +22,12 @@ import com.hubspot.jinjava.interpret.UnknownTokenException; import com.hubspot.jinjava.interpret.errorcategory.BasicTemplateErrorCategory; import com.hubspot.jinjava.lib.fn.ELFunctionDefinition; +import com.hubspot.jinjava.objects.serialization.PyishObjectMapper; import com.hubspot.jinjava.util.WhitespaceUtils; import de.odysseus.el.tree.TreeBuilderException; import java.util.Arrays; import java.util.List; +import java.util.Objects; import javax.el.ELException; import javax.el.ExpressionFactory; import javax.el.PropertyNotFoundException; @@ -39,7 +41,7 @@ public class ExpressionResolver { private final JinjavaInterpreter interpreter; private final ExpressionFactory expressionFactory; - private final JinjavaInterpreterResolver resolver; + private final ReturnTypeValidatingJinjavaInterpreterResolver resolver; private final JinjavaELContext elContext; private final ObjectUnwrapper objectUnwrapper; @@ -53,7 +55,11 @@ public ExpressionResolver(JinjavaInterpreter interpreter, Jinjava jinjava) { ? jinjava.getEagerExpressionFactory() : jinjava.getExpressionFactory(); - this.resolver = new JinjavaInterpreterResolver(interpreter); + this.resolver = + new ReturnTypeValidatingJinjavaInterpreterResolver( + interpreter.getConfig().getReturnTypeValidator(), + new JinjavaInterpreterResolver(interpreter) + ); this.elContext = new JinjavaELContext(interpreter, resolver); for (ELFunctionDefinition fn : jinjava.getGlobalContext().getAllFunctions()) { this.elContext.setFunction(fn.getNamespace(), fn.getLocalName(), fn.getMethod()); @@ -343,4 +349,12 @@ public Object resolveProperty(Object object, List propertyNames) { public Object wrap(Object object) { return resolver.wrap(object); } + + public String getAsString(Object object) { + if (interpreter.getConfig().getLegacyOverrides().isUsePyishObjectMapper()) { + // resolver. + return PyishObjectMapper.getAsUnquotedPyishString(object); + } + return Objects.toString(object, ""); + } } diff --git a/src/main/java/com/hubspot/jinjava/el/HasInterpreter.java b/src/main/java/com/hubspot/jinjava/el/HasInterpreter.java new file mode 100644 index 000000000..8b22e98ee --- /dev/null +++ b/src/main/java/com/hubspot/jinjava/el/HasInterpreter.java @@ -0,0 +1,7 @@ +package com.hubspot.jinjava.el; + +import com.hubspot.jinjava.interpret.JinjavaInterpreter; + +public interface HasInterpreter { + JinjavaInterpreter interpreter(); +} diff --git a/src/main/java/com/hubspot/jinjava/el/JinjavaELContext.java b/src/main/java/com/hubspot/jinjava/el/JinjavaELContext.java index 15365de2c..8336b3e95 100644 --- a/src/main/java/com/hubspot/jinjava/el/JinjavaELContext.java +++ b/src/main/java/com/hubspot/jinjava/el/JinjavaELContext.java @@ -5,11 +5,12 @@ import java.lang.reflect.Method; import javax.el.ELResolver; -public class JinjavaELContext extends SimpleContext { +public class JinjavaELContext extends SimpleContext implements HasInterpreter { private JinjavaInterpreter interpreter; private MacroFunctionMapper functionMapper; + @Deprecated public JinjavaELContext() { super(); } @@ -19,6 +20,11 @@ public JinjavaELContext(JinjavaInterpreter interpreter, ELResolver resolver) { this.interpreter = interpreter; } + @Override + public JinjavaInterpreter interpreter() { + return interpreter; + } + @Override public MacroFunctionMapper getFunctionMapper() { if (functionMapper == null) { diff --git a/src/main/java/com/hubspot/jinjava/el/JinjavaInterpreterResolver.java b/src/main/java/com/hubspot/jinjava/el/JinjavaInterpreterResolver.java index 869bc6b8a..a0b393ea3 100644 --- a/src/main/java/com/hubspot/jinjava/el/JinjavaInterpreterResolver.java +++ b/src/main/java/com/hubspot/jinjava/el/JinjavaInterpreterResolver.java @@ -21,6 +21,7 @@ import com.hubspot.jinjava.objects.PyWrapper; import com.hubspot.jinjava.objects.collections.SizeLimitingPyList; import com.hubspot.jinjava.objects.collections.SizeLimitingPyMap; +import com.hubspot.jinjava.objects.collections.SizeLimitingPySet; import com.hubspot.jinjava.objects.date.FormattedDate; import com.hubspot.jinjava.objects.date.InvalidDateFormatException; import com.hubspot.jinjava.objects.date.PyishDate; @@ -39,6 +40,7 @@ import java.util.Locale; import java.util.Map; import java.util.Objects; +import java.util.Set; import javax.el.ArrayELResolver; import javax.el.CompositeELResolver; import javax.el.ELContext; @@ -274,7 +276,7 @@ private Object getValue( } context.setPropertyResolved(true); - return wrap(value); + return value; } @SuppressWarnings("unchecked") @@ -308,6 +310,12 @@ Object wrap(Object value) { interpreter.getConfig().getMaxListSize() ); } + if (Set.class.isAssignableFrom(value.getClass())) { + return new SizeLimitingPySet( + (Set) value, + interpreter.getConfig().getMaxListSize() + ); + } if (Map.class.isAssignableFrom(value.getClass())) { // FIXME: ensure keys are actually strings, if not, convert them return new SizeLimitingPyMap( diff --git a/src/main/java/com/hubspot/jinjava/el/NoInvokeELContext.java b/src/main/java/com/hubspot/jinjava/el/NoInvokeELContext.java index 1f05ec464..0409f21aa 100644 --- a/src/main/java/com/hubspot/jinjava/el/NoInvokeELContext.java +++ b/src/main/java/com/hubspot/jinjava/el/NoInvokeELContext.java @@ -1,13 +1,14 @@ package com.hubspot.jinjava.el; +import com.hubspot.jinjava.interpret.JinjavaInterpreter; import javax.el.ELContext; import javax.el.ELResolver; import javax.el.FunctionMapper; import javax.el.VariableMapper; -public class NoInvokeELContext extends ELContext { +public class NoInvokeELContext extends ELContext implements HasInterpreter { - private ELContext delegate; + private final ELContext delegate; private NoInvokeELResolver elResolver; public NoInvokeELContext(ELContext delegate) { @@ -31,4 +32,9 @@ public FunctionMapper getFunctionMapper() { public VariableMapper getVariableMapper() { return delegate.getVariableMapper(); } + + @Override + public JinjavaInterpreter interpreter() { + return ((HasInterpreter) delegate).interpreter(); + } } diff --git a/src/main/java/com/hubspot/jinjava/el/ReturnTypeValidatingJinjavaInterpreterResolver.java b/src/main/java/com/hubspot/jinjava/el/ReturnTypeValidatingJinjavaInterpreterResolver.java new file mode 100644 index 000000000..0a874f5e1 --- /dev/null +++ b/src/main/java/com/hubspot/jinjava/el/ReturnTypeValidatingJinjavaInterpreterResolver.java @@ -0,0 +1,73 @@ +package com.hubspot.jinjava.el; + +import com.hubspot.jinjava.el.ext.AllowlistReturnTypeValidator; +import java.beans.FeatureDescriptor; +import java.util.Iterator; +import javax.el.ELContext; +import javax.el.ELResolver; + +class ReturnTypeValidatingJinjavaInterpreterResolver extends ELResolver { + + private final AllowlistReturnTypeValidator returnTypeValidator; + private final JinjavaInterpreterResolver delegate; + + ReturnTypeValidatingJinjavaInterpreterResolver( + AllowlistReturnTypeValidator returnTypeValidator, + JinjavaInterpreterResolver delegate + ) { + this.returnTypeValidator = returnTypeValidator; + this.delegate = delegate; + } + + @Override + public Class getCommonPropertyType(ELContext context, Object base) { + return delegate.getCommonPropertyType(context, base); + } + + @Override + public Iterator getFeatureDescriptors( + ELContext context, + Object base + ) { + return delegate.getFeatureDescriptors(context, base); + } + + @Override + public Class getType(ELContext context, Object base, Object property) { + return delegate.getType(context, base, property); + } + + @Override + public Object getValue(ELContext context, Object base, Object property) { + return returnTypeValidator.validateReturnType( + wrap(delegate.getValue(context, base, property)) + ); + } + + @Override + public boolean isReadOnly(ELContext context, Object base, Object property) { + return delegate.isReadOnly(context, base, property); + } + + @Override + public void setValue(ELContext context, Object base, Object property, Object value) { + delegate.setValue(context, base, property, value); + } + + @Override + public Object invoke( + ELContext context, + Object base, + Object method, + Class[] paramTypes, + Object[] params + ) { + return returnTypeValidator.validateReturnType( + wrap(delegate.invoke(context, base, method, paramTypes, params)) + ); + } + + Object wrap(Object object) { + return delegate.wrap(object); + } +} diff --git a/src/main/java/com/hubspot/jinjava/el/ext/AllowlistGroup.java b/src/main/java/com/hubspot/jinjava/el/ext/AllowlistGroup.java new file mode 100644 index 000000000..2ba2b8dd6 --- /dev/null +++ b/src/main/java/com/hubspot/jinjava/el/ext/AllowlistGroup.java @@ -0,0 +1,206 @@ +package com.hubspot.jinjava.el.ext; + +import com.google.common.collect.ForwardingCollection; +import com.google.common.collect.ForwardingList; +import com.google.common.collect.ForwardingMap; +import com.google.common.collect.ForwardingSet; +import com.hubspot.jinjava.interpret.NullValue; +import com.hubspot.jinjava.lib.exptest.ExpTest; +import com.hubspot.jinjava.lib.filter.Filter; +import com.hubspot.jinjava.lib.fn.MacroFunction; +import com.hubspot.jinjava.lib.fn.eager.EagerMacroFunction; +import com.hubspot.jinjava.objects.DummyObject; +import com.hubspot.jinjava.objects.Namespace; +import com.hubspot.jinjava.objects.SafeString; +import com.hubspot.jinjava.objects.collections.PyList; +import com.hubspot.jinjava.objects.collections.PyMap; +import com.hubspot.jinjava.objects.collections.PySet; +import com.hubspot.jinjava.objects.collections.SizeLimitingPyList; +import com.hubspot.jinjava.objects.collections.SizeLimitingPyMap; +import com.hubspot.jinjava.objects.collections.SizeLimitingPySet; +import com.hubspot.jinjava.objects.collections.SnakeCaseAccessibleMap; +import com.hubspot.jinjava.objects.date.FormattedDate; +import com.hubspot.jinjava.objects.date.PyishDate; +import com.hubspot.jinjava.util.ForLoop; +import java.lang.reflect.Method; +import java.math.BigDecimal; +import java.math.BigInteger; +import java.time.ZonedDateTime; +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.Map; + +public enum AllowlistGroup { + JavaPrimitives { + private static final String[] ARRAY = { + String.class.getCanonicalName(), + Long.class.getCanonicalName(), + Integer.class.getCanonicalName(), + Double.class.getCanonicalName(), + Byte.class.getCanonicalName(), + Character.class.getCanonicalName(), + Float.class.getCanonicalName(), + Boolean.class.getCanonicalName(), + Short.class.getCanonicalName(), + long.class.getCanonicalName(), + int.class.getCanonicalName(), + double.class.getCanonicalName(), + byte.class.getCanonicalName(), + char.class.getCanonicalName(), + float.class.getCanonicalName(), + boolean.class.getCanonicalName(), + short.class.getCanonicalName(), + BigDecimal.class.getCanonicalName(), + BigInteger.class.getCanonicalName(), + }; + + @Override + String[] allowedReturnTypeClasses() { + return ARRAY; + } + + @Override + String[] allowedDeclaredMethodsFromClasses() { + return ARRAY; + } + }, + JinjavaObjects { + private static final String[] ARRAY = { + PyList.class.getCanonicalName(), + PyMap.class.getCanonicalName(), + SizeLimitingPyMap.class.getCanonicalName(), + SizeLimitingPyList.class.getCanonicalName(), + SnakeCaseAccessibleMap.class.getCanonicalName(), + FormattedDate.class.getCanonicalName(), + PyishDate.class.getCanonicalName(), + DummyObject.class.getCanonicalName(), + Namespace.class.getCanonicalName(), + SafeString.class.getCanonicalName(), + NullValue.class.getCanonicalName(), + }; + + @Override + String[] allowedReturnTypeClasses() { + return ARRAY; + } + + @Override + String[] allowedDeclaredMethodsFromClasses() { + return ARRAY; + } + }, + Collections { + private static final String[] ARRAY = { + Map.Entry.class.getCanonicalName(), + PyList.class.getCanonicalName(), + PyMap.class.getCanonicalName(), + PySet.class.getCanonicalName(), + SizeLimitingPyMap.class.getCanonicalName(), + SizeLimitingPyList.class.getCanonicalName(), + SizeLimitingPySet.class.getCanonicalName(), + ArrayList.class.getCanonicalName(), + ForwardingList.class.getCanonicalName(), + ForwardingMap.class.getCanonicalName(), + ForwardingSet.class.getCanonicalName(), + ForwardingCollection.class.getCanonicalName(), + LinkedHashMap.class.getCanonicalName(), + "%s.Entry".formatted(LinkedHashMap.class.getCanonicalName()), + "%s.LinkedValues".formatted(LinkedHashMap.class.getCanonicalName()), + }; + + @Override + String[] allowedReturnTypeClasses() { + return ARRAY; + } + + @Override + String[] allowedDeclaredMethodsFromClasses() { + return ARRAY; + } + + @Override + boolean enableArrays() { + return true; + } + }, + JinjavaTagConstructs { + private static final String[] ARRAY = { + ForLoop.class.getCanonicalName(), + MacroFunction.class.getCanonicalName(), + EagerMacroFunction.class.getCanonicalName(), + }; + + @Override + String[] allowedReturnTypeClasses() { + return ARRAY; + } + + @Override + String[] allowedDeclaredMethodsFromClasses() { + return ARRAY; + } + }, + JinjavaFilters { + private static final String[] ARRAY = { Filter.class.getPackageName() }; + + @Override + String[] allowedDeclaredMethodsFromCanonicalClassPrefixes() { + return ARRAY; + } + + @Override + String[] allowedReturnTypeCanonicalClassPrefixes() { + return ARRAY; + } + }, + JinjavaFunctions { + private static final String[] ARRAY = { ZonedDateTime.class.getCanonicalName() }; + + @Override + String[] allowedDeclaredMethodsFromClasses() { + return ARRAY; + } + + @Override + String[] allowedReturnTypeClasses() { + return ARRAY; + } + }, + JinjavaExpTests { + private static final String[] ARRAY = { ExpTest.class.getPackageName() }; + + @Override + String[] allowedDeclaredMethodsFromCanonicalClassPrefixes() { + return ARRAY; + } + + @Override + String[] allowedReturnTypeCanonicalClassPrefixes() { + return ARRAY; + } + }; + + Method[] allowMethods() { + return new Method[0]; + } + + String[] allowedDeclaredMethodsFromCanonicalClassPrefixes() { + return new String[0]; + } + + String[] allowedDeclaredMethodsFromClasses() { + return new String[0]; + } + + String[] allowedReturnTypeCanonicalClassPrefixes() { + return new String[0]; + } + + String[] allowedReturnTypeClasses() { + return new String[0]; + } + + boolean enableArrays() { + return false; + } +} diff --git a/src/main/java/com/hubspot/jinjava/el/ext/AllowlistMethodValidator.java b/src/main/java/com/hubspot/jinjava/el/ext/AllowlistMethodValidator.java new file mode 100644 index 000000000..a29fac7d4 --- /dev/null +++ b/src/main/java/com/hubspot/jinjava/el/ext/AllowlistMethodValidator.java @@ -0,0 +1,76 @@ +package com.hubspot.jinjava.el.ext; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import java.lang.reflect.Method; +import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Consumer; + +public final class AllowlistMethodValidator { + + public static final AllowlistMethodValidator DEFAULT = AllowlistMethodValidator.create( + MethodValidatorConfig.builder().addDefaultAllowlistGroups().build() + ); + private final ConcurrentHashMap allowedMethodsCache; + private final ImmutableSet allowedMethods; + private final ImmutableSet allowedDeclaredMethodsFromCanonicalClassPrefixes; + private final ImmutableSet allowedDeclaredMethodsFromCanonicalClassNames; + private final Consumer onRejectedMethod; + private final ImmutableList additionalValidators; + + public static AllowlistMethodValidator create( + MethodValidatorConfig methodValidatorConfig, + MethodValidator... additionalValidators + ) { + return new AllowlistMethodValidator( + methodValidatorConfig, + ImmutableList.copyOf(additionalValidators) + ); + } + + private AllowlistMethodValidator( + MethodValidatorConfig methodValidatorConfig, + ImmutableList additionalValidators + ) { + this.allowedMethods = methodValidatorConfig.allowedMethods(); + this.allowedDeclaredMethodsFromCanonicalClassPrefixes = + methodValidatorConfig.allowedDeclaredMethodsFromCanonicalClassPrefixes(); + this.allowedDeclaredMethodsFromCanonicalClassNames = + methodValidatorConfig.allowedDeclaredMethodsFromCanonicalClassNames(); + this.onRejectedMethod = methodValidatorConfig.onRejectedMethod(); + this.additionalValidators = additionalValidators; + this.allowedMethodsCache = new ConcurrentHashMap<>(); + } + + public Method validateMethod(Method m) { + if (m == null) { + return null; + } + boolean isAllowedMethod = allowedMethodsCache.computeIfAbsent( + m, + m1 -> { + Class clazz = m1.getDeclaringClass(); + String canonicalClassName = clazz.getCanonicalName(); + return ( + allowedMethods.contains(m1) || + allowedDeclaredMethodsFromCanonicalClassNames.contains(canonicalClassName) || + allowedDeclaredMethodsFromCanonicalClassPrefixes + .stream() + .anyMatch(canonicalClassName::startsWith) + ); + } + ); + if (!isAllowedMethod) { + onRejectedMethod.accept(m); + return null; + } + for (MethodValidator v : additionalValidators) { + if (v.validateMethod(m) == null) { + onRejectedMethod.accept(m); + return null; + } + } + + return m; + } +} diff --git a/src/main/java/com/hubspot/jinjava/el/ext/AllowlistReturnTypeValidator.java b/src/main/java/com/hubspot/jinjava/el/ext/AllowlistReturnTypeValidator.java new file mode 100644 index 000000000..739cdd9ef --- /dev/null +++ b/src/main/java/com/hubspot/jinjava/el/ext/AllowlistReturnTypeValidator.java @@ -0,0 +1,73 @@ +package com.hubspot.jinjava.el.ext; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Consumer; + +public final class AllowlistReturnTypeValidator { + + public static final AllowlistReturnTypeValidator DEFAULT = + AllowlistReturnTypeValidator.create( + ReturnTypeValidatorConfig.builder().addDefaultAllowlistGroups().build() + ); + private final ConcurrentHashMap allowedReturnTypesCache; + + private final ImmutableSet allowedCanonicalClassPrefixes; + private final ImmutableSet allowedCanonicalClassNames; + private final boolean allowArrays; + private final Consumer> onRejectedClass; + private final ImmutableList additionalValidators; + + public static AllowlistReturnTypeValidator create( + ReturnTypeValidatorConfig returnTypeValidatorConfig, + ReturnTypeValidator... additionalValidators + ) { + return new AllowlistReturnTypeValidator( + returnTypeValidatorConfig, + ImmutableList.copyOf(additionalValidators) + ); + } + + private AllowlistReturnTypeValidator( + ReturnTypeValidatorConfig returnTypeValidatorConfig, + ImmutableList additionalValidators + ) { + this.allowedCanonicalClassPrefixes = + returnTypeValidatorConfig.allowedCanonicalClassPrefixes(); + this.allowedCanonicalClassNames = + returnTypeValidatorConfig.allowedCanonicalClassNames(); + this.allowArrays = returnTypeValidatorConfig.allowArrays(); + this.onRejectedClass = returnTypeValidatorConfig.onRejectedClass(); + this.additionalValidators = additionalValidators; + this.allowedReturnTypesCache = new ConcurrentHashMap<>(); + } + + public Object validateReturnType(Object o) { + if (o == null) { + return null; + } + Class clazz = o.getClass(); + if (clazz.isArray() && allowArrays) { + return o; + } + String canonicalClassName = clazz.getCanonicalName(); + boolean isAllowedClassName = allowedReturnTypesCache.computeIfAbsent( + canonicalClassName, + c -> + allowedCanonicalClassNames.contains(canonicalClassName) || + allowedCanonicalClassPrefixes.stream().anyMatch(canonicalClassName::startsWith) + ); + if (!isAllowedClassName) { + onRejectedClass.accept(clazz); + return null; + } + for (ReturnTypeValidator v : additionalValidators) { + if (v.validateReturnType(o) == null) { + onRejectedClass.accept(clazz); + return null; + } + } + return o; + } +} diff --git a/src/main/java/com/hubspot/jinjava/el/ext/AstDict.java b/src/main/java/com/hubspot/jinjava/el/ext/AstDict.java index 01e626bf6..d92acf7b3 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/AstDict.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/AstDict.java @@ -1,5 +1,6 @@ package com.hubspot.jinjava.el.ext; +import com.hubspot.jinjava.el.HasInterpreter; import com.hubspot.jinjava.interpret.JinjavaInterpreter; import com.hubspot.jinjava.interpret.TemplateStateException; import com.hubspot.jinjava.objects.collections.SizeLimitingPyMap; @@ -26,10 +27,7 @@ public AstDict(Map dict) { public Object eval(Bindings bindings, ELContext context) { Map resolved = new LinkedHashMap<>(); - JinjavaInterpreter interpreter = (JinjavaInterpreter) context - .getELResolver() - .getValue(context, null, ExtendedParser.INTERPRETER); - + JinjavaInterpreter interpreter = ((HasInterpreter) context).interpreter(); for (Map.Entry entry : dict.entrySet()) { String key; AstNode entryKey = entry.getKey(); diff --git a/src/main/java/com/hubspot/jinjava/el/ext/AstFilterChain.java b/src/main/java/com/hubspot/jinjava/el/ext/AstFilterChain.java index cecaa9ea6..bb29d5694 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/AstFilterChain.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/AstFilterChain.java @@ -1,5 +1,6 @@ package com.hubspot.jinjava.el.ext; +import com.hubspot.jinjava.el.HasInterpreter; import com.hubspot.jinjava.interpret.DisabledException; import com.hubspot.jinjava.interpret.JinjavaInterpreter; import com.hubspot.jinjava.interpret.TemplateError; @@ -122,9 +123,7 @@ public Object eval(Bindings bindings, ELContext context) { } protected JinjavaInterpreter getInterpreter(ELContext context) { - return (JinjavaInterpreter) context - .getELResolver() - .getValue(context, null, ExtendedParser.INTERPRETER); + return ((HasInterpreter) context).interpreter(); } protected Object[] evaluateFilterArgs( diff --git a/src/main/java/com/hubspot/jinjava/el/ext/AstList.java b/src/main/java/com/hubspot/jinjava/el/ext/AstList.java index 75648e4d4..163548a2e 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/AstList.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/AstList.java @@ -1,5 +1,6 @@ package com.hubspot.jinjava.el.ext; +import com.hubspot.jinjava.el.HasInterpreter; import com.hubspot.jinjava.interpret.JinjavaInterpreter; import com.hubspot.jinjava.objects.collections.SizeLimitingPyList; import de.odysseus.el.tree.Bindings; @@ -26,9 +27,7 @@ public Object eval(Bindings bindings, ELContext context) { list.add(elements.getChild(i).eval(bindings, context)); } - JinjavaInterpreter interpreter = (JinjavaInterpreter) context - .getELResolver() - .getValue(context, null, ExtendedParser.INTERPRETER); + JinjavaInterpreter interpreter = ((HasInterpreter) context).interpreter(); return new SizeLimitingPyList(list, interpreter.getConfig().getMaxListSize()); } diff --git a/src/main/java/com/hubspot/jinjava/el/ext/AstMacroFunction.java b/src/main/java/com/hubspot/jinjava/el/ext/AstMacroFunction.java index 6975d5ce6..044249407 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/AstMacroFunction.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/AstMacroFunction.java @@ -2,6 +2,7 @@ import com.google.common.collect.ImmutableMap; import com.hubspot.algebra.Result; +import com.hubspot.jinjava.el.HasInterpreter; import com.hubspot.jinjava.interpret.AutoCloseableSupplier; import com.hubspot.jinjava.interpret.AutoCloseableSupplier.AutoCloseableImpl; import com.hubspot.jinjava.interpret.CallStack; @@ -31,9 +32,7 @@ public AstMacroFunction(String name, int index, AstParameters params, boolean va @Override public Object eval(Bindings bindings, ELContext context) { - JinjavaInterpreter interpreter = (JinjavaInterpreter) context - .getELResolver() - .getValue(context, null, ExtendedParser.INTERPRETER); + JinjavaInterpreter interpreter = ((HasInterpreter) context).interpreter(); MacroFunction macroFunction = interpreter.getContext().getGlobalMacro(getName()); if (macroFunction != null) { diff --git a/src/main/java/com/hubspot/jinjava/el/ext/AstRangeBracket.java b/src/main/java/com/hubspot/jinjava/el/ext/AstRangeBracket.java index 33b65fcba..97eccfee2 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/AstRangeBracket.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/AstRangeBracket.java @@ -1,6 +1,7 @@ package com.hubspot.jinjava.el.ext; import com.google.common.collect.Iterables; +import com.hubspot.jinjava.el.HasInterpreter; import com.hubspot.jinjava.interpret.JinjavaInterpreter; import com.hubspot.jinjava.objects.collections.PyList; import com.hubspot.jinjava.objects.collections.SizeLimitingPyList; @@ -79,9 +80,7 @@ public Object eval(Bindings bindings, ELContext context) { int startNum = ((Number) start).intValue(); int endNum = ((Number) end).intValue(); - JinjavaInterpreter interpreter = (JinjavaInterpreter) context - .getELResolver() - .getValue(context, null, ExtendedParser.INTERPRETER); + JinjavaInterpreter interpreter = ((HasInterpreter) context).interpreter(); PyList result = new SizeLimitingPyList( new ArrayList<>(), diff --git a/src/main/java/com/hubspot/jinjava/el/ext/BannedAllowlistOptions.java b/src/main/java/com/hubspot/jinjava/el/ext/BannedAllowlistOptions.java new file mode 100644 index 000000000..9f8eee070 --- /dev/null +++ b/src/main/java/com/hubspot/jinjava/el/ext/BannedAllowlistOptions.java @@ -0,0 +1,66 @@ +package com.hubspot.jinjava.el.ext; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.collect.ImmutableSet; +import java.lang.reflect.Method; +import java.util.Arrays; +import java.util.List; +import java.util.Set; +import java.util.stream.Stream; +import org.immutables.value.Value; + +public class BannedAllowlistOptions { + + // These aren't required, but they prevent someone from misconfiguring Jinjava to allow sandbox bypass unintentionally + private static final String JAVA_LANG_REFLECT_PACKAGE = + Method.class.getPackage().getName(); // java.lang.reflect + private static final String JACKSON_DATABIND_PACKAGE = + ObjectMapper.class.getPackage().getName(); // com.fasterxml.jackson.databind + + private static final String[] BANNED_PREFIXES = { + Class.class.getCanonicalName(), + Object.class.getCanonicalName(), + JAVA_LANG_REFLECT_PACKAGE, + JACKSON_DATABIND_PACKAGE, + }; + + private static final Set ALLOWED_JINJAVA_PREFIXES = Stream + .concat( + Stream.of("com.hubspot.jinjava.testobjects"), + Arrays + .stream(AllowlistGroup.values()) + .flatMap(g -> + Stream + .of( + g.allowedDeclaredMethodsFromCanonicalClassPrefixes(), + g.allowedReturnTypeCanonicalClassPrefixes(), + g.allowedDeclaredMethodsFromClasses(), + g.allowedReturnTypeClasses() + ) + .flatMap(Arrays::stream) + ) + ) + .collect(ImmutableSet.toImmutableSet()); + + @Value.Check + public static List findBannedPrefixes(Stream prefixes) { + return prefixes + .filter(prefixOrName -> + Arrays + .stream(BANNED_PREFIXES) + .anyMatch(banned -> + banned.startsWith(prefixOrName) || prefixOrName.startsWith(banned) + ) || + isIllegalJinjavaClass(prefixOrName) + ) + .toList(); + } + + private static boolean isIllegalJinjavaClass(String prefixOrName) { + if (!prefixOrName.startsWith("com.hubspot.jinjava")) { + return false; + } + // e.g. com.hubspot.jinjava.lib.exptest is allowed, but com.hubspot.jinjava.Jinjava will not be + return ALLOWED_JINJAVA_PREFIXES.stream().noneMatch(prefixOrName::startsWith); + } +} diff --git a/src/main/java/com/hubspot/jinjava/el/ext/BeanELResolver.java b/src/main/java/com/hubspot/jinjava/el/ext/BeanELResolver.java index a37720465..9210ca86d 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/BeanELResolver.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/BeanELResolver.java @@ -436,7 +436,7 @@ public void setValue(ELContext context, Object base, Object property, Object val if (readOnly) { throw new PropertyNotWritableException("resolver is read-only"); } - Method method = toBeanProperty(base, property).getWriteMethod(); + Method method = getWriteMethod(base, property); if (method == null) { throw new PropertyNotWritableException("Cannot write property: " + property); } @@ -654,11 +654,15 @@ protected Object[] coerceParams( return args; } + protected Method getWriteMethod(Object base, Object property) { + return toBeanProperty(base, property).getWriteMethod(); + } + protected Method getReadMethod(Object base, Object property) { return toBeanProperty(base, property).getReadMethod(); } - private void coerceValue( + protected void coerceValue( Object array, int index, ExpressionFactory factory, diff --git a/src/main/java/com/hubspot/jinjava/el/ext/ExtendedParser.java b/src/main/java/com/hubspot/jinjava/el/ext/ExtendedParser.java index 543726a7b..54726af70 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/ExtendedParser.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/ExtendedParser.java @@ -129,7 +129,7 @@ public AstNode createAstNode(AstNode... children) { } protected AstNode interpreter() { - return identifier(INTERPRETER); + return new AstNull(); } @Override diff --git a/src/main/java/com/hubspot/jinjava/el/ext/JinjavaBeanELResolver.java b/src/main/java/com/hubspot/jinjava/el/ext/JinjavaBeanELResolver.java index 5a3779b14..c9ec31f2d 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/JinjavaBeanELResolver.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/JinjavaBeanELResolver.java @@ -1,19 +1,27 @@ package com.hubspot.jinjava.el.ext; -import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.base.CaseFormat; import com.google.common.collect.ImmutableSet; +import com.hubspot.jinjava.JinjavaConfig; import com.hubspot.jinjava.interpret.DeferredValueException; import com.hubspot.jinjava.interpret.JinjavaInterpreter; import com.hubspot.jinjava.util.EagerReconstructionUtils; +import java.beans.IntrospectionException; +import java.beans.Introspector; +import java.beans.MethodDescriptor; import java.lang.invoke.MethodType; -import java.lang.reflect.Constructor; -import java.lang.reflect.Field; +import java.lang.reflect.Array; import java.lang.reflect.Method; +import java.util.HashMap; import java.util.LinkedList; import java.util.List; +import java.util.Map; +import java.util.Objects; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import javax.el.ELContext; +import javax.el.ELException; +import javax.el.ExpressionFactory; import javax.el.MethodNotFoundException; /** @@ -21,25 +29,6 @@ */ public class JinjavaBeanELResolver extends BeanELResolver { - private static final Set DEFAULT_RESTRICTED_PROPERTIES = ImmutableSet - .builder() - .add("class") - .build(); - - private static final Set DEFAULT_RESTRICTED_METHODS = ImmutableSet - .builder() - .add("class") - .add("clone") - .add("hashCode") - .add("getClass") - .add("getDeclaringClass") - .add("forName") - .add("notify") - .add("notifyAll") - .add("wait") - .add("readValueAs") - .build(); - private static final Set DEFERRED_EXECUTION_RESTRICTED_METHODS = ImmutableSet .builder() .add("put") @@ -61,45 +50,87 @@ public class JinjavaBeanELResolver extends BeanELResolver { .add("set") .add("merge") .build(); - private static final String JAVA_LANG_REFLECT_PACKAGE = - Method.class.getPackage().getName(); // java.lang.reflect - private static final String JACKSON_DATABIND_PACKAGE = - ObjectMapper.class.getPackage().getName(); // com.fasterxml.jackson.databind - /** - * Creates a new read/write {@link JinjavaBeanELResolver}. - */ - public JinjavaBeanELResolver() {} + protected static final class BeanMethods { + + private final Map> map = new HashMap<>(); + + public BeanMethods(Class baseClass) { + MethodDescriptor[] descriptors; + try { + descriptors = Introspector.getBeanInfo(baseClass).getMethodDescriptors(); + } catch (IntrospectionException e) { + throw new ELException(e); + } + for (MethodDescriptor descriptor : descriptors) { + map.compute( + descriptor.getName(), + (k, v) -> { + if (v == null) { + v = new LinkedList<>(); + } + v.add(new BeanMethod(descriptor)); + return v; + } + ); + } + } + + public List getBeanMethods(String methodName) { + return map.get(methodName); + } + + protected static final class BeanMethod { + + private final MethodDescriptor descriptor; + + private Method method; + + public BeanMethod(MethodDescriptor descriptor) { + this.descriptor = descriptor; + } + + public Method getMethod() { + if (method == null) { + method = findAccessibleMethod(descriptor.getMethod()); + } + return method; + } + } + } + + private final ConcurrentHashMap, BeanMethods> beanMethodsCache; + + public JinjavaBeanELResolver() { + this(true); + } /** - * Creates a new {@link JinjavaBeanELResolver} whose read-only status is determined by the given parameter. + * Creates a new read/write {@link JinjavaBeanELResolver}. */ public JinjavaBeanELResolver(boolean readOnly) { super(readOnly); + this.beanMethodsCache = new ConcurrentHashMap, BeanMethods>(); } @Override public Class getType(ELContext context, Object base, Object property) { - return super.getType(context, base, validatePropertyName(property)); + return super.getType(context, base, transformPropertyName(property)); } @Override public Object getValue(ELContext context, Object base, Object property) { - if (isRestrictedClass(base)) { - return null; - } - Object result = super.getValue(context, base, validatePropertyName(property)); - return result instanceof Class ? null : result; + return super.getValue(context, base, transformPropertyName(property)); } @Override public boolean isReadOnly(ELContext context, Object base, Object property) { - return super.isReadOnly(context, base, validatePropertyName(property)); + return super.isReadOnly(context, base, transformPropertyName(property)); } @Override public void setValue(ELContext context, Object base, Object property, Object value) { - super.setValue(context, base, validatePropertyName(property), value); + super.setValue(context, base, transformPropertyName(property), value); } @Override @@ -110,25 +141,9 @@ public Object invoke( Class[] paramTypes, Object[] params ) { - JinjavaInterpreter interpreter = JinjavaInterpreter.getCurrent(); - - if ( - method == null || - DEFAULT_RESTRICTED_METHODS.contains(method.toString()) || - (interpreter != null && - interpreter.getConfig().getRestrictedMethods().contains(method.toString())) - ) { - throw new MethodNotFoundException( - "Cannot find method '" + method + "' in " + base.getClass() - ); + if (method == null) { + throw new MethodNotFoundException("Cannot find method null in " + base.getClass()); } - - if (isRestrictedClass(base)) { - throw new MethodNotFoundException( - "Cannot find method '" + method + "' in " + base.getClass() - ); - } - if ( DEFERRED_EXECUTION_RESTRICTED_METHODS.contains(method.toString()) && EagerReconstructionUtils.isDeferredExecutionMode() @@ -142,15 +157,23 @@ public Object invoke( ); } - Object result = super.invoke(context, base, method, paramTypes, params); + return super.invoke(context, base, method, paramTypes, params); + } - if (isRestrictedClass(result)) { - throw new MethodNotFoundException( - "Cannot find method '" + method + "' in " + base.getClass() - ); + // As opposed to supporting ____int3rpr3t3r____, coerce JinjavaInterpreter on validated methods + @Override + protected void coerceValue( + Object array, + int index, + ExpressionFactory factory, + Object value, + Class type + ) { + if (type.equals(JinjavaInterpreter.class)) { + Array.set(array, index, JinjavaInterpreter.getCurrent()); + } else { + super.coerceValue(array, index, factory, value, type); } - - return result; } @Override @@ -161,37 +184,64 @@ protected Method findMethod( Object[] params, int paramCount ) { + Method method; if (types != null) { - return super.findMethod(base, name, types, params, paramCount); - } - Method varArgsMethod = null; + method = super.findMethod(base, name, types, params, paramCount); + } else { + Method varArgsMethod = null; + BeanMethods beanMethods = beanMethodsCache.get(base.getClass()); + if (beanMethods == null) { + BeanMethods newBeanMethods = new BeanMethods(base.getClass()); + beanMethods = beanMethodsCache.putIfAbsent(base.getClass(), newBeanMethods); + if (beanMethods == null) { // put succeeded, use new value + beanMethods = newBeanMethods; + } + } - Method[] methods = base.getClass().getMethods(); - List potentialMethods = new LinkedList<>(); + List potentialMethods = new LinkedList<>(); - for (Method method : methods) { - if (method.getName().equals(name)) { - int formalParamCount = method.getParameterTypes().length; - if (method.isVarArgs() && paramCount >= formalParamCount - 1) { - varArgsMethod = method; + for (BeanMethods.BeanMethod bm : beanMethods.getBeanMethods(name)) { + Method m = bm.getMethod(); + int formalParamCount = m.getParameterTypes().length; + if (m.isVarArgs() && paramCount >= formalParamCount - 1) { + varArgsMethod = m; } else if (paramCount == formalParamCount) { - potentialMethods.add(findAccessibleMethod(method)); + potentialMethods.add(m); } } - } - final Method finalVarArgsMethod = varArgsMethod; - return potentialMethods - .stream() - .filter(method -> checkAssignableParameterTypes(params, method)) - .min(JinjavaBeanELResolver::pickMoreSpecificMethod) - .orElseGet(() -> + final Method finalVarArgsMethod = varArgsMethod; + method = potentialMethods .stream() - .findAny() - .orElseGet(() -> - finalVarArgsMethod == null ? null : findAccessibleMethod(finalVarArgsMethod) - ) - ); + .filter(m -> checkAssignableParameterTypes(params, m)) + .min(JinjavaBeanELResolver::pickMoreSpecificMethod) + .orElseGet(() -> potentialMethods.stream().findAny().orElse(finalVarArgsMethod) + ); + } + return getJinjavaConfig().getMethodValidator().validateMethod(method); + } + + @Override + protected Method getWriteMethod(Object base, Object property) { + return getJinjavaConfig() + .getMethodValidator() + .validateMethod(super.getWriteMethod(base, property)); + } + + @Override + protected Method getReadMethod(Object base, Object property) { + return getJinjavaConfig() + .getMethodValidator() + .validateMethod(super.getReadMethod(base, property)); + } + + private static JinjavaConfig getJinjavaConfig() { + return Objects + .requireNonNull( + JinjavaInterpreter.getCurrent(), + "JinjavaInterpreter.closeablePushCurrent must be used if using a JinjavaBeanELResolver directly" + ) + .getConfig(); } private static boolean checkAssignableParameterTypes(Object[] params, Method method) { @@ -221,22 +271,6 @@ private static int pickMoreSpecificMethod(Method methodA, Method methodB) { return 1; } - private String validatePropertyName(Object property) { - String propertyName = transformPropertyName(property); - - JinjavaInterpreter interpreter = JinjavaInterpreter.getCurrent(); - - if ( - DEFAULT_RESTRICTED_PROPERTIES.contains(propertyName) || - (interpreter != null && - interpreter.getConfig().getRestrictedProperties().contains(propertyName)) - ) { - return null; - } - - return propertyName; - } - /** * Transform snake case to property name. */ @@ -251,24 +285,4 @@ private String transformPropertyName(Object property) { } return CaseFormat.LOWER_UNDERSCORE.to(CaseFormat.LOWER_CAMEL, propertyStr); } - - protected boolean isRestrictedClass(Object o) { - if (o == null) { - return false; - } - - Package oPackage = o.getClass().getPackage(); - return ( - (oPackage != null && - (oPackage.getName().startsWith(JAVA_LANG_REFLECT_PACKAGE) || - oPackage.getName().startsWith(JACKSON_DATABIND_PACKAGE))) || - o instanceof Class || - o instanceof ClassLoader || - o instanceof Thread || - o instanceof Method || - o instanceof Field || - o instanceof Constructor || - o instanceof JinjavaInterpreter - ); - } } diff --git a/src/main/java/com/hubspot/jinjava/el/ext/MethodValidator.java b/src/main/java/com/hubspot/jinjava/el/ext/MethodValidator.java new file mode 100644 index 000000000..eb065b7d2 --- /dev/null +++ b/src/main/java/com/hubspot/jinjava/el/ext/MethodValidator.java @@ -0,0 +1,10 @@ +package com.hubspot.jinjava.el.ext; + +import java.lang.reflect.Method; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; + +public interface MethodValidator { + @Nullable + Method validateMethod(@Nonnull Method m); +} diff --git a/src/main/java/com/hubspot/jinjava/el/ext/MethodValidatorConfig.java b/src/main/java/com/hubspot/jinjava/el/ext/MethodValidatorConfig.java new file mode 100644 index 000000000..52ecea9e1 --- /dev/null +++ b/src/main/java/com/hubspot/jinjava/el/ext/MethodValidatorConfig.java @@ -0,0 +1,77 @@ +package com.hubspot.jinjava.el.ext; + +import com.google.common.collect.ImmutableSet; +import com.hubspot.jinjava.JinjavaImmutableStyle; +import java.lang.reflect.Method; +import java.util.List; +import java.util.function.Consumer; +import java.util.function.Function; +import java.util.stream.Stream; +import org.immutables.value.Value; + +@Value.Immutable(singleton = true) +@JinjavaImmutableStyle +public abstract class MethodValidatorConfig { + + public abstract ImmutableSet allowedMethods(); + + public abstract ImmutableSet allowedDeclaredMethodsFromCanonicalClassPrefixes(); + + public abstract ImmutableSet allowedDeclaredMethodsFromCanonicalClassNames(); + + @Value.Default + public Consumer onRejectedMethod() { + return m -> {}; + } + + @Value.Check + void banClassesAndMethods() { + List list = BannedAllowlistOptions.findBannedPrefixes( + Stream + .of( + allowedMethods() + .stream() + .map(method -> method.getDeclaringClass().getCanonicalName()), + allowedDeclaredMethodsFromCanonicalClassPrefixes().stream(), + allowedDeclaredMethodsFromCanonicalClassNames().stream() + ) + .flatMap(Function.identity()) + ); + if (!list.isEmpty()) { + throw new IllegalStateException( + "Banned classes or prefixes (Object.class, Class.class, java.lang.reflect, com.fasterxml.jackson.databind) are not allowed: " + + list + ); + } + } + + public static MethodValidatorConfig of() { + return ImmutableMethodValidatorConfig.of(); + } + + public static Builder builder() { + return new Builder(); + } + + public static class Builder extends ImmutableMethodValidatorConfig.Builder { + + Builder() {} + + public Builder addDefaultAllowlistGroups() { + return addAllowlistGroups(AllowlistGroup.values()); + } + + public Builder addAllowlistGroups(AllowlistGroup... allowlistGroups) { + for (AllowlistGroup allowlistGroup : allowlistGroups) { + this.addAllowedMethods(allowlistGroup.allowMethods()) + .addAllowedDeclaredMethodsFromCanonicalClassPrefixes( + allowlistGroup.allowedDeclaredMethodsFromCanonicalClassPrefixes() + ) + .addAllowedDeclaredMethodsFromCanonicalClassNames( + allowlistGroup.allowedDeclaredMethodsFromClasses() + ); + } + return this; + } + } +} diff --git a/src/main/java/com/hubspot/jinjava/el/ext/ReturnTypeValidator.java b/src/main/java/com/hubspot/jinjava/el/ext/ReturnTypeValidator.java new file mode 100644 index 000000000..f60f0fe2a --- /dev/null +++ b/src/main/java/com/hubspot/jinjava/el/ext/ReturnTypeValidator.java @@ -0,0 +1,8 @@ +package com.hubspot.jinjava.el.ext; + +import javax.annotation.Nullable; + +public interface ReturnTypeValidator { + @Nullable + Object validateReturnType(Object o); +} diff --git a/src/main/java/com/hubspot/jinjava/el/ext/ReturnTypeValidatorConfig.java b/src/main/java/com/hubspot/jinjava/el/ext/ReturnTypeValidatorConfig.java new file mode 100644 index 000000000..8a885c519 --- /dev/null +++ b/src/main/java/com/hubspot/jinjava/el/ext/ReturnTypeValidatorConfig.java @@ -0,0 +1,76 @@ +package com.hubspot.jinjava.el.ext; + +import com.google.common.collect.ImmutableSet; +import com.hubspot.jinjava.JinjavaImmutableStyle; +import java.util.List; +import java.util.function.Consumer; +import java.util.function.Function; +import java.util.stream.Stream; +import org.immutables.value.Value; + +@Value.Immutable(singleton = true) +@JinjavaImmutableStyle +public abstract class ReturnTypeValidatorConfig { + + public abstract ImmutableSet allowedCanonicalClassPrefixes(); + + public abstract ImmutableSet allowedCanonicalClassNames(); + + @Value.Default + public Consumer> onRejectedClass() { + return m -> {}; + } + + @Value.Default + public boolean allowArrays() { + return false; + } + + @Value.Check + void banClassesAndMethods() { + List list = BannedAllowlistOptions.findBannedPrefixes( + Stream + .of( + allowedCanonicalClassPrefixes().stream(), + allowedCanonicalClassNames().stream() + ) + .flatMap(Function.identity()) + ); + if (!list.isEmpty()) { + throw new IllegalStateException( + "Banned classes or prefixes (Object.class, Class.class, java.lang.reflect, com.fasterxml.jackson.databind) are not allowed: " + + list + ); + } + } + + public static ReturnTypeValidatorConfig of() { + return ImmutableReturnTypeValidatorConfig.of(); + } + + public static Builder builder() { + return new Builder(); + } + + public static class Builder extends ImmutableReturnTypeValidatorConfig.Builder { + + Builder() {} + + public Builder addDefaultAllowlistGroups() { + return addAllowlistGroups(AllowlistGroup.values()); + } + + public Builder addAllowlistGroups(AllowlistGroup... allowlistGroups) { + for (AllowlistGroup allowlistGroup : allowlistGroups) { + this.addAllowedCanonicalClassPrefixes( + allowlistGroup.allowedReturnTypeCanonicalClassPrefixes() + ) + .addAllowedCanonicalClassNames(allowlistGroup.allowedReturnTypeClasses()); + if (allowlistGroup.enableArrays()) { + this.setAllowArrays(true); + } + } + return this; + } + } +} diff --git a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstDict.java b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstDict.java index 4bfac2111..8bdff2434 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstDict.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstDict.java @@ -1,8 +1,8 @@ package com.hubspot.jinjava.el.ext.eager; +import com.hubspot.jinjava.el.HasInterpreter; import com.hubspot.jinjava.el.ext.AstDict; import com.hubspot.jinjava.el.ext.DeferredParsingException; -import com.hubspot.jinjava.el.ext.ExtendedParser; import com.hubspot.jinjava.el.ext.IdentifierPreservationStrategy; import com.hubspot.jinjava.interpret.JinjavaInterpreter; import com.hubspot.jinjava.util.EagerExpressionResolver; @@ -38,9 +38,7 @@ public String getPartiallyResolved( DeferredParsingException deferredParsingException, IdentifierPreservationStrategy identifierPreservationStrategy ) { - JinjavaInterpreter interpreter = (JinjavaInterpreter) context - .getELResolver() - .getValue(context, null, ExtendedParser.INTERPRETER); + JinjavaInterpreter interpreter = ((HasInterpreter) context).interpreter(); StringJoiner joiner = new StringJoiner(", "); dict.forEach((key, value) -> { StringJoiner kvJoiner = new StringJoiner(": "); diff --git a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstMacroFunction.java b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstMacroFunction.java index 4e3ff190b..85d2bb8fd 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstMacroFunction.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstMacroFunction.java @@ -1,13 +1,12 @@ package com.hubspot.jinjava.el.ext.eager; +import com.hubspot.jinjava.el.HasInterpreter; import com.hubspot.jinjava.el.ext.AstMacroFunction; import com.hubspot.jinjava.el.ext.DeferredInvocationResolutionException; import com.hubspot.jinjava.el.ext.DeferredParsingException; -import com.hubspot.jinjava.el.ext.ExtendedParser; import com.hubspot.jinjava.el.ext.IdentifierPreservationStrategy; import com.hubspot.jinjava.interpret.Context.TemporaryValueClosable; import com.hubspot.jinjava.interpret.DeferredValueException; -import com.hubspot.jinjava.interpret.JinjavaInterpreter; import de.odysseus.el.tree.Bindings; import de.odysseus.el.tree.impl.ast.AstParameters; import java.lang.reflect.Array; @@ -79,9 +78,8 @@ protected Object invoke( // This is just the AstFunction.invoke, but surrounded with this try-with-resources try ( TemporaryValueClosable c = - ((JinjavaInterpreter) context - .getELResolver() - .getValue(context, null, ExtendedParser.INTERPRETER)).getContext() + ((HasInterpreter) context).interpreter() + .getContext() .withPartialMacroEvaluation(false) ) { params = new Object[types.length]; diff --git a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstParameters.java b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstParameters.java index afc49dac6..6cb9fc4ee 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstParameters.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstParameters.java @@ -1,11 +1,10 @@ package com.hubspot.jinjava.el.ext.eager; +import com.hubspot.jinjava.el.HasInterpreter; import com.hubspot.jinjava.el.ext.DeferredParsingException; -import com.hubspot.jinjava.el.ext.ExtendedParser; import com.hubspot.jinjava.el.ext.IdentifierPreservationStrategy; import com.hubspot.jinjava.interpret.Context.TemporaryValueClosable; import com.hubspot.jinjava.interpret.DeferredValueException; -import com.hubspot.jinjava.interpret.JinjavaInterpreter; import de.odysseus.el.tree.Bindings; import de.odysseus.el.tree.impl.ast.AstNode; import de.odysseus.el.tree.impl.ast.AstParameters; @@ -41,9 +40,8 @@ private EagerAstParameters(List nodes, boolean convertedToEvalResultHol public Object[] eval(Bindings bindings, ELContext context) { try ( TemporaryValueClosable c = - ((JinjavaInterpreter) context - .getELResolver() - .getValue(context, null, ExtendedParser.INTERPRETER)).getContext() + ((HasInterpreter) context).interpreter() + .getContext() .withPartialMacroEvaluation(false) ) { try { diff --git a/src/main/java/com/hubspot/jinjava/el/ext/eager/EvalResultHolder.java b/src/main/java/com/hubspot/jinjava/el/ext/eager/EvalResultHolder.java index 425cc0ddd..b2bda607e 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/eager/EvalResultHolder.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/eager/EvalResultHolder.java @@ -1,10 +1,10 @@ package com.hubspot.jinjava.el.ext.eager; +import com.hubspot.jinjava.el.HasInterpreter; import com.hubspot.jinjava.el.ext.DeferredParsingException; import com.hubspot.jinjava.el.ext.ExtendedParser; import com.hubspot.jinjava.el.ext.IdentifierPreservationStrategy; import com.hubspot.jinjava.interpret.DeferredValueException; -import com.hubspot.jinjava.interpret.JinjavaInterpreter; import com.hubspot.jinjava.interpret.MetaContextVariables; import com.hubspot.jinjava.interpret.PartiallyDeferredValue; import com.hubspot.jinjava.util.EagerExpressionResolver; @@ -52,10 +52,7 @@ default Object checkEvalResultSize(ELContext context) { if ( evalResult instanceof Collection && ((Collection) evalResult).size() > 100 && // TODO make size configurable - ((JinjavaInterpreter) context - .getELResolver() - .getValue(context, null, ExtendedParser.INTERPRETER)).getContext() - .isDeferLargeObjects() + ((HasInterpreter) context).interpreter().getContext().isDeferLargeObjects() ) { throw new DeferredValueException("Collection too big"); } @@ -128,9 +125,7 @@ static String reconstructNode( if ( MetaContextVariables.isMetaContextVariable( name, - ((JinjavaInterpreter) context - .getELResolver() - .getValue(context, null, ExtendedParser.INTERPRETER)).getContext() + ((HasInterpreter) context).interpreter().getContext() ) ) { return name; diff --git a/src/main/java/com/hubspot/jinjava/interpret/Context.java b/src/main/java/com/hubspot/jinjava/interpret/Context.java index a2d83f983..980a3759e 100644 --- a/src/main/java/com/hubspot/jinjava/interpret/Context.java +++ b/src/main/java/com/hubspot/jinjava/interpret/Context.java @@ -18,6 +18,7 @@ import com.google.common.annotations.Beta; import com.google.common.collect.HashMultimap; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.SetMultimap; import com.google.common.collect.Sets; @@ -41,7 +42,6 @@ import com.hubspot.jinjava.util.ScopeMap; import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -192,7 +192,7 @@ public Context( : parent == null ? null : parent.getCurrentPathStack(); if (disabled == null) { - disabled = new HashMap<>(); + disabled = ImmutableMap.of(); } this.expTestLibrary = @@ -583,10 +583,11 @@ public void registerFilter(Filter f) { } public boolean isFunctionDisabled(String name) { - return ( - disabled != null && - disabled.getOrDefault(Library.FUNCTION, Collections.emptySet()).contains(name) - ); + if (disabled == null) { + return false; + } + Set disabledFunctions = disabled.get(Library.FUNCTION); + return disabledFunctions != null && disabledFunctions.contains(name); } public ELFunctionDefinition getFunction(String name) { @@ -606,10 +607,13 @@ public Collection getAllFunctions() { if (parent != null) { fns.addAll(parent.getAllFunctions()); } - - final Set disabledFunctions = disabled == null - ? new HashSet<>() - : disabled.getOrDefault(Library.FUNCTION, new HashSet<>()); + if (disabled == null) { + return fns; + } + Set disabledFunctions = disabled.get(Library.FUNCTION); + if (disabledFunctions == null) { + return fns; + } return fns .stream() .filter(f -> !disabledFunctions.contains(f.getName())) diff --git a/src/main/java/com/hubspot/jinjava/lib/filter/RenderFilter.java b/src/main/java/com/hubspot/jinjava/lib/filter/RenderFilter.java index 837c091ca..3cdf41db7 100644 --- a/src/main/java/com/hubspot/jinjava/lib/filter/RenderFilter.java +++ b/src/main/java/com/hubspot/jinjava/lib/filter/RenderFilter.java @@ -1,6 +1,5 @@ package com.hubspot.jinjava.lib.filter; -import com.hubspot.jinjava.JinjavaConfig; import com.hubspot.jinjava.doc.annotations.JinjavaDoc; import com.hubspot.jinjava.doc.annotations.JinjavaParam; import com.hubspot.jinjava.doc.annotations.JinjavaSnippet; @@ -35,10 +34,7 @@ public Object filter(Object var, JinjavaInterpreter interpreter, String... args) result = interpreter.renderFlat( Objects.toString(var), - NumberUtils.toLong( - firstArg, - JinjavaConfig.newBuilder().build().getMaxOutputSize() - ) + NumberUtils.toLong(firstArg, interpreter.getConfig().getMaxOutputSize()) ); } else { result = interpreter.renderFlat(Objects.toString(var)); diff --git a/src/main/java/com/hubspot/jinjava/lib/filter/ReverseFilter.java b/src/main/java/com/hubspot/jinjava/lib/filter/ReverseFilter.java index 8d946ac68..25fc8a22d 100644 --- a/src/main/java/com/hubspot/jinjava/lib/filter/ReverseFilter.java +++ b/src/main/java/com/hubspot/jinjava/lib/filter/ReverseFilter.java @@ -19,8 +19,10 @@ import com.hubspot.jinjava.doc.annotations.JinjavaParam; import com.hubspot.jinjava.doc.annotations.JinjavaSnippet; import com.hubspot.jinjava.interpret.JinjavaInterpreter; -import java.lang.reflect.Array; +import com.hubspot.jinjava.objects.collections.ArrayBacked; import java.util.Collection; +import java.util.Iterator; +import java.util.NoSuchElementException; @JinjavaDoc( value = "Reverse the object or return an iterator the iterates over it the other way round.", @@ -48,24 +50,11 @@ public Object filter(Object object, JinjavaInterpreter interpreter, String... ar } // collection if (object instanceof Collection) { - Object[] origin = ((Collection) object).toArray(); - int length = origin.length; - Object[] res = new Object[length]; - length--; - for (int i = 0; i <= length; i++) { - res[i] = origin[length - i]; - } - return res; + return ReverseArrayIterator.create(((Collection) object).toArray()); } // array if (object.getClass().isArray()) { - int length = Array.getLength(object); - Object[] res = new Object[length]; - length--; - for (int i = 0; i <= length; i++) { - res[i] = Array.get(object, length - i); - } - return res; + return ReverseArrayIterator.create((Object[]) object); } // string if (object instanceof String) { @@ -86,4 +75,37 @@ public Object filter(Object object, JinjavaInterpreter interpreter, String... ar public String getName() { return "reverse"; } + + static class ReverseArrayIterator implements Iterator, ArrayBacked { + + private final T[] array; + private int index; + + static ReverseArrayIterator create(T[] array) { + return new ReverseArrayIterator<>(array); + } + + private ReverseArrayIterator(T[] array) { + this.array = array; + index = array.length - 1; + } + + @Override + public T next() { + if (index < 0) { + throw new NoSuchElementException(); + } + return array[index--]; + } + + @Override + public boolean hasNext() { + return index >= 0; + } + + @Override + public T[] backingArray() { + return array; + } + } } diff --git a/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerCycleTag.java b/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerCycleTag.java index 41df6cf5c..0592ec3ad 100644 --- a/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerCycleTag.java +++ b/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerCycleTag.java @@ -2,7 +2,6 @@ import com.google.common.annotations.Beta; import com.google.common.collect.ImmutableMap; -import com.hubspot.jinjava.el.ext.ExtendedParser; import com.hubspot.jinjava.interpret.JinjavaInterpreter; import com.hubspot.jinjava.interpret.TemplateSyntaxException; import com.hubspot.jinjava.lib.tag.CycleTag; @@ -239,19 +238,17 @@ private static String getIsIterable(String var, int forIndex, TagToken tagToken) String tokenEnd = tagToken.getSymbols().getExpressionEndWithTag(); return ( String.format( - "%s if exptest:iterable.evaluate(%s, %s) %s", + "%s if exptest:iterable.evaluate(%s, null) %s", tokenStart, var, - ExtendedParser.INTERPRETER, tokenEnd ) + // modulo indexing String.format( - "{{ %s[%d %% filter:length.filter(%s, %s)] }}", + "{{ %s[%d %% filter:length.filter(%s, null)] }}", var, forIndex, - var, - ExtendedParser.INTERPRETER + var ) + String.format("%s else %s", tokenStart, tokenEnd) + String.format("{{ %s }}", var) + diff --git a/src/main/java/com/hubspot/jinjava/objects/DummyObject.java b/src/main/java/com/hubspot/jinjava/objects/DummyObject.java index 55b7d1d0e..3d082cea7 100644 --- a/src/main/java/com/hubspot/jinjava/objects/DummyObject.java +++ b/src/main/java/com/hubspot/jinjava/objects/DummyObject.java @@ -1,11 +1,12 @@ package com.hubspot.jinjava.objects; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; import java.util.Collection; import java.util.Map; import java.util.Set; -public class DummyObject implements Map, PyWrapper { +public class DummyObject implements Map, PyWrapper { @Override public int size() { @@ -33,7 +34,7 @@ public Object get(Object key) { } @Override - public Object put(String key, Object value) { + public Object put(Object key, Object value) { return new DummyObject(); } @@ -43,14 +44,14 @@ public Object remove(Object key) { } @Override - public void putAll(Map m) {} + public void putAll(Map m) {} @Override public void clear() {} @Override - public Set keySet() { - return null; + public Set keySet() { + return ImmutableSet.of(new DummyObject()); } @Override @@ -59,7 +60,7 @@ public Collection values() { } @Override - public Set> entrySet() { - return null; + public Set> entrySet() { + return ImmutableSet.of(Map.entry(new DummyObject(), new DummyObject())); } } diff --git a/src/main/java/com/hubspot/jinjava/objects/collections/ArrayBacked.java b/src/main/java/com/hubspot/jinjava/objects/collections/ArrayBacked.java new file mode 100644 index 000000000..34a3586f4 --- /dev/null +++ b/src/main/java/com/hubspot/jinjava/objects/collections/ArrayBacked.java @@ -0,0 +1,5 @@ +package com.hubspot.jinjava.objects.collections; + +public interface ArrayBacked { + T[] backingArray(); +} diff --git a/src/main/java/com/hubspot/jinjava/objects/collections/PySet.java b/src/main/java/com/hubspot/jinjava/objects/collections/PySet.java new file mode 100644 index 000000000..6c92560e2 --- /dev/null +++ b/src/main/java/com/hubspot/jinjava/objects/collections/PySet.java @@ -0,0 +1,38 @@ +package com.hubspot.jinjava.objects.collections; + +import com.google.common.collect.ForwardingSet; +import com.hubspot.jinjava.objects.PyWrapper; +import java.util.Objects; +import java.util.Set; + +public class PySet extends ForwardingSet implements PyWrapper { + + private boolean computingHashCode = false; + private final Set set; + + public PySet(Set set) { + this.set = set; + } + + @Override + protected Set delegate() { + return set; + } + + /** + * This is not thread-safe + * @return hashCode, preventing recursion + */ + @Override + public int hashCode() { + if (computingHashCode) { + return Objects.hashCode(null); + } + try { + computingHashCode = true; + return super.hashCode(); + } finally { + computingHashCode = false; + } + } +} diff --git a/src/main/java/com/hubspot/jinjava/objects/collections/SizeLimitingPySet.java b/src/main/java/com/hubspot/jinjava/objects/collections/SizeLimitingPySet.java new file mode 100644 index 000000000..d7f100e2a --- /dev/null +++ b/src/main/java/com/hubspot/jinjava/objects/collections/SizeLimitingPySet.java @@ -0,0 +1,68 @@ +package com.hubspot.jinjava.objects.collections; + +import com.hubspot.jinjava.interpret.CollectionTooBigException; +import com.hubspot.jinjava.interpret.JinjavaInterpreter; +import com.hubspot.jinjava.interpret.TemplateError; +import com.hubspot.jinjava.interpret.TemplateError.ErrorReason; +import com.hubspot.jinjava.interpret.TemplateError.ErrorType; +import com.hubspot.jinjava.objects.PyWrapper; +import java.util.Collection; +import java.util.Set; +import javax.annotation.Nonnull; + +public class SizeLimitingPySet extends PySet implements PyWrapper { + + private int maxSize; + private boolean hasWarned; + + public SizeLimitingPySet(Set set, int maxSize) { + super(set); + if (set == null) { + throw new IllegalArgumentException("set is null"); + } + if (maxSize <= 0) { + throw new IllegalArgumentException("maxSize must be >= 1"); + } + + this.maxSize = maxSize; + if (set.size() > maxSize) { + throw new CollectionTooBigException(set.size(), maxSize); + } + } + + @Override + public boolean add(Object element) { + checkSize(size() + 1); + return super.add(element); + } + + @Override + public boolean addAll(@Nonnull Collection elements) { + if (elements == null || elements.isEmpty()) { + return false; + } + checkSize(size() + elements.size()); + return super.addAll(elements); + } + + private void checkSize(int newSize) { + if (newSize > maxSize) { + throw new CollectionTooBigException(newSize, maxSize); + } else if (!hasWarned && newSize >= maxSize * 0.9) { + hasWarned = true; + JinjavaInterpreter + .getCurrent() + .addError( + new TemplateError( + ErrorType.WARNING, + ErrorReason.COLLECTION_TOO_BIG, + String.format("Set is at 90%% of max size (%d of %d)", newSize, maxSize), + null, + -1, + -1, + new CollectionTooBigException(newSize, maxSize) + ) + ); + } + } +} diff --git a/src/main/java/com/hubspot/jinjava/util/EagerExpressionResolver.java b/src/main/java/com/hubspot/jinjava/util/EagerExpressionResolver.java index 88763eb73..1a17c5017 100644 --- a/src/main/java/com/hubspot/jinjava/util/EagerExpressionResolver.java +++ b/src/main/java/com/hubspot/jinjava/util/EagerExpressionResolver.java @@ -13,6 +13,7 @@ import com.hubspot.jinjava.interpret.PartiallyDeferredValue; import com.hubspot.jinjava.interpret.TemplateSyntaxException; import com.hubspot.jinjava.interpret.UnknownTokenException; +import com.hubspot.jinjava.objects.collections.ArrayBacked; import com.hubspot.jinjava.objects.serialization.PyishObjectMapper; import com.hubspot.jinjava.objects.serialization.PyishSerializable; import com.hubspot.jinjava.tree.ExpressionNode; @@ -328,6 +329,9 @@ private static boolean isResolvableObjectRec( if (isPrimitive(val)) { return true; } + if (val instanceof ArrayBacked arrayBacked) { + val = arrayBacked.backingArray(); + } try { if (val instanceof Collection || val instanceof Map) { int size = val instanceof Collection diff --git a/src/main/java/com/hubspot/jinjava/util/EagerReconstructionUtils.java b/src/main/java/com/hubspot/jinjava/util/EagerReconstructionUtils.java index 385fed453..cef5d396f 100644 --- a/src/main/java/com/hubspot/jinjava/util/EagerReconstructionUtils.java +++ b/src/main/java/com/hubspot/jinjava/util/EagerReconstructionUtils.java @@ -466,12 +466,11 @@ public static String buildSetTag( return ""; } Map> disabled = interpreter.getConfig().getDisabled(); - if ( - disabled != null && - disabled.containsKey(Library.TAG) && - disabled.get(Library.TAG).contains(SetTag.TAG_NAME) - ) { - throw new DisabledException("set tag disabled"); + if (disabled != null) { + Set disabledTags = disabled.get(Library.TAG); + if (disabledTags != null && disabledTags.contains(SetTag.TAG_NAME)) { + throw new DisabledException("set tag disabled"); + } } StringJoiner vars = new StringJoiner(","); @@ -538,12 +537,11 @@ public static String buildBlockSetTag( boolean registerDeferredToken ) { Map> disabled = interpreter.getConfig().getDisabled(); - if ( - disabled != null && - disabled.containsKey(Library.TAG) && - disabled.get(Library.TAG).contains(SetTag.TAG_NAME) - ) { - throw new DisabledException("set tag disabled"); + if (disabled != null) { + Set disabledTags = disabled.get(Library.TAG); + if (disabledTags != null && disabledTags.contains(SetTag.TAG_NAME)) { + throw new DisabledException("set tag disabled"); + } } LengthLimitingStringJoiner blockSetTokenBuilder = new LengthLimitingStringJoiner( @@ -593,12 +591,11 @@ public static String buildDoUpdateTag( JinjavaInterpreter interpreter ) { Map> disabled = interpreter.getConfig().getDisabled(); - if ( - disabled != null && - disabled.containsKey(Library.TAG) && - disabled.get(Library.TAG).contains(DoTag.TAG_NAME) - ) { - throw new DisabledException("do tag disabled"); + if (disabled != null) { + Set disabledTags = disabled.get(Library.TAG); + if (disabledTags != null && disabledTags.contains(DoTag.TAG_NAME)) { + throw new DisabledException("do tag disabled"); + } } return new LengthLimitingStringJoiner(interpreter.getConfig().getMaxOutputSize(), " ") .add(interpreter.getConfig().getTokenScannerSymbols().getExpressionStartWithTag()) @@ -667,12 +664,11 @@ public static String wrapInTag( boolean registerDeferredToken ) { Map> disabled = interpreter.getConfig().getDisabled(); - if ( - disabled != null && - disabled.containsKey(Library.TAG) && - disabled.get(Library.TAG).contains(tagNameToWrap) - ) { - throw new DisabledException(String.format("%s tag disabled", tagNameToWrap)); + if (disabled != null) { + Set disabledTags = disabled.get(Library.TAG); + if (disabledTags != null && disabledTags.contains(tagNameToWrap)) { + throw new DisabledException(String.format("%s tag disabled", tagNameToWrap)); + } } StringJoiner startTokenBuilder = new StringJoiner(" "); StringJoiner endTokenBuilder = new StringJoiner(" "); diff --git a/src/test/java/com/hubspot/jinjava/BaseJinjavaTest.java b/src/test/java/com/hubspot/jinjava/BaseJinjavaTest.java index 2579b2d5d..c0c88e852 100644 --- a/src/test/java/com/hubspot/jinjava/BaseJinjavaTest.java +++ b/src/test/java/com/hubspot/jinjava/BaseJinjavaTest.java @@ -1,29 +1,42 @@ package com.hubspot.jinjava; +import com.hubspot.jinjava.el.ext.AllowlistMethodValidator; +import com.hubspot.jinjava.el.ext.AllowlistReturnTypeValidator; +import com.hubspot.jinjava.el.ext.MethodValidatorConfig; +import com.hubspot.jinjava.el.ext.ReturnTypeValidatorConfig; import org.junit.Before; public abstract class BaseJinjavaTest { + public static final AllowlistMethodValidator METHOD_VALIDATOR = + AllowlistMethodValidator.create( + MethodValidatorConfig + .builder() + .addDefaultAllowlistGroups() + .addAllowedDeclaredMethodsFromCanonicalClassPrefixes( + "com.hubspot.jinjava.testobjects" + ) + .build() + ); + public static final AllowlistReturnTypeValidator RETURN_TYPE_VALIDATOR = + AllowlistReturnTypeValidator.create( + ReturnTypeValidatorConfig + .builder() + .addDefaultAllowlistGroups() + .addAllowedCanonicalClassPrefixes("com.hubspot.jinjava.testobjects") + .build() + ); public Jinjava jinjava; @Before public void baseSetup() { - jinjava = - new Jinjava( - BaseJinjavaTest - .newConfigBuilder() - .withLegacyOverrides( - LegacyOverrides - .newBuilder() - .withUsePyishObjectMapper(true) - .withKeepNullableLoopValues(true) - .build() - ) - .build() - ); + jinjava = new Jinjava(BaseJinjavaTest.newConfigBuilder().build()); } public static JinjavaConfig.Builder newConfigBuilder() { - return JinjavaConfig.builder(); + return JinjavaConfig + .builder() + .withMethodValidator(METHOD_VALIDATOR) + .withReturnTypeValidator(RETURN_TYPE_VALIDATOR); } } diff --git a/src/test/java/com/hubspot/jinjava/EagerTest.java b/src/test/java/com/hubspot/jinjava/EagerTest.java index e408d2cc2..78d196aa0 100644 --- a/src/test/java/com/hubspot/jinjava/EagerTest.java +++ b/src/test/java/com/hubspot/jinjava/EagerTest.java @@ -216,7 +216,7 @@ public void itPreserveDeferredVariableResolvingEqualToInOrCondition() { assertThat(output) .isEqualTo( - "{% if false || exptest:equalto.evaluate('a', ____int3rpr3t3r____, deferred) %}preserved{% endif %}" + "{% if false || exptest:equalto.evaluate('a', null, deferred) %}preserved{% endif %}" ); assertThat(interpreter.getErrors()).isEmpty(); localContext.put("deferred", "a"); @@ -307,8 +307,7 @@ public void itPreservesForTag() { @Test public void itPreservesFilters() { String output = interpreter.render("{{ deferred|capitalize }}"); - assertThat(output) - .isEqualTo("{{ filter:capitalize.filter(deferred, ____int3rpr3t3r____) }}"); + assertThat(output).isEqualTo("{{ filter:capitalize.filter(deferred, null) }}"); assertThat(interpreter.getErrors()).isEmpty(); localContext.put("deferred", "foo"); assertThat(interpreter.render(output)).isEqualTo("Foo"); @@ -318,17 +317,14 @@ public void itPreservesFilters() { public void itPreservesFunctions() { String output = interpreter.render("{{ deferred|datetimeformat('%B %e, %Y') }}"); assertThat(output) - .isEqualTo( - "{{ filter:datetimeformat.filter(deferred, ____int3rpr3t3r____, '%B %e, %Y') }}" - ); + .isEqualTo("{{ filter:datetimeformat.filter(deferred, null, '%B %e, %Y') }}"); assertThat(interpreter.getErrors()).isEmpty(); } @Test public void itPreservesRandomness() { String output = interpreter.render("{{ [1, 2, 3]|shuffle }}"); - assertThat(output) - .isEqualTo("{{ filter:shuffle.filter([1, 2, 3], ____int3rpr3t3r____) }}"); + assertThat(output).isEqualTo("{{ filter:shuffle.filter([1, 2, 3], null) }}"); assertThat(interpreter.getErrors()).isEmpty(); } diff --git a/src/test/java/com/hubspot/jinjava/FilterOverrideTest.java b/src/test/java/com/hubspot/jinjava/FilterOverrideTest.java index e49556552..f77ccbf70 100644 --- a/src/test/java/com/hubspot/jinjava/FilterOverrideTest.java +++ b/src/test/java/com/hubspot/jinjava/FilterOverrideTest.java @@ -10,7 +10,7 @@ public class FilterOverrideTest { @Test public void itAllowsUsersToOverrideBuiltInFilters() { - Jinjava jinjava = new Jinjava(); + Jinjava jinjava = new Jinjava(BaseJinjavaTest.newConfigBuilder().build()); String template = "{{ 5 | add(6) }}"; assertThat(jinjava.render(template, new HashMap<>())).isEqualTo("11"); diff --git a/src/test/java/com/hubspot/jinjava/el/ExpressionResolverTest.java b/src/test/java/com/hubspot/jinjava/el/ExpressionResolverTest.java index a690adbd1..83347281a 100644 --- a/src/test/java/com/hubspot/jinjava/el/ExpressionResolverTest.java +++ b/src/test/java/com/hubspot/jinjava/el/ExpressionResolverTest.java @@ -27,6 +27,7 @@ import java.util.Objects; import java.util.Set; import java.util.function.Supplier; +import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -39,9 +40,15 @@ public class ExpressionResolverTest { @Before public void setup() { - jinjava = new Jinjava(); + jinjava = new Jinjava(BaseJinjavaTest.newConfigBuilder().build()); interpreter = jinjava.newInterpreter(); context = interpreter.getContext(); + JinjavaInterpreter.pushCurrent(interpreter); + } + + @After + public void teardown() { + JinjavaInterpreter.popCurrent(); } @Test @@ -251,7 +258,7 @@ public void unknownProperty() { interpreter.resolveELExpression("foo", 23); assertThat(interpreter.getErrorsCopy()).isEmpty(); - context.put("foo", new Object()); + context.put("foo", ""); interpreter.resolveELExpression("foo.bar", 23); assertThat(interpreter.getErrorsCopy()).hasSize(1); @@ -308,7 +315,10 @@ public void blackListedMethods() { assertThat(interpreter.getErrorsCopy()).isNotEmpty(); TemplateError e = interpreter.getErrorsCopy().get(0); - assertThat(e.getMessage()).contains("Cannot find method 'wait'"); + assertThat(e.getMessage()) + .contains( + "Cannot find method wait with 0 parameters in class com.hubspot.jinjava.testobjects.ExpressionResolverTestObjects$MyClass" + ); } @Test @@ -318,7 +328,10 @@ public void itWillNotReturnClassObjects() { assertThat(interpreter.getErrorsCopy()).isNotEmpty(); TemplateError e = interpreter.getErrorsCopy().get(0); - assertThat(e.getMessage()).contains("Cannot find method 'getClass'"); + assertThat(e.getMessage()) + .contains( + "Cannot find method getClass with 0 parameters in class com.hubspot.jinjava.testobjects.ExpressionResolverTestObjects$MyClass" + ); } @Test @@ -390,6 +403,7 @@ public void itBlocksDisabledFunctions() { ); String template = "hi {% for i in range(1, 3) %}{{i}} {% endfor %}"; + JinjavaInterpreter.popCurrent(); String rendered = jinjava.render(template, context); assertEquals("hi 1 2 ", rendered); @@ -547,15 +561,11 @@ public void itResolvesAlternateExpTestSyntax() { assertThat(interpreter.render("{% if 2 is even %}yes{% endif %}")).isEqualTo("yes"); assertThat( - interpreter.render( - "{% if exptest:even.evaluate(2, ____int3rpr3t3r____) %}yes{% endif %}" - ) + interpreter.render("{% if exptest:even.evaluate(2, null) %}yes{% endif %}") ) .isEqualTo("yes"); assertThat( - interpreter.render( - "{% if exptest:false.evaluate(false, ____int3rpr3t3r____) %}yes{% endif %}" - ) + interpreter.render("{% if exptest:false.evaluate(false, null) %}yes{% endif %}") ) .isEqualTo("yes"); } @@ -563,15 +573,11 @@ public void itResolvesAlternateExpTestSyntax() { @Test public void itResolvesAlternateExpTestSyntaxForTrueAndFalseExpTests() { assertThat( - interpreter.render( - "{% if exptest:false.evaluate(false, ____int3rpr3t3r____) %}yes{% endif %}" - ) + interpreter.render("{% if exptest:false.evaluate(false, null) %}yes{% endif %}") ) .isEqualTo("yes"); assertThat( - interpreter.render( - "{% if exptest:true.evaluate(true, ____int3rpr3t3r____) %}yes{% endif %}" - ) + interpreter.render("{% if exptest:true.evaluate(true, null) %}yes{% endif %}") ) .isEqualTo("yes"); } @@ -579,14 +585,12 @@ public void itResolvesAlternateExpTestSyntaxForTrueAndFalseExpTests() { @Test public void itResolvesAlternateExpTestSyntaxForInExpTests() { assertThat( - interpreter.render( - "{% if exptest:in.evaluate(1, ____int3rpr3t3r____, [1]) %}yes{% endif %}" - ) + interpreter.render("{% if exptest:in.evaluate(1, null, [1]) %}yes{% endif %}") ) .isEqualTo("yes"); assertThat( interpreter.render( - "{% if exptest:in.evaluate(2, ____int3rpr3t3r____, [1]) %}yes{% else %}no{% endif %}" + "{% if exptest:in.evaluate(2, null, [1]) %}yes{% else %}no{% endif %}" ) ) .isEqualTo("no"); diff --git a/src/test/java/com/hubspot/jinjava/el/ext/AstDictTest.java b/src/test/java/com/hubspot/jinjava/el/ext/AstDictTest.java index f0ebff5fd..c5bbc0d6d 100644 --- a/src/test/java/com/hubspot/jinjava/el/ext/AstDictTest.java +++ b/src/test/java/com/hubspot/jinjava/el/ext/AstDictTest.java @@ -3,76 +3,104 @@ import static org.assertj.core.api.Assertions.assertThat; import com.google.common.collect.ImmutableMap; -import com.hubspot.jinjava.Jinjava; +import com.hubspot.jinjava.BaseJinjavaTest; import com.hubspot.jinjava.interpret.JinjavaInterpreter; import com.hubspot.jinjava.interpret.TemplateError.ErrorType; import com.hubspot.jinjava.testobjects.AstDictTestObjects; import java.util.Set; -import org.junit.Before; import org.junit.Test; -public class AstDictTest { - - private JinjavaInterpreter interpreter; - - @Before - public void setup() { - interpreter = new Jinjava().newInterpreter(); - } +public class AstDictTest extends BaseJinjavaTest { @Test public void itGetsDictValues() { - interpreter.getContext().put("foo", ImmutableMap.of("bar", "test")); - assertThat(interpreter.resolveELExpression("foo.bar", -1)).isEqualTo("test"); + try ( + var a = JinjavaInterpreter.closeablePushCurrent(jinjava.newInterpreter()).get() + ) { + JinjavaInterpreter interpreter = a.value(); + interpreter.getContext().put("foo", ImmutableMap.of("bar", "test")); + assertThat(interpreter.resolveELExpression("foo.bar", -1)).isEqualTo("test"); + } } @Test public void itGetsDictValuesWithEnumKeys() { - interpreter.getContext().put("foo", ImmutableMap.of(ErrorType.FATAL, "test")); - assertThat(interpreter.resolveELExpression("foo.FATAL", -1)).isEqualTo("test"); + try ( + var a = JinjavaInterpreter.closeablePushCurrent(jinjava.newInterpreter()).get() + ) { + JinjavaInterpreter interpreter = a.value(); + interpreter.getContext().put("foo", ImmutableMap.of(ErrorType.FATAL, "test")); + assertThat(interpreter.resolveELExpression("foo.FATAL", -1)).isEqualTo("test"); + } } @Test public void itGetsDictValuesWithEnumKeysUsingToString() { - interpreter - .getContext() - .put("foo", ImmutableMap.of(AstDictTestObjects.TestEnum.BAR, "test")); - assertThat(interpreter.resolveELExpression("foo.barName", -1)).isEqualTo("test"); + try ( + var a = JinjavaInterpreter.closeablePushCurrent(jinjava.newInterpreter()).get() + ) { + JinjavaInterpreter interpreter = a.value(); + interpreter + .getContext() + .put("foo", ImmutableMap.of(AstDictTestObjects.TestEnum.BAR, "test")); + assertThat(interpreter.resolveELExpression("foo.barName", -1)).isEqualTo("test"); + } } @Test public void itDoesItemsMethodCall() { - interpreter - .getContext() - .put("foo", ImmutableMap.of(AstDictTestObjects.TestEnum.BAR, "test")); - assertThat(interpreter.resolveELExpression("foo.items()", -1)) - .isInstanceOf(Set.class); + try ( + var a = JinjavaInterpreter.closeablePushCurrent(jinjava.newInterpreter()).get() + ) { + JinjavaInterpreter interpreter = a.value(); + interpreter + .getContext() + .put("foo", ImmutableMap.of(AstDictTestObjects.TestEnum.BAR, "test")); + assertThat(interpreter.resolveELExpression("foo.items()", -1)) + .isInstanceOf(Set.class); + } } @Test public void itDoesKeysMethodCall() { - interpreter - .getContext() - .put("foo", ImmutableMap.of(AstDictTestObjects.TestEnum.BAR, "test")); - assertThat(interpreter.resolveELExpression("foo.keys()", -1)).isInstanceOf(Set.class); + try ( + var a = JinjavaInterpreter.closeablePushCurrent(jinjava.newInterpreter()).get() + ) { + JinjavaInterpreter interpreter = a.value(); + interpreter + .getContext() + .put("foo", ImmutableMap.of(AstDictTestObjects.TestEnum.BAR, "test")); + assertThat(interpreter.resolveELExpression("foo.keys()", -1)) + .isInstanceOf(Set.class); + } } @Test public void itHandlesEmptyMaps() { - interpreter.getContext().put("foo", ImmutableMap.of()); - assertThat(interpreter.resolveELExpression("foo.FATAL", -1)).isNull(); - assertThat(interpreter.getErrors()).isEmpty(); + try ( + var a = JinjavaInterpreter.closeablePushCurrent(jinjava.newInterpreter()).get() + ) { + JinjavaInterpreter interpreter = a.value(); + interpreter.getContext().put("foo", ImmutableMap.of()); + assertThat(interpreter.resolveELExpression("foo.FATAL", -1)).isNull(); + assertThat(interpreter.getErrors()).isEmpty(); + } } @Test public void itGetsDictValuesWithEnumKeysInObjects() { - interpreter - .getContext() - .put( - "test", - new AstDictTestObjects.TestClass(ImmutableMap.of(ErrorType.FATAL, "test")) - ); - assertThat(interpreter.resolveELExpression("test.my_map.FATAL", -1)) - .isEqualTo("test"); + try ( + var a = JinjavaInterpreter.closeablePushCurrent(jinjava.newInterpreter()).get() + ) { + JinjavaInterpreter interpreter = a.value(); + interpreter + .getContext() + .put( + "test", + new AstDictTestObjects.TestClass(ImmutableMap.of(ErrorType.FATAL, "test")) + ); + assertThat(interpreter.resolveELExpression("test.my_map.FATAL", -1)) + .isEqualTo("test"); + } } } diff --git a/src/test/java/com/hubspot/jinjava/el/ext/ExtendedParserTest.java b/src/test/java/com/hubspot/jinjava/el/ext/ExtendedParserTest.java index e839d453e..228128248 100644 --- a/src/test/java/com/hubspot/jinjava/el/ext/ExtendedParserTest.java +++ b/src/test/java/com/hubspot/jinjava/el/ext/ExtendedParserTest.java @@ -11,6 +11,7 @@ import de.odysseus.el.tree.impl.ast.AstMethod; import de.odysseus.el.tree.impl.ast.AstNested; import de.odysseus.el.tree.impl.ast.AstNode; +import de.odysseus.el.tree.impl.ast.AstNull; import de.odysseus.el.tree.impl.ast.AstParameters; import de.odysseus.el.tree.impl.ast.AstString; import org.assertj.core.api.Assertions; @@ -136,9 +137,7 @@ public void itResolvesAlternateExpTestSyntax() { assertThat(regularSyntax).isInstanceOf(AstMethod.class); assertThat(regularSyntax.getChild(0)).isInstanceOf(AstDot.class); assertThat(regularSyntax.getChild(1)).isInstanceOf(AstParameters.class); - AstNode alternateSyntax = buildExpressionNodes( - "#{exptest:even.evaluate(2, ____int3rpr3t3r____)}" - ); + AstNode alternateSyntax = buildExpressionNodes("#{exptest:even.evaluate(2, null)}"); assertThat(alternateSyntax).isInstanceOf(AstMethod.class); assertThat(alternateSyntax.getChild(0)).isInstanceOf(AstDot.class); @@ -147,16 +146,12 @@ public void itResolvesAlternateExpTestSyntax() { @Test public void itResolvesAlternateExpTestSyntaxForTrueAndFalseExpTests() { - AstNode falseExpTest = buildExpressionNodes( - "#{exptest:false.evaluate(2, ____int3rpr3t3r____)}" - ); + AstNode falseExpTest = buildExpressionNodes("#{exptest:false.evaluate(2, null)}"); assertThat(falseExpTest).isInstanceOf(AstMethod.class); assertThat(falseExpTest.getChild(0)).isInstanceOf(AstDot.class); assertThat(falseExpTest.getChild(1)).isInstanceOf(AstParameters.class); - AstNode trueExpTest = buildExpressionNodes( - "#{exptest:true.evaluate(2, ____int3rpr3t3r____)}" - ); + AstNode trueExpTest = buildExpressionNodes("#{exptest:true.evaluate(2, null)}"); assertThat(trueExpTest).isInstanceOf(AstMethod.class); assertThat(trueExpTest.getChild(0)).isInstanceOf(AstDot.class); assertThat(trueExpTest.getChild(1)).isInstanceOf(AstParameters.class); @@ -164,9 +159,7 @@ public void itResolvesAlternateExpTestSyntaxForTrueAndFalseExpTests() { @Test public void itResolvesAlternateExpTestSyntaxForInExpTest() { - AstNode inExpTest = buildExpressionNodes( - "#{exptest:in.evaluate(2, ____int3rpr3t3r____, [])}" - ); + AstNode inExpTest = buildExpressionNodes("#{exptest:in.evaluate(2, null, [])}"); assertThat(inExpTest).isInstanceOf(AstMethod.class); assertThat(inExpTest.getChild(0)).isInstanceOf(AstDot.class); assertThat(inExpTest.getChild(1)).isInstanceOf(AstParameters.class); @@ -183,12 +176,10 @@ private void assertForExpression( AstParameters astParameters = (AstParameters) astNode.getChild(1); assertThat(astParameters.getChild(0)).isInstanceOf(AstString.class); - assertThat(astParameters.getChild(1)).isInstanceOf(AstIdentifier.class); + assertThat(astParameters.getChild(1)).isInstanceOf(AstNull.class); assertThat(astParameters.getChild(2)).isInstanceOf(AstString.class); assertThat(astParameters.getChild(0).eval(null, null)).isEqualTo(leftExpected); - assertThat(((AstIdentifier) astParameters.getChild(1)).getName()) - .isEqualTo("____int3rpr3t3r____"); assertThat(astParameters.getChild(2).eval(null, null)).isEqualTo(rightExpected); } diff --git a/src/test/java/com/hubspot/jinjava/el/ext/JinjavaBeanELResolverTest.java b/src/test/java/com/hubspot/jinjava/el/ext/JinjavaBeanELResolverTest.java index 5a57abd58..0456b1491 100644 --- a/src/test/java/com/hubspot/jinjava/el/ext/JinjavaBeanELResolverTest.java +++ b/src/test/java/com/hubspot/jinjava/el/ext/JinjavaBeanELResolverTest.java @@ -2,19 +2,14 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; -import com.fasterxml.jackson.databind.ObjectMapper; -import com.google.common.collect.ImmutableSet; -import com.hubspot.jinjava.JinjavaConfig; +import com.hubspot.jinjava.BaseJinjavaTest; +import com.hubspot.jinjava.Jinjava; import com.hubspot.jinjava.el.JinjavaELContext; import com.hubspot.jinjava.interpret.AutoCloseableSupplier; import com.hubspot.jinjava.interpret.JinjavaInterpreter; import com.hubspot.jinjava.testobjects.JinjavaBeanELResolverTestObjects; -import java.util.List; import javax.el.ELContext; -import javax.el.MethodNotFoundException; import javax.el.PropertyNotFoundException; import org.junit.Before; import org.junit.Test; @@ -23,196 +18,134 @@ public class JinjavaBeanELResolverTest { private JinjavaBeanELResolver jinjavaBeanELResolver; private ELContext elContext; - - JinjavaInterpreter interpreter = mock(JinjavaInterpreter.class); - JinjavaConfig config = mock(JinjavaConfig.class); + private Jinjava jinjava; @Before public void setUp() throws Exception { jinjavaBeanELResolver = new JinjavaBeanELResolver(); elContext = new JinjavaELContext(); - when(interpreter.getConfig()).thenReturn(config); + jinjava = new Jinjava(BaseJinjavaTest.newConfigBuilder().build()); } @Test public void itInvokesProperStringReplace() { - assertThat( - jinjavaBeanELResolver.invoke( - elContext, - "abcd", - "replace", - null, - new Object[] { "abcd", "efgh" } + try ( + var a = JinjavaInterpreter.closeablePushCurrent(jinjava.newInterpreter()).get() + ) { + assertThat( + jinjavaBeanELResolver.invoke( + elContext, + "abcd", + "replace", + null, + new Object[] { "abcd", "efgh" } + ) ) - ) - .isEqualTo("efgh"); - assertThat( - jinjavaBeanELResolver.invoke( - elContext, - "abcd", - "replace", - null, - new Object[] { 'a', 'e' } + .isEqualTo("efgh"); + assertThat( + jinjavaBeanELResolver.invoke( + elContext, + "abcd", + "replace", + null, + new Object[] { 'a', 'e' } + ) ) - ) - .isEqualTo("ebcd"); + .isEqualTo("ebcd"); + } } @Test public void itInvokesBestMethodWithSingleParam() { - JinjavaBeanELResolverTestObjects.TempItInvokesBestMethodWithSingleParam var = - new JinjavaBeanELResolverTestObjects.TempItInvokesBestMethodWithSingleParam(); - assertThat( - jinjavaBeanELResolver.invoke(elContext, var, "getResult", null, new Object[] { 1 }) - ) - .isEqualTo("int"); - assertThat( - jinjavaBeanELResolver.invoke( - elContext, - var, - "getResult", - null, - new Object[] { "1" } - ) - ) - .isEqualTo("String"); - assertThat( - jinjavaBeanELResolver.invoke( - elContext, - var, - "getResult", - null, - new Object[] { new Object() } - ) - ) - .isEqualTo("Object"); - } - - @Test - public void itPrefersPrimitives() { - JinjavaBeanELResolverTestObjects.TempItPrefersPrimitives var = - new JinjavaBeanELResolverTestObjects.TempItPrefersPrimitives(); - assertThat( - jinjavaBeanELResolver.invoke( - elContext, - var, - "getResult", - null, - new Object[] { 1, 2 } - ) - ) - .isEqualTo("int Integer"); - assertThat( - jinjavaBeanELResolver.invoke( - elContext, - var, - "getResult", - null, - new Object[] { 1, Integer.valueOf(2) } - ) - ) - .isEqualTo("int Integer"); // should be "int object", but we can't figure that out - assertThat( - jinjavaBeanELResolver.invoke( - elContext, - var, - "getResult", - null, - new Object[] { Integer.valueOf(1), 2 } + try ( + var a = JinjavaInterpreter.closeablePushCurrent(jinjava.newInterpreter()).get() + ) { + JinjavaBeanELResolverTestObjects.TempItInvokesBestMethodWithSingleParam var = + new JinjavaBeanELResolverTestObjects.TempItInvokesBestMethodWithSingleParam(); + assertThat( + jinjavaBeanELResolver.invoke( + elContext, + var, + "getResult", + null, + new Object[] { 1 } + ) ) - ) - .isEqualTo("int Integer"); // should be "Number int", but we can't figure that out - } - - @Test - public void itThrowsExceptionWhenMethodIsRestrictedFromConfig() { - JinjavaInterpreter.pushCurrent(interpreter); - when(config.getRestrictedMethods()).thenReturn(ImmutableSet.of("foo")); - assertThatThrownBy(() -> - jinjavaBeanELResolver.invoke(elContext, "abcd", "foo", null, new Object[] { 1 }) + .isEqualTo("int"); + assertThat( + jinjavaBeanELResolver.invoke( + elContext, + var, + "getResult", + null, + new Object[] { "1" } + ) ) - .isInstanceOf(MethodNotFoundException.class) - .hasMessageStartingWith("Cannot find method 'foo'"); - JinjavaInterpreter.popCurrent(); - } - - @Test - public void itThrowsExceptionWhenPropertyIsRestrictedFromConfig() { - JinjavaInterpreter.pushCurrent(interpreter); - when(config.getRestrictedProperties()).thenReturn(ImmutableSet.of("property1")); - assertThatThrownBy(() -> - jinjavaBeanELResolver.getValue(elContext, "abcd", "property1") + .isEqualTo("String"); + assertThat( + jinjavaBeanELResolver.invoke( + elContext, + var, + "getResult", + null, + new Object[] { new Object() } + ) ) - .isInstanceOf(PropertyNotFoundException.class) - .hasMessageStartingWith("Could not find property"); - JinjavaInterpreter.popCurrent(); - } - - @Test - public void itDoesNotAllowAccessingPropertiesOfInterpreter() { - try ( - AutoCloseableSupplier.AutoCloseableImpl c = JinjavaInterpreter - .closeablePushCurrent(interpreter) - .get() - ) { - assertThat(jinjavaBeanELResolver.getValue(elContext, interpreter, "config")) - .isNull(); + .isEqualTo("Object"); } } @Test - public void itDoesNotGettingFromObjectMapper() { + public void itPrefersPrimitives() { try ( - AutoCloseableSupplier.AutoCloseableImpl c = JinjavaInterpreter - .closeablePushCurrent(interpreter) - .get() + var a = JinjavaInterpreter.closeablePushCurrent(jinjava.newInterpreter()).get() ) { + JinjavaBeanELResolverTestObjects.TempItPrefersPrimitives var = + new JinjavaBeanELResolverTestObjects.TempItPrefersPrimitives(); assertThat( - jinjavaBeanELResolver.getValue(elContext, new ObjectMapper(), "dateFormat") + jinjavaBeanELResolver.invoke( + elContext, + var, + "getResult", + null, + new Object[] { 1, 2 } + ) ) - .isNull(); - } - } - - @Test - public void itDoesNotAllowInvokingFromObjectMapper() throws NoSuchMethodException { - try ( - AutoCloseableSupplier.AutoCloseableImpl c = JinjavaInterpreter - .closeablePushCurrent(interpreter) - .get() - ) { - ObjectMapper objectMapper = new ObjectMapper(); - assertThatThrownBy(() -> - jinjavaBeanELResolver.invoke( - elContext, - objectMapper, - "getDateFormat", - new Class[] {}, - new Object[] {} - ) + .isEqualTo("int Integer"); + assertThat( + jinjavaBeanELResolver.invoke( + elContext, + var, + "getResult", + null, + new Object[] { 1, Integer.valueOf(2) } + ) + ) + .isEqualTo("int Integer"); // should be "int object", but we can't figure that out + assertThat( + jinjavaBeanELResolver.invoke( + elContext, + var, + "getResult", + null, + new Object[] { Integer.valueOf(1), 2 } ) - .isInstanceOf(MethodNotFoundException.class); + ) + .isEqualTo("int Integer"); // should be "Number int", but we can't figure that out } } @Test - public void itDoesNotAllowInvokingFromMethod() throws NoSuchMethodException { + public void itDoesNotAllowAccessingPropertiesOfInterpreter() { try ( - AutoCloseableSupplier.AutoCloseableImpl c = JinjavaInterpreter - .closeablePushCurrent(interpreter) + AutoCloseableSupplier.AutoCloseableImpl a = JinjavaInterpreter + .closeablePushCurrent(jinjava.newInterpreter()) .get() ) { - List list = List.of("foo"); assertThatThrownBy(() -> - jinjavaBeanELResolver.invoke( - elContext, - list.getClass().getMethod("get", int.class), - "invoke", - new Class[] { Object.class, Object[].class }, - new Object[] { list, 0 } - ) + jinjavaBeanELResolver.getValue(elContext, a.value(), "config") ) - .isInstanceOf(MethodNotFoundException.class); + .isInstanceOf(PropertyNotFoundException.class); } } } diff --git a/src/test/java/com/hubspot/jinjava/el/ext/ValidatorConfigBannedConstructsTest.java b/src/test/java/com/hubspot/jinjava/el/ext/ValidatorConfigBannedConstructsTest.java new file mode 100644 index 000000000..976271504 --- /dev/null +++ b/src/test/java/com/hubspot/jinjava/el/ext/ValidatorConfigBannedConstructsTest.java @@ -0,0 +1,197 @@ +package com.hubspot.jinjava.el.ext; + +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.hubspot.jinjava.interpret.JinjavaInterpreter; +import java.lang.reflect.Method; +import org.junit.Test; + +public class ValidatorConfigBannedConstructsTest { + + // MethodValidatorConfig: allowedMethods() path + + @Test + public void itRejectsObjectMethodInAllowedMethods() throws NoSuchMethodException { + Method toStringMethod = Object.class.getMethod("toString"); + assertThatThrownBy(() -> + MethodValidatorConfig.builder().addAllowedMethods(toStringMethod).build() + ) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Banned classes or prefixes"); + } + + @Test + public void itRejectsClassMethodInAllowedMethods() throws NoSuchMethodException { + Method getNameMethod = Class.class.getMethod("getName"); + assertThatThrownBy(() -> + MethodValidatorConfig.builder().addAllowedMethods(getNameMethod).build() + ) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Banned classes or prefixes"); + } + + // MethodValidatorConfig: allowedDeclaredMethodsFromCanonicalClassNames() path + + @Test + public void itRejectsObjectClassInAllowedDeclaredMethodClassNames() { + assertThatThrownBy(() -> + MethodValidatorConfig + .builder() + .addAllowedDeclaredMethodsFromCanonicalClassNames( + Object.class.getCanonicalName() + ) + .build() + ) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Banned classes or prefixes"); + } + + @Test + public void itRejectsClassClassInAllowedDeclaredMethodClassNames() { + assertThatThrownBy(() -> + MethodValidatorConfig + .builder() + .addAllowedDeclaredMethodsFromCanonicalClassNames( + Class.class.getCanonicalName() + ) + .build() + ) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Banned classes or prefixes"); + } + + @Test + public void itRejectsObjectMapperInAllowedDeclaredMethodClassNames() { + assertThatThrownBy(() -> + MethodValidatorConfig + .builder() + .addAllowedDeclaredMethodsFromCanonicalClassNames( + ObjectMapper.class.getCanonicalName() + ) + .build() + ) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Banned classes or prefixes"); + } + + @Test + public void itRejectsJinjavaInterpreterInAllowedDeclaredMethodClassNames() { + assertThatThrownBy(() -> + MethodValidatorConfig + .builder() + .addAllowedDeclaredMethodsFromCanonicalClassNames( + JinjavaInterpreter.class.getCanonicalName() + ) + .build() + ) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Banned classes or prefixes"); + } + + // MethodValidatorConfig: allowedDeclaredMethodsFromCanonicalClassPrefixes() path + + @Test + public void itRejectsReflectPackageInAllowedDeclaredMethodPrefixes() { + assertThatThrownBy(() -> + MethodValidatorConfig + .builder() + .addAllowedDeclaredMethodsFromCanonicalClassPrefixes( + Method.class.getPackageName() + ) + .build() + ) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Banned classes or prefixes"); + } + + @Test + public void itRejectsJacksonDatabindPackageInAllowedDeclaredMethodPrefixes() { + assertThatThrownBy(() -> + MethodValidatorConfig + .builder() + .addAllowedDeclaredMethodsFromCanonicalClassPrefixes( + ObjectMapper.class.getPackageName() + ) + .build() + ) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Banned classes or prefixes"); + } + + // ReturnTypeValidatorConfig: allowedCanonicalClassNames() path + + @Test + public void itRejectsObjectClassInAllowedReturnTypeClassNames() { + assertThatThrownBy(() -> + ReturnTypeValidatorConfig + .builder() + .addAllowedCanonicalClassNames(Object.class.getCanonicalName()) + .build() + ) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Banned classes or prefixes"); + } + + @Test + public void itRejectsClassClassInAllowedReturnTypeClassNames() { + assertThatThrownBy(() -> + ReturnTypeValidatorConfig + .builder() + .addAllowedCanonicalClassNames(Class.class.getCanonicalName()) + .build() + ) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Banned classes or prefixes"); + } + + @Test + public void itRejectsObjectMapperInAllowedReturnTypeClassNames() { + assertThatThrownBy(() -> + ReturnTypeValidatorConfig + .builder() + .addAllowedCanonicalClassNames(ObjectMapper.class.getCanonicalName()) + .build() + ) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Banned classes or prefixes"); + } + + @Test + public void itRejectsJinjavaInterpreterInAllowedReturnTypeClassNames() { + assertThatThrownBy(() -> + ReturnTypeValidatorConfig + .builder() + .addAllowedCanonicalClassNames(JinjavaInterpreter.class.getCanonicalName()) + .build() + ) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Banned classes or prefixes"); + } + + // ReturnTypeValidatorConfig: allowedCanonicalClassPrefixes() path + + @Test + public void itRejectsReflectPackageInAllowedReturnTypePrefixes() { + assertThatThrownBy(() -> + ReturnTypeValidatorConfig + .builder() + .addAllowedCanonicalClassPrefixes(Method.class.getPackageName()) + .build() + ) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Banned classes or prefixes"); + } + + @Test + public void itRejectsJacksonDatabindPackageInAllowedReturnTypePrefixes() { + assertThatThrownBy(() -> + ReturnTypeValidatorConfig + .builder() + .addAllowedCanonicalClassPrefixes(ObjectMapper.class.getPackageName()) + .build() + ) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Banned classes or prefixes"); + } +} diff --git a/src/test/java/com/hubspot/jinjava/el/ext/eager/EagerAstDotTest.java b/src/test/java/com/hubspot/jinjava/el/ext/eager/EagerAstDotTest.java index f86620f8e..6b13a7bc8 100644 --- a/src/test/java/com/hubspot/jinjava/el/ext/eager/EagerAstDotTest.java +++ b/src/test/java/com/hubspot/jinjava/el/ext/eager/EagerAstDotTest.java @@ -12,7 +12,7 @@ import com.hubspot.jinjava.interpret.JinjavaInterpreter; import com.hubspot.jinjava.mode.EagerExecutionMode; import com.hubspot.jinjava.random.RandomNumberGeneratorStrategy; -import com.hubspot.jinjava.testobjects.EagerAstDotTestObjects.Foo; +import com.hubspot.jinjava.testobjects.EagerAstDotTestObjects; import org.junit.Before; import org.junit.Test; @@ -28,6 +28,8 @@ public void setup() { .withLegacyOverrides( LegacyOverrides.newBuilder().withUsePyishObjectMapper(true).build() ) + .withMethodValidator(BaseJinjavaTest.METHOD_VALIDATOR) + .withReturnTypeValidator(BaseJinjavaTest.RETURN_TYPE_VALIDATOR) .withMaxMacroRecursionDepth(5) .withEnableRecursiveMacroCalls(true) .build(); @@ -43,25 +45,32 @@ public void setup() { @Test public void itDefersWhenDotThrowsDeferredValueException() { - interpreter.getContext().put("foo", new Foo()); - assertThat(interpreter.render("{{ foo.deferred }}")).isEqualTo("{{ foo.deferred }}"); + try (var a = JinjavaInterpreter.closeablePushCurrent(interpreter).get()) { + interpreter.getContext().put("foo", new EagerAstDotTestObjects.Foo()); + assertThat(interpreter.render("{{ foo.deferred }}")) + .isEqualTo("{{ foo.deferred }}"); + } } @Test public void itResolvedDeferredMapWithDot() { - interpreter.getContext().put("foo", new Foo()); + interpreter.getContext().put("foo", new EagerAstDotTestObjects.Foo()); assertThat(interpreter.render("{{ foo.resolved }}")).isEqualTo("resolved"); } @Test public void itResolvedNestedDeferredMapWithDot() { - interpreter.getContext().put("foo_map", ImmutableMap.of("bar", new Foo())); + interpreter + .getContext() + .put("foo_map", ImmutableMap.of("bar", new EagerAstDotTestObjects.Foo())); assertThat(interpreter.render("{{ foo_map.bar.resolved }}")).isEqualTo("resolved"); } @Test public void itDefersNodeWhenNestedDeferredMapDotThrowsDeferredValueException() { - interpreter.getContext().put("foo_map", ImmutableMap.of("bar", new Foo())); + interpreter + .getContext() + .put("foo_map", ImmutableMap.of("bar", new EagerAstDotTestObjects.Foo())); assertThat(interpreter.render("{{ foo_map.bar.deferred }}")) .isEqualTo("{{ foo_map.bar.deferred }}"); assertThat(interpreter.getContext().getDeferredNodes()).isNotEmpty(); diff --git a/src/test/java/com/hubspot/jinjava/el/ext/eager/EagerAstMethodTest.java b/src/test/java/com/hubspot/jinjava/el/ext/eager/EagerAstMethodTest.java index 30ff441b8..c60117f6c 100644 --- a/src/test/java/com/hubspot/jinjava/el/ext/eager/EagerAstMethodTest.java +++ b/src/test/java/com/hubspot/jinjava/el/ext/eager/EagerAstMethodTest.java @@ -36,6 +36,8 @@ public void setup() { .withLegacyOverrides( LegacyOverrides.newBuilder().withUsePyishObjectMapper(true).build() ) + .withMethodValidator(BaseJinjavaTest.METHOD_VALIDATOR) + .withReturnTypeValidator(BaseJinjavaTest.RETURN_TYPE_VALIDATOR) .withMaxMacroRecursionDepth(5) .withEnableRecursiveMacroCalls(true) .build(); @@ -233,7 +235,7 @@ public void itPreservesUnresolvable() { fail("Should throw DeferredParsingException"); } catch (DeferredParsingException e) { assertThat(e.getDeferredEvalResult()) - .isEqualTo("filter:upper.filter(foo_object.deferred, ____int3rpr3t3r____)"); + .isEqualTo("filter:upper.filter(foo_object.deferred, null)"); } } } diff --git a/src/test/java/com/hubspot/jinjava/interpret/JinjavaInterpreterTest.java b/src/test/java/com/hubspot/jinjava/interpret/JinjavaInterpreterTest.java index c2fca7cdc..1d099dcdf 100644 --- a/src/test/java/com/hubspot/jinjava/interpret/JinjavaInterpreterTest.java +++ b/src/test/java/com/hubspot/jinjava/interpret/JinjavaInterpreterTest.java @@ -109,94 +109,118 @@ public void resolveBlockStubsWithCycle() { @Test public void singleWordProperty() { - assertThat( - interpreter.resolveProperty(new JinjavaInterpreterTestObjects.Foo("a"), "bar") - ) - .isEqualTo("a"); + try (var a = JinjavaInterpreter.closeablePushCurrent(interpreter).get()) { + assertThat( + interpreter.resolveProperty(new JinjavaInterpreterTestObjects.Foo("a"), "bar") + ) + .isEqualTo("a"); + } } @Test public void multiWordCamelCase() { - assertThat( - interpreter.resolveProperty(new JinjavaInterpreterTestObjects.Foo("a"), "barFoo") - ) - .isEqualTo("a"); + try (var a = JinjavaInterpreter.closeablePushCurrent(interpreter).get()) { + assertThat( + interpreter.resolveProperty(new JinjavaInterpreterTestObjects.Foo("a"), "barFoo") + ) + .isEqualTo("a"); + } } @Test public void multiWordSnakeCase() { - assertThat( - interpreter.resolveProperty(new JinjavaInterpreterTestObjects.Foo("a"), "bar_foo") - ) - .isEqualTo("a"); + try (var a = JinjavaInterpreter.closeablePushCurrent(interpreter).get()) { + assertThat( + interpreter.resolveProperty(new JinjavaInterpreterTestObjects.Foo("a"), "bar_foo") + ) + .isEqualTo("a"); + } } @Test public void multiWordNumberSnakeCase() { - assertThat( - interpreter.resolveProperty(new JinjavaInterpreterTestObjects.Foo("a"), "bar_foo_1") - ) - .isEqualTo("a"); + try (var a = JinjavaInterpreter.closeablePushCurrent(interpreter).get()) { + assertThat( + interpreter.resolveProperty( + new JinjavaInterpreterTestObjects.Foo("a"), + "bar_foo_1" + ) + ) + .isEqualTo("a"); + } } @Test public void jsonIgnore() { - assertThat( - interpreter.resolveProperty(new JinjavaInterpreterTestObjects.Foo("a"), "barHidden") - ) - .isEqualTo("a"); + try (var a = JinjavaInterpreter.closeablePushCurrent(interpreter).get()) { + assertThat( + interpreter.resolveProperty( + new JinjavaInterpreterTestObjects.Foo("a"), + "barHidden" + ) + ) + .isEqualTo("a"); + } } @Test public void triesBeanMethodFirst() { - assertThat( - interpreter - .resolveProperty(ZonedDateTime.parse("2013-09-19T12:12:12+00:00"), "year") - .toString() - ) - .isEqualTo("2013"); + try (var a = JinjavaInterpreter.closeablePushCurrent(interpreter).get()) { + assertThat( + interpreter + .resolveProperty(ZonedDateTime.parse("2013-09-19T12:12:12+00:00"), "year") + .toString() + ) + .isEqualTo("2013"); + } } @Test public void enterScopeTryFinally() { - interpreter.getContext().put("foo", "parent"); - - interpreter.enterScope(); - try { - interpreter.getContext().put("foo", "child"); - assertThat(interpreter.resolveELExpression("foo", 1)).isEqualTo("child"); - } finally { - interpreter.leaveScope(); + try (var a = JinjavaInterpreter.closeablePushCurrent(interpreter).get()) { + interpreter.getContext().put("foo", "parent"); + + interpreter.enterScope(); + try { + interpreter.getContext().put("foo", "child"); + assertThat(interpreter.resolveELExpression("foo", 1)).isEqualTo("child"); + } finally { + interpreter.leaveScope(); + } + + assertThat(interpreter.resolveELExpression("foo", 1)).isEqualTo("parent"); } - - assertThat(interpreter.resolveELExpression("foo", 1)).isEqualTo("parent"); } @Test public void enterScopeTryWithResources() { - interpreter.getContext().put("foo", "parent"); + try (var a = JinjavaInterpreter.closeablePushCurrent(interpreter).get()) { + interpreter.getContext().put("foo", "parent"); - try (InterpreterScopeClosable c = interpreter.enterScope()) { - interpreter.getContext().put("foo", "child"); - assertThat(interpreter.resolveELExpression("foo", 1)).isEqualTo("child"); - } + try (InterpreterScopeClosable c = interpreter.enterScope()) { + interpreter.getContext().put("foo", "child"); + assertThat(interpreter.resolveELExpression("foo", 1)).isEqualTo("child"); + } - assertThat(interpreter.resolveELExpression("foo", 1)).isEqualTo("parent"); + assertThat(interpreter.resolveELExpression("foo", 1)).isEqualTo("parent"); + } } @Test public void bubbleUpDependenciesFromLowerScope() { - String dependencyType = "foo"; - String dependencyIdentifier = "123"; - - interpreter.enterScope(); - interpreter.getContext().addDependency(dependencyType, dependencyIdentifier); - assertThat(interpreter.getContext().getDependencies().get(dependencyType)) - .contains(dependencyIdentifier); - interpreter.leaveScope(); + try (var a = JinjavaInterpreter.closeablePushCurrent(interpreter).get()) { + String dependencyType = "foo"; + String dependencyIdentifier = "123"; + + interpreter.enterScope(); + interpreter.getContext().addDependency(dependencyType, dependencyIdentifier); + assertThat(interpreter.getContext().getDependencies().get(dependencyType)) + .contains(dependencyIdentifier); + interpreter.leaveScope(); - assertThat(interpreter.getContext().getDependencies().get(dependencyType)) - .contains(dependencyIdentifier); + assertThat(interpreter.getContext().getDependencies().get(dependencyType)) + .contains(dependencyIdentifier); + } } @Test @@ -614,7 +638,6 @@ public void itOutputsUndefinedVariableError() { @Test public void itDoesNotAllowAccessingPropertiesOfInterpreter() { - assertThat(jinjava.render("{{ ____int3rpr3t3r____.config }}", new HashMap<>())) - .isEqualTo(""); + assertThat(jinjava.render("{{ null.config }}", new HashMap<>())).isEqualTo(""); } } diff --git a/src/test/java/com/hubspot/jinjava/lib/filter/time/FormatDateFilterTest.java b/src/test/java/com/hubspot/jinjava/lib/filter/time/FormatDateFilterTest.java index f94b58484..65c0b321b 100644 --- a/src/test/java/com/hubspot/jinjava/lib/filter/time/FormatDateFilterTest.java +++ b/src/test/java/com/hubspot/jinjava/lib/filter/time/FormatDateFilterTest.java @@ -33,7 +33,7 @@ public class FormatDateFilterTest { @Before public void setUp() throws Exception { - jinjava = new Jinjava(); + jinjava = new Jinjava(BaseJinjavaTest.newConfigBuilder().build()); jinjava.getGlobalContext().registerClasses(FormatDateFilter.class); } diff --git a/src/test/java/com/hubspot/jinjava/lib/tag/IfTagTest.java b/src/test/java/com/hubspot/jinjava/lib/tag/IfTagTest.java index 31c589efa..62ae0e483 100644 --- a/src/test/java/com/hubspot/jinjava/lib/tag/IfTagTest.java +++ b/src/test/java/com/hubspot/jinjava/lib/tag/IfTagTest.java @@ -8,7 +8,6 @@ import com.hubspot.jinjava.testobjects.IfTagTestObjects; import com.hubspot.jinjava.tree.TagNode; import com.hubspot.jinjava.tree.TreeParser; -import com.hubspot.jinjava.util.ObjectTruthValueTest; import java.io.IOException; import java.nio.charset.StandardCharsets; import org.junit.Before; diff --git a/src/test/java/com/hubspot/jinjava/lib/tag/ImportTagTest.java b/src/test/java/com/hubspot/jinjava/lib/tag/ImportTagTest.java index 1e26bb0d6..afd9a7f18 100644 --- a/src/test/java/com/hubspot/jinjava/lib/tag/ImportTagTest.java +++ b/src/test/java/com/hubspot/jinjava/lib/tag/ImportTagTest.java @@ -13,7 +13,7 @@ import com.hubspot.jinjava.loader.LocationResolver; import com.hubspot.jinjava.loader.RelativePathResolver; import com.hubspot.jinjava.loader.ResourceLocator; -import com.hubspot.jinjava.testobjects.EagerImportTagTestObjects; +import com.hubspot.jinjava.testobjects.EagerImportTagTestObjects.PrintPathFilter; import com.hubspot.jinjava.tree.Node; import java.io.IOException; import java.nio.charset.Charset; @@ -31,7 +31,7 @@ public class ImportTagTest extends BaseInterpretingTest { @Before public void setup() { context.put("padding", 42); - context.registerFilter(new EagerImportTagTestObjects.PrintPathFilter()); + context.registerFilter(new PrintPathFilter()); } @Test diff --git a/src/test/java/com/hubspot/jinjava/lib/tag/ValidationModeTest.java b/src/test/java/com/hubspot/jinjava/lib/tag/ValidationModeTest.java index 3a5e0aad8..3e0b1438f 100644 --- a/src/test/java/com/hubspot/jinjava/lib/tag/ValidationModeTest.java +++ b/src/test/java/com/hubspot/jinjava/lib/tag/ValidationModeTest.java @@ -11,6 +11,7 @@ import com.hubspot.jinjava.lib.fn.ELFunctionDefinition; import com.hubspot.jinjava.lib.fn.MacroFunction; import com.hubspot.jinjava.testobjects.ValidationModeTestObjects; +import com.hubspot.jinjava.testobjects.ValidationModeTestObjects; import com.hubspot.jinjava.tree.Node; import com.hubspot.jinjava.tree.TextNode; import com.hubspot.jinjava.tree.parse.DefaultTokenScannerSymbols; @@ -51,7 +52,7 @@ public void setup() { "validationTestFunction" ); - jinjava = new Jinjava(); + jinjava = new Jinjava(BaseJinjavaTest.newConfigBuilder().build()); jinjava.getGlobalContext().registerFilter(validationFilter); jinjava.getGlobalContext().registerFunction(validationFunction); interpreter = jinjava.newInterpreter(); diff --git a/src/test/java/com/hubspot/jinjava/lib/tag/eager/EagerImportTagTest.java b/src/test/java/com/hubspot/jinjava/lib/tag/eager/EagerImportTagTest.java index 58de257ed..ee05c7748 100644 --- a/src/test/java/com/hubspot/jinjava/lib/tag/eager/EagerImportTagTest.java +++ b/src/test/java/com/hubspot/jinjava/lib/tag/eager/EagerImportTagTest.java @@ -423,7 +423,7 @@ public void itCorrectlySetsAliasedPathForSecondPass() { assertThat(firstPassResult) .isEqualTo( "{% set deferred_import_resource_path = 'import-macro.jinja' %}{% macro m.print_path_macro(var) %}\n" + - "{{ filter:print_path.filter(var, ____int3rpr3t3r____) }}\n" + + "{{ filter:print_path.filter(var, null) }}\n" + "{{ var }}\n" + "{% endmacro %}{% set deferred_import_resource_path = null %}{{ m.print_path_macro(foo) }}" ); @@ -442,7 +442,7 @@ public void itCorrectlySetsPathForSecondPass() { assertThat(firstPassResult) .isEqualTo( "{% set deferred_import_resource_path = 'import-macro.jinja' %}{% macro print_path_macro(var) %}\n" + - "{{ filter:print_path.filter(var, ____int3rpr3t3r____) }}\n" + + "{{ filter:print_path.filter(var, null) }}\n" + "{{ var }}\n" + "{% endmacro %}{% set deferred_import_resource_path = null %}{{ print_path_macro(foo) }}" ); @@ -460,9 +460,9 @@ public void itCorrectlySetsNestedPathsForSecondPass() { ); assertThat(firstPassResult) .isEqualTo( - "{% set deferred_import_resource_path = 'double-import-macro.jinja' %}{% macro print_path_macro2(var) %}{{ filter:print_path.filter(var, ____int3rpr3t3r____) }}\n" + + "{% set deferred_import_resource_path = 'double-import-macro.jinja' %}{% macro print_path_macro2(var) %}{{ filter:print_path.filter(var, null) }}\n" + "{% set deferred_import_resource_path = 'import-macro.jinja' %}{% macro print_path_macro(var) %}\n" + - "{{ filter:print_path.filter(var, ____int3rpr3t3r____) }}\n" + + "{{ filter:print_path.filter(var, null) }}\n" + "{{ var }}\n" + "{% endmacro %}{% set deferred_import_resource_path = 'double-import-macro.jinja' %}{{ print_path_macro(var) }}{% endmacro %}{% set deferred_import_resource_path = null %}{{ print_path_macro2(foo) }}" ); diff --git a/src/test/java/com/hubspot/jinjava/lib/tag/eager/EagerSetTagTest.java b/src/test/java/com/hubspot/jinjava/lib/tag/eager/EagerSetTagTest.java index 73f484d4d..f80628e69 100644 --- a/src/test/java/com/hubspot/jinjava/lib/tag/eager/EagerSetTagTest.java +++ b/src/test/java/com/hubspot/jinjava/lib/tag/eager/EagerSetTagTest.java @@ -142,9 +142,7 @@ public void itDefersBlockWithFilter() { final String result = interpreter.render(template); assertThat(result) - .isEqualTo( - "{% set foo = filter:add.filter(2, ____int3rpr3t3r____, deferred) %}{{ foo }}" - ); + .isEqualTo("{% set foo = filter:add.filter(2, null, deferred) %}{{ foo }}"); assertThat( context .getDeferredTokens() @@ -179,7 +177,7 @@ public void itDefersDeferredBlockWithDeferredFilter() { assertThat(result) .isEqualTo( - "{% set foo %}{{ 1 + deferred }}{% endset %}{% set foo = filter:add.filter(foo, ____int3rpr3t3r____, filter:int.filter(deferred, ____int3rpr3t3r____)) %}{{ foo }}" + "{% set foo %}{{ 1 + deferred }}{% endset %}{% set foo = filter:add.filter(foo, null, filter:int.filter(deferred, null)) %}{{ foo }}" ); context.remove("foo"); context.put("deferred", 2); @@ -203,7 +201,7 @@ public void itDefersInDeferredExecutionModeWithFilter() { assertThat(result) .isEqualTo( - "{% set foo %}1{% endset %}{% set foo = filter:add.filter(1, ____int3rpr3t3r____, deferred) %}{{ foo }}" + "{% set foo %}1{% endset %}{% set foo = filter:add.filter(1, null, deferred) %}{{ foo }}" ); assertThat( context diff --git a/src/test/java/com/hubspot/jinjava/objects/collections/PyMapTest.java b/src/test/java/com/hubspot/jinjava/objects/collections/PyMapTest.java index 37a170166..0d0a897af 100644 --- a/src/test/java/com/hubspot/jinjava/objects/collections/PyMapTest.java +++ b/src/test/java/com/hubspot/jinjava/objects/collections/PyMapTest.java @@ -6,7 +6,6 @@ import com.google.common.collect.ImmutableList; import com.hubspot.jinjava.BaseJinjavaTest; import com.hubspot.jinjava.Jinjava; -import com.hubspot.jinjava.JinjavaConfig; import com.hubspot.jinjava.LegacyOverrides; import com.hubspot.jinjava.interpret.IndexOutOfRangeException; import com.hubspot.jinjava.interpret.RenderResult; @@ -274,7 +273,16 @@ public void itUpdatesKeysWithStaticName() { } @Test - public void itDoesntSetKeysWithVariableNameByDefault() { + public void itDoesntSetKeysWithVariableNameWhenLegacy() { + jinjava = + new Jinjava( + BaseJinjavaTest + .newConfigBuilder() + .withLegacyOverrides( + LegacyOverrides.newBuilder().withEvaluateMapKeys(false).build() + ) + .build() + ); assertThat( jinjava.render( "{% set keyName = \"key1\" %}" + @@ -346,7 +354,14 @@ public void itFallsBackUnknownVariableNameToString() { } @Test - public void itDoesntUpdateKeysWithVariableNameByDefault() { + public void itDoesntUpdateKeysWithVariableNameWhenLegacy() { + jinjava = + new Jinjava( + BaseJinjavaTest + .newConfigBuilder() + .withLegacyOverrides(LegacyOverrides.THREE_POINT_0.withEvaluateMapKeys(false)) + .build() + ); assertThat( jinjava.render( "{% set test = {\"key1\": \"value1\"} %}" + diff --git a/src/test/java/com/hubspot/jinjava/tree/TreeParserTest.java b/src/test/java/com/hubspot/jinjava/tree/TreeParserTest.java index e5a8374c5..80fc04a27 100644 --- a/src/test/java/com/hubspot/jinjava/tree/TreeParserTest.java +++ b/src/test/java/com/hubspot/jinjava/tree/TreeParserTest.java @@ -251,22 +251,19 @@ public void itWarnsTwiceAgainstUnclosedBlockTag() { public void itTrimsNotes() { String expression = "A\n{#- note -#}\nB"; final Node tree = new TreeParser(interpreter, expression).buildTree(); - assertThat(interpreter.render(tree)).isEqualTo("A\n\nB"); + assertThat(interpreter.render(tree)).isEqualTo("AB"); interpreter = new Jinjava( BaseJinjavaTest .newConfigBuilder() .withLegacyOverrides( - LegacyOverrides - .newBuilder() - .withUseTrimmingForNotesAndExpressions(true) - .build() + LegacyOverrides.THREE_POINT_0.withUseTrimmingForNotesAndExpressions(false) ) .build() ) .newInterpreter(); final Node newTree = new TreeParser(interpreter, expression).buildTree(); - assertThat(interpreter.render(newTree)).isEqualTo("AB"); + assertThat(interpreter.render(newTree)).isEqualTo("A\n\nB"); } @Test @@ -346,41 +343,38 @@ public void itMergesTextNodesWhileRespectingTrim() { public void itTrimsExpressions() { String expression = "A\n{{- 'B' -}}\nC"; final Node tree = new TreeParser(interpreter, expression).buildTree(); - assertThat(interpreter.render(tree)).isEqualTo("A\nB\nC"); + assertThat(interpreter.render(tree)).isEqualTo("ABC"); interpreter = new Jinjava( BaseJinjavaTest .newConfigBuilder() .withLegacyOverrides( - LegacyOverrides - .newBuilder() - .withUseTrimmingForNotesAndExpressions(true) - .build() + LegacyOverrides.THREE_POINT_0.withUseTrimmingForNotesAndExpressions(false) ) .build() ) .newInterpreter(); final Node newTree = new TreeParser(interpreter, expression).buildTree(); - assertThat(interpreter.render(newTree)).isEqualTo("ABC"); + assertThat(interpreter.render(newTree)).isEqualTo("A\nB\nC"); } @Test public void itDoesNotMergeAdjacentTextNodesWhenLegacyOverrideIsApplied() { String expression = "A\n{%- if true -%}\n{# comment #}\nB{% endif %}"; final Node tree = new TreeParser(interpreter, expression).buildTree(); - assertThat(interpreter.render(tree)).isEqualTo("AB"); + assertThat(interpreter.render(tree)).isEqualTo("A\nB"); interpreter = new Jinjava( BaseJinjavaTest .newConfigBuilder() .withLegacyOverrides( - LegacyOverrides.newBuilder().withAllowAdjacentTextNodes(true).build() + LegacyOverrides.THREE_POINT_0.withAllowAdjacentTextNodes(false) ) .build() ) .newInterpreter(); final Node overriddenTree = new TreeParser(interpreter, expression).buildTree(); - assertThat(interpreter.render(overriddenTree)).isEqualTo("A\nB"); + assertThat(interpreter.render(overriddenTree)).isEqualTo("AB"); } @Test diff --git a/src/test/java/com/hubspot/jinjava/util/EagerExpressionResolverTest.java b/src/test/java/com/hubspot/jinjava/util/EagerExpressionResolverTest.java index 471237f33..d3458e3bb 100644 --- a/src/test/java/com/hubspot/jinjava/util/EagerExpressionResolverTest.java +++ b/src/test/java/com/hubspot/jinjava/util/EagerExpressionResolverTest.java @@ -273,6 +273,7 @@ public void itSplitsAndIndexesOnNonWords() { @Test public void itSupportsOrderOfOperations() { + eagerResolveExpression("['a','b'][1]"); EagerExpressionResult eagerExpressionResult = eagerResolveExpression( "[0,1]|reverse + deferred" ); @@ -640,7 +641,7 @@ public void itHandlesDeferredExpTests() { interpreter.getContext().setThrowInterpreterErrors(true); String partiallyResolved = eagerExpressionResult.toString(); assertThat(partiallyResolved) - .isEqualTo("exptest:equalto.evaluateNegated(4, ____int3rpr3t3r____, deferred)"); + .isEqualTo("exptest:equalto.evaluateNegated(4, null, deferred)"); assertThat(eagerExpressionResult.getDeferredWords()) .containsExactlyInAnyOrder("deferred", "equalto.evaluateNegated"); context.put("deferred", 4); @@ -755,7 +756,7 @@ public void itHandlesDeferredMethod() { assertThat(eagerResolveExpression("deferred.append(foo)").toString()) .isEqualTo("deferred.append('foo')"); assertThat(eagerResolveExpression("deferred[1 + 1] | length").toString()) - .isEqualTo("filter:length.filter(deferred[2], ____int3rpr3t3r____)"); + .isEqualTo("filter:length.filter(deferred[2], null)"); } @Test @@ -822,7 +823,7 @@ public void itDoesNotSplitJsonInArrayResolvedExpression() { @Test public void itHandlesRandom() { assertThat(eagerResolveExpression("range(1)|random").toString()) - .isEqualTo("filter:random.filter(range(1), ____int3rpr3t3r____)"); + .isEqualTo("filter:random.filter(range(1), null)"); } @Test diff --git a/src/test/resources/eager/defers-macro-in-expression/test.expected.jinja b/src/test/resources/eager/defers-macro-in-expression/test.expected.jinja index 7443a94eb..18ad7e65b 100644 --- a/src/test/resources/eager/defers-macro-in-expression/test.expected.jinja +++ b/src/test/resources/eager/defers-macro-in-expression/test.expected.jinja @@ -1,10 +1,10 @@ 2 {% macro plus(foo, add) %}\ -{{ foo + (filter:int.filter(add, ____int3rpr3t3r____)) }}\ +{{ foo + (filter:int.filter(add, null)) }}\ {% endmacro %}\ {{ plus(deferred, 1.1) }}\ {% set deferred = deferred + 2 %} {% macro plus(foo, add) %}\ -{{ foo + (filter:int.filter(add, ____int3rpr3t3r____)) }}\ +{{ foo + (filter:int.filter(add, null)) }}\ {% endmacro %}\ {{ plus(deferred, 3.1) }} \ No newline at end of file diff --git a/src/test/resources/eager/defers-macro-in-for/test.expected.jinja b/src/test/resources/eager/defers-macro-in-for/test.expected.jinja index 3311b8714..e9540a513 100644 --- a/src/test/resources/eager/defers-macro-in-for/test.expected.jinja +++ b/src/test/resources/eager/defers-macro-in-for/test.expected.jinja @@ -3,6 +3,6 @@ {% do my_list.append(num) %}\ {{ my_list }}\ {% endmacro %}\ -{% for item in filter:split.filter(macro_append(deferred), ____int3rpr3t3r____, ',', 2) %} +{% for item in filter:split.filter(macro_append(deferred), null, ',', 2) %} {{ item }} {% endfor %} diff --git a/src/test/resources/eager/defers-macro-in-if/test.expected.jinja b/src/test/resources/eager/defers-macro-in-if/test.expected.jinja index 67f28d9e4..9837ae87b 100644 --- a/src/test/resources/eager/defers-macro-in-if/test.expected.jinja +++ b/src/test/resources/eager/defers-macro-in-if/test.expected.jinja @@ -3,6 +3,6 @@ {% do my_list.append(num) %}\ {{ my_list }}\ {% endmacro %}\ -{% if [] == filter:split.filter(macro_append(deferred), ____int3rpr3t3r____, ',', 2) %} +{% if [] == filter:split.filter(macro_append(deferred), null, ',', 2) %} {{ my_list }} {% endif %} diff --git a/src/test/resources/eager/does-not-override-import-modification-in-for/test.expected.jinja b/src/test/resources/eager/does-not-override-import-modification-in-for/test.expected.jinja index 0e1e4a8a2..e37dfc69f 100644 --- a/src/test/resources/eager/does-not-override-import-modification-in-for/test.expected.jinja +++ b/src/test/resources/eager/does-not-override-import-modification-in-for/test.expected.jinja @@ -11,7 +11,7 @@ {% endif %} -{% set foo = filter:join.filter([foo, 'b'], ____int3rpr3t3r____, '') %}\ +{% set foo = filter:join.filter([foo, 'b'], null, '') %}\ {% do __temp_meta_import_alias_3016318__.update({'foo': foo}) %} {% do __temp_meta_import_alias_3016318__.update({'foo': foo,'import_resource_path': 'eager/supplements/deferred-modification.jinja'}) %}\ {% endfor %}\ @@ -25,12 +25,12 @@ {% for __ignored__ in [0] %}\ {% if deferred %} -{% set foo = filter:join.filter([foo, 'a'], ____int3rpr3t3r____, '') %}\ +{% set foo = filter:join.filter([foo, 'a'], null, '') %}\ {% do __temp_meta_import_alias_3016319__.update({'foo': foo}) %} {% endif %} -{% set foo = filter:join.filter([foo, 'b'], ____int3rpr3t3r____, '') %}\ +{% set foo = filter:join.filter([foo, 'b'], null, '') %}\ {% do __temp_meta_import_alias_3016319__.update({'foo': foo}) %} {% do __temp_meta_import_alias_3016319__.update({'import_resource_path': 'eager/supplements/deferred-modification.jinja'}) %}\ {% endfor %}\ @@ -45,12 +45,12 @@ {% for __ignored__ in [0] %}\ {% if deferred %} -{% set foo = filter:join.filter([foo, 'a'], ____int3rpr3t3r____, '') %}\ +{% set foo = filter:join.filter([foo, 'a'], null, '') %}\ {% do __temp_meta_import_alias_3016318__.update({'foo': foo}) %} {% endif %} -{% set foo = filter:join.filter([foo, 'b'], ____int3rpr3t3r____, '') %}\ +{% set foo = filter:join.filter([foo, 'b'], null, '') %}\ {% do __temp_meta_import_alias_3016318__.update({'foo': foo}) %} {% do __temp_meta_import_alias_3016318__.update({'import_resource_path': 'eager/supplements/deferred-modification.jinja'}) %}\ {% endfor %}\ @@ -64,12 +64,12 @@ {% for __ignored__ in [0] %}\ {% if deferred %} -{% set foo = filter:join.filter([foo, 'a'], ____int3rpr3t3r____, '') %}\ +{% set foo = filter:join.filter([foo, 'a'], null, '') %}\ {% do __temp_meta_import_alias_3016319__.update({'foo': foo}) %} {% endif %} -{% set foo = filter:join.filter([foo, 'b'], ____int3rpr3t3r____, '') %}\ +{% set foo = filter:join.filter([foo, 'b'], null, '') %}\ {% do __temp_meta_import_alias_3016319__.update({'foo': foo}) %} {% do __temp_meta_import_alias_3016319__.update({'import_resource_path': 'eager/supplements/deferred-modification.jinja'}) %}\ {% endfor %}\ diff --git a/src/test/resources/eager/fully-defers-filtered-macro/test.expected.jinja b/src/test/resources/eager/fully-defers-filtered-macro/test.expected.jinja index 4938403bd..76727bbcc 100644 --- a/src/test/resources/eager/fully-defers-filtered-macro/test.expected.jinja +++ b/src/test/resources/eager/fully-defers-filtered-macro/test.expected.jinja @@ -1,5 +1,5 @@ {% macro flashy(foo) %}\ -{{ filter:upper.filter(foo, ____int3rpr3t3r____) }} +{{ filter:upper.filter(foo, null) }} A flashy {{ deferred }}\ .{% endmacro %}\ {% set __macro_flashy_1625622909_temp_variable_0__ %}\ @@ -12,4 +12,4 @@ BAR {% set __macro_silly_2092874071_temp_variable_0__ %}\ A silly {{ deferred }}\ .{% endset %}\ -{{ filter:upper.filter(__macro_silly_2092874071_temp_variable_0__, ____int3rpr3t3r____) }} \ No newline at end of file +{{ filter:upper.filter(__macro_silly_2092874071_temp_variable_0__, null) }} \ No newline at end of file diff --git a/src/test/resources/eager/handles-deferred-cycle-as/test.expected.jinja b/src/test/resources/eager/handles-deferred-cycle-as/test.expected.jinja index 5893e2ee2..48b005b8c 100644 --- a/src/test/resources/eager/handles-deferred-cycle-as/test.expected.jinja +++ b/src/test/resources/eager/handles-deferred-cycle-as/test.expected.jinja @@ -1,19 +1,19 @@ {% for __ignored__ in [0] %} {% set c = [1, deferred] %} -{% if exptest:iterable.evaluate(c, ____int3rpr3t3r____) %}\ -{{ c[0 % filter:length.filter(c, ____int3rpr3t3r____)] }}\ +{% if exptest:iterable.evaluate(c, null) %}\ +{{ c[0 % filter:length.filter(c, null)] }}\ {% else %}\ {{ c }}\ {% endif %} {% set c = [2, deferred] %} -{% if exptest:iterable.evaluate(c, ____int3rpr3t3r____) %}\ -{{ c[1 % filter:length.filter(c, ____int3rpr3t3r____)] }}\ +{% if exptest:iterable.evaluate(c, null) %}\ +{{ c[1 % filter:length.filter(c, null)] }}\ {% else %}\ {{ c }}\ {% endif %} {% set c = [3, deferred] %} -{% if exptest:iterable.evaluate(c, ____int3rpr3t3r____) %}\ -{{ c[2 % filter:length.filter(c, ____int3rpr3t3r____)] }}\ +{% if exptest:iterable.evaluate(c, null) %}\ +{{ c[2 % filter:length.filter(c, null)] }}\ {% else %}\ {{ c }}\ {% endif %}\ diff --git a/src/test/resources/eager/handles-deferred-for-loop-var-from-macro/test.expected.jinja b/src/test/resources/eager/handles-deferred-for-loop-var-from-macro/test.expected.jinja index 54d9fc69a..d7b09f4d6 100644 --- a/src/test/resources/eager/handles-deferred-for-loop-var-from-macro/test.expected.jinja +++ b/src/test/resources/eager/handles-deferred-for-loop-var-from-macro/test.expected.jinja @@ -5,12 +5,12 @@ {% set __macro_doIt_1327224118_temp_variable_0__ %} {{ deferred ~ '{\"a\":\"a\"}' }} {% endset %}\ -{{ filter:upper.filter(__macro_doIt_1327224118_temp_variable_0__, ____int3rpr3t3r____) }} +{{ filter:upper.filter(__macro_doIt_1327224118_temp_variable_0__, null) }} {% set __macro_doIt_1327224118_temp_variable_1__ %} {{ deferred ~ '{\"b\":\"b\"}' }} {% endset %}\ -{{ filter:upper.filter(__macro_doIt_1327224118_temp_variable_1__, ____int3rpr3t3r____) }} +{{ filter:upper.filter(__macro_doIt_1327224118_temp_variable_1__, null) }} {% endfor %} {% endset %}\ -{{ filter:upper.filter(__macro_getData_357124436_temp_variable_0__, ____int3rpr3t3r____) }} +{{ filter:upper.filter(__macro_getData_357124436_temp_variable_0__, null) }} diff --git a/src/test/resources/eager/handles-deferred-value-in-render-filter/test.expected.jinja b/src/test/resources/eager/handles-deferred-value-in-render-filter/test.expected.jinja index 21a51dcc1..5070d0d93 100644 --- a/src/test/resources/eager/handles-deferred-value-in-render-filter/test.expected.jinja +++ b/src/test/resources/eager/handles-deferred-value-in-render-filter/test.expected.jinja @@ -1,9 +1,9 @@ -{% set __render_524436216_temp_variable__ %}\ -Hi {{ filter:escape.filter(deferred, ____int3rpr3t3r____) }}\ +{% set __render_572171194_temp_variable__ %}\ +Hi {{ filter:escape.filter(deferred, null) }}\ {% endset %}\ -{{ filter:escape_jinjava.filter(__render_524436216_temp_variable__, ____int3rpr3t3r____) }} +{{ filter:escape_jinjava.filter(__render_572171194_temp_variable__, null) }} -{% set __render_524436216_temp_variable__ %}\ -Hi {{ filter:escape.filter(deferred, ____int3rpr3t3r____) }}\ +{% set __render_572171194_temp_variable__ %}\ +Hi {{ filter:escape.filter(deferred, null) }}\ {% endset %}\ -{{ __render_524436216_temp_variable__ }} \ No newline at end of file +{{ __render_572171194_temp_variable__ }} \ No newline at end of file diff --git a/src/test/resources/eager/handles-double-import-modification/test.expected.jinja b/src/test/resources/eager/handles-double-import-modification/test.expected.jinja index 66acdde36..7790ece96 100644 --- a/src/test/resources/eager/handles-double-import-modification/test.expected.jinja +++ b/src/test/resources/eager/handles-double-import-modification/test.expected.jinja @@ -9,7 +9,7 @@ {% endif %} -{% set foo = filter:join.filter([foo, 'b'], ____int3rpr3t3r____, '') %}\ +{% set foo = filter:join.filter([foo, 'b'], null, '') %}\ {% do __temp_meta_import_alias_3016318__.update({'foo': foo}) %} {% do __temp_meta_import_alias_3016318__.update({'foo': foo,'import_resource_path': 'eager/supplements/deferred-modification.jinja'}) %}\ {% endfor %}\ @@ -28,7 +28,7 @@ {% endif %} -{% set foo = filter:join.filter([foo, 'b'], ____int3rpr3t3r____, '') %}\ +{% set foo = filter:join.filter([foo, 'b'], null, '') %}\ {% do __temp_meta_import_alias_3016319__.update({'foo': foo}) %} {% do __temp_meta_import_alias_3016319__.update({'foo': foo,'import_resource_path': 'eager/supplements/deferred-modification.jinja'}) %}\ {% endfor %}\ diff --git a/src/test/resources/eager/handles-same-name-import-var/test.expected.jinja b/src/test/resources/eager/handles-same-name-import-var/test.expected.jinja index 66cfa99f0..7059c551c 100644 --- a/src/test/resources/eager/handles-same-name-import-var/test.expected.jinja +++ b/src/test/resources/eager/handles-same-name-import-var/test.expected.jinja @@ -35,5 +35,5 @@ {% set my_var = __temp_meta_import_alias_1059697132__ %}\ {% set current_path,__temp_meta_current_path_944750549__ = __temp_meta_current_path_944750549__,null %}\ {% enddo %} -{{ filter:dictsort.filter(my_var, ____int3rpr3t3r____, false, 'key') }} +{{ filter:dictsort.filter(my_var, null, false, 'key') }} {% endif %} diff --git a/src/test/resources/eager/reconstructs-block-set-variables-in-for-loop/test.expected.jinja b/src/test/resources/eager/reconstructs-block-set-variables-in-for-loop/test.expected.jinja index d0152a7f9..4340b7b25 100644 --- a/src/test/resources/eager/reconstructs-block-set-variables-in-for-loop/test.expected.jinja +++ b/src/test/resources/eager/reconstructs-block-set-variables-in-for-loop/test.expected.jinja @@ -2,5 +2,5 @@ {% set __macro_foo_97643642_temp_variable_0__ %} {{ deferred }} {% endset %}\ -{{ filter:int.filter(__macro_foo_97643642_temp_variable_0__, ____int3rpr3t3r____) + 3 }} +{{ filter:int.filter(__macro_foo_97643642_temp_variable_0__, null) + 3 }} {% endfor %} diff --git a/src/test/resources/eager/reconstructs-fromed-macro/test.expected.jinja b/src/test/resources/eager/reconstructs-fromed-macro/test.expected.jinja index c7213b008..8e02146cf 100644 --- a/src/test/resources/eager/reconstructs-fromed-macro/test.expected.jinja +++ b/src/test/resources/eager/reconstructs-fromed-macro/test.expected.jinja @@ -1,6 +1,6 @@ {% set deferred_import_resource_path = 'eager/reconstructs-fromed-macro/has-macro.jinja' %}\ {% macro to_upper(param) %} - {{ filter:upper.filter(param, ____int3rpr3t3r____) }} + {{ filter:upper.filter(param, null) }} {% endmacro %}\ {% set deferred_import_resource_path = null %}\ {{ to_upper(deferred) }} \ No newline at end of file diff --git a/src/test/resources/eager/uses-unique-macro-names/test.expected.jinja b/src/test/resources/eager/uses-unique-macro-names/test.expected.jinja index 54c40b809..e3585f49e 100644 --- a/src/test/resources/eager/uses-unique-macro-names/test.expected.jinja +++ b/src/test/resources/eager/uses-unique-macro-names/test.expected.jinja @@ -3,13 +3,13 @@ {% set __macro_foo_97643642_temp_variable_0__ %} Goodbye {{ myname }} {% endset %}\ -{% set a = filter:upper.filter(__macro_foo_97643642_temp_variable_0__, ____int3rpr3t3r____) %} +{% set a = filter:upper.filter(__macro_foo_97643642_temp_variable_0__, null) %} {% do %}\ {% set __temp_meta_current_path_203114534__,current_path = current_path,'eager/uses-unique-macro-names/macro-with-filter.jinja' %} {% set __macro_foo_1717337666_temp_variable_0__ %}\ Hello {{ myname }}\ {% endset %}\ -{% set b = filter:upper.filter(__macro_foo_1717337666_temp_variable_0__, ____int3rpr3t3r____) %} +{% set b = filter:upper.filter(__macro_foo_1717337666_temp_variable_0__, null) %} {% set current_path,__temp_meta_current_path_203114534__ = __temp_meta_current_path_203114534__,null %}\ {% enddo %} {{ a }} From f72754aa330e4c5b286e62251a1623e7cd36f6c5 Mon Sep 17 00:00:00 2001 From: jasmith-hs Date: Thu, 5 Mar 2026 11:06:31 -0500 Subject: [PATCH 2/8] Address Sidekick review feedback - Remove stale commented-out line in ExpressionResolver - Remove @Value.Check from static utility method in BannedAllowlistOptions - Make BeanMethod.method volatile for safe publication across threads - Guard against getBeanMethods returning null for unknown method names - Guard against getCanonicalName returning null for anonymous/local classes in AllowlistReturnTypeValidator and AllowlistMethodValidator Co-Authored-By: Claude Sonnet 4.6 --- .../java/com/hubspot/jinjava/el/ExpressionResolver.java | 1 - .../hubspot/jinjava/el/ext/AllowlistMethodValidator.java | 3 +++ .../jinjava/el/ext/AllowlistReturnTypeValidator.java | 4 ++++ .../hubspot/jinjava/el/ext/BannedAllowlistOptions.java | 3 --- .../com/hubspot/jinjava/el/ext/JinjavaBeanELResolver.java | 8 ++++++-- 5 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/hubspot/jinjava/el/ExpressionResolver.java b/src/main/java/com/hubspot/jinjava/el/ExpressionResolver.java index 60befb6e4..aaab4a4c6 100644 --- a/src/main/java/com/hubspot/jinjava/el/ExpressionResolver.java +++ b/src/main/java/com/hubspot/jinjava/el/ExpressionResolver.java @@ -352,7 +352,6 @@ public Object wrap(Object object) { public String getAsString(Object object) { if (interpreter.getConfig().getLegacyOverrides().isUsePyishObjectMapper()) { - // resolver. return PyishObjectMapper.getAsUnquotedPyishString(object); } return Objects.toString(object, ""); diff --git a/src/main/java/com/hubspot/jinjava/el/ext/AllowlistMethodValidator.java b/src/main/java/com/hubspot/jinjava/el/ext/AllowlistMethodValidator.java index a29fac7d4..7e19b70c1 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/AllowlistMethodValidator.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/AllowlistMethodValidator.java @@ -51,6 +51,9 @@ public Method validateMethod(Method m) { m1 -> { Class clazz = m1.getDeclaringClass(); String canonicalClassName = clazz.getCanonicalName(); + if (canonicalClassName == null) { + return false; + } return ( allowedMethods.contains(m1) || allowedDeclaredMethodsFromCanonicalClassNames.contains(canonicalClassName) || diff --git a/src/main/java/com/hubspot/jinjava/el/ext/AllowlistReturnTypeValidator.java b/src/main/java/com/hubspot/jinjava/el/ext/AllowlistReturnTypeValidator.java index 739cdd9ef..a8b84d714 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/AllowlistReturnTypeValidator.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/AllowlistReturnTypeValidator.java @@ -52,6 +52,10 @@ public Object validateReturnType(Object o) { return o; } String canonicalClassName = clazz.getCanonicalName(); + if (canonicalClassName == null) { + onRejectedClass.accept(clazz); + return null; + } boolean isAllowedClassName = allowedReturnTypesCache.computeIfAbsent( canonicalClassName, c -> diff --git a/src/main/java/com/hubspot/jinjava/el/ext/BannedAllowlistOptions.java b/src/main/java/com/hubspot/jinjava/el/ext/BannedAllowlistOptions.java index 9f8eee070..ed3f1246b 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/BannedAllowlistOptions.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/BannedAllowlistOptions.java @@ -7,8 +7,6 @@ import java.util.List; import java.util.Set; import java.util.stream.Stream; -import org.immutables.value.Value; - public class BannedAllowlistOptions { // These aren't required, but they prevent someone from misconfiguring Jinjava to allow sandbox bypass unintentionally @@ -42,7 +40,6 @@ public class BannedAllowlistOptions { ) .collect(ImmutableSet.toImmutableSet()); - @Value.Check public static List findBannedPrefixes(Stream prefixes) { return prefixes .filter(prefixOrName -> diff --git a/src/main/java/com/hubspot/jinjava/el/ext/JinjavaBeanELResolver.java b/src/main/java/com/hubspot/jinjava/el/ext/JinjavaBeanELResolver.java index c9ec31f2d..4d53d5cba 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/JinjavaBeanELResolver.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/JinjavaBeanELResolver.java @@ -84,7 +84,7 @@ protected static final class BeanMethod { private final MethodDescriptor descriptor; - private Method method; + private volatile Method method; public BeanMethod(MethodDescriptor descriptor) { this.descriptor = descriptor; @@ -200,7 +200,11 @@ protected Method findMethod( List potentialMethods = new LinkedList<>(); - for (BeanMethods.BeanMethod bm : beanMethods.getBeanMethods(name)) { + List methodsForName = beanMethods.getBeanMethods(name); + if (methodsForName == null) { + methodsForName = List.of(); + } + for (BeanMethods.BeanMethod bm : methodsForName) { Method m = bm.getMethod(); int formalParamCount = m.getParameterTypes().length; if (m.isVarArgs() && paramCount >= formalParamCount - 1) { From 2d8eb1c211162d93112e2ac98c5abf2fb10849b6 Mon Sep 17 00:00:00 2001 From: jasmith-hs Date: Thu, 5 Mar 2026 11:07:16 -0500 Subject: [PATCH 3/8] Apply Spotless formatting --- .../java/com/hubspot/jinjava/el/ext/BannedAllowlistOptions.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/com/hubspot/jinjava/el/ext/BannedAllowlistOptions.java b/src/main/java/com/hubspot/jinjava/el/ext/BannedAllowlistOptions.java index ed3f1246b..0f5295ba7 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/BannedAllowlistOptions.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/BannedAllowlistOptions.java @@ -7,6 +7,7 @@ import java.util.List; import java.util.Set; import java.util.stream.Stream; + public class BannedAllowlistOptions { // These aren't required, but they prevent someone from misconfiguring Jinjava to allow sandbox bypass unintentionally From 4230d8fbc2eefd452245f3e0d87c3b3bf1f9d052 Mon Sep 17 00:00:00 2001 From: jasmith-hs Date: Thu, 5 Mar 2026 13:30:13 -0500 Subject: [PATCH 4/8] Address further Sidekick review feedback - Use instanceof pattern matching in NoInvokeELContext to avoid bare ClassCastException when delegate doesn't implement HasInterpreter - Guard against null JinjavaInterpreter.getCurrent() in SizeLimitingPySet.checkSize Co-Authored-By: Claude Sonnet 4.6 --- .../java/com/hubspot/jinjava/el/NoInvokeELContext.java | 5 ++++- .../jinjava/objects/collections/SizeLimitingPySet.java | 8 +++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/hubspot/jinjava/el/NoInvokeELContext.java b/src/main/java/com/hubspot/jinjava/el/NoInvokeELContext.java index 0409f21aa..3d3f92f4e 100644 --- a/src/main/java/com/hubspot/jinjava/el/NoInvokeELContext.java +++ b/src/main/java/com/hubspot/jinjava/el/NoInvokeELContext.java @@ -35,6 +35,9 @@ public VariableMapper getVariableMapper() { @Override public JinjavaInterpreter interpreter() { - return ((HasInterpreter) delegate).interpreter(); + if (delegate instanceof HasInterpreter hasInterpreter) { + return hasInterpreter.interpreter(); + } + return JinjavaInterpreter.getCurrent(); } } diff --git a/src/main/java/com/hubspot/jinjava/objects/collections/SizeLimitingPySet.java b/src/main/java/com/hubspot/jinjava/objects/collections/SizeLimitingPySet.java index d7f100e2a..a81bff399 100644 --- a/src/main/java/com/hubspot/jinjava/objects/collections/SizeLimitingPySet.java +++ b/src/main/java/com/hubspot/jinjava/objects/collections/SizeLimitingPySet.java @@ -50,9 +50,11 @@ private void checkSize(int newSize) { throw new CollectionTooBigException(newSize, maxSize); } else if (!hasWarned && newSize >= maxSize * 0.9) { hasWarned = true; - JinjavaInterpreter - .getCurrent() - .addError( + JinjavaInterpreter current = JinjavaInterpreter.getCurrent(); + if (current == null) { + return; + } + current.addError( new TemplateError( ErrorType.WARNING, ErrorReason.COLLECTION_TOO_BIG, From ab6139d459f5b36a309e6ae6841d69123b3af269 Mon Sep 17 00:00:00 2001 From: jasmith-hs Date: Thu, 5 Mar 2026 13:31:04 -0500 Subject: [PATCH 5/8] Apply Spotless formatting --- .../collections/SizeLimitingPySet.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/main/java/com/hubspot/jinjava/objects/collections/SizeLimitingPySet.java b/src/main/java/com/hubspot/jinjava/objects/collections/SizeLimitingPySet.java index a81bff399..ac12e871c 100644 --- a/src/main/java/com/hubspot/jinjava/objects/collections/SizeLimitingPySet.java +++ b/src/main/java/com/hubspot/jinjava/objects/collections/SizeLimitingPySet.java @@ -55,16 +55,16 @@ private void checkSize(int newSize) { return; } current.addError( - new TemplateError( - ErrorType.WARNING, - ErrorReason.COLLECTION_TOO_BIG, - String.format("Set is at 90%% of max size (%d of %d)", newSize, maxSize), - null, - -1, - -1, - new CollectionTooBigException(newSize, maxSize) - ) - ); + new TemplateError( + ErrorType.WARNING, + ErrorReason.COLLECTION_TOO_BIG, + String.format("Set is at 90%% of max size (%d of %d)", newSize, maxSize), + null, + -1, + -1, + new CollectionTooBigException(newSize, maxSize) + ) + ); } } } From 6876ccb7a721af6710c34bbadcd70b4621615914 Mon Sep 17 00:00:00 2001 From: jasmith-hs Date: Thu, 5 Mar 2026 15:51:06 -0500 Subject: [PATCH 6/8] Handle missing interpreter in JinjavaBeanELResolver; add PySet to JinjavaObjects allowlist group Co-Authored-By: Claude Sonnet 4.6 --- .../jinjava/el/ext/AllowlistGroup.java | 2 ++ .../jinjava/el/ext/JinjavaBeanELResolver.java | 22 +++++++------------ 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/src/main/java/com/hubspot/jinjava/el/ext/AllowlistGroup.java b/src/main/java/com/hubspot/jinjava/el/ext/AllowlistGroup.java index 2ba2b8dd6..9d2fd1cb5 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/AllowlistGroup.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/AllowlistGroup.java @@ -68,8 +68,10 @@ String[] allowedDeclaredMethodsFromClasses() { private static final String[] ARRAY = { PyList.class.getCanonicalName(), PyMap.class.getCanonicalName(), + PySet.class.getCanonicalName(), SizeLimitingPyMap.class.getCanonicalName(), SizeLimitingPyList.class.getCanonicalName(), + SizeLimitingPySet.class.getCanonicalName(), SnakeCaseAccessibleMap.class.getCanonicalName(), FormattedDate.class.getCanonicalName(), PyishDate.class.getCanonicalName(), diff --git a/src/main/java/com/hubspot/jinjava/el/ext/JinjavaBeanELResolver.java b/src/main/java/com/hubspot/jinjava/el/ext/JinjavaBeanELResolver.java index 4d53d5cba..51db256bf 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/JinjavaBeanELResolver.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/JinjavaBeanELResolver.java @@ -2,7 +2,6 @@ import com.google.common.base.CaseFormat; import com.google.common.collect.ImmutableSet; -import com.hubspot.jinjava.JinjavaConfig; import com.hubspot.jinjava.interpret.DeferredValueException; import com.hubspot.jinjava.interpret.JinjavaInterpreter; import com.hubspot.jinjava.util.EagerReconstructionUtils; @@ -16,7 +15,6 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import javax.el.ELContext; @@ -222,30 +220,26 @@ protected Method findMethod( .orElseGet(() -> potentialMethods.stream().findAny().orElse(finalVarArgsMethod) ); } - return getJinjavaConfig().getMethodValidator().validateMethod(method); + return getAllowlistMethodValidator().validateMethod(method); } @Override protected Method getWriteMethod(Object base, Object property) { - return getJinjavaConfig() - .getMethodValidator() + return getAllowlistMethodValidator() .validateMethod(super.getWriteMethod(base, property)); } @Override protected Method getReadMethod(Object base, Object property) { - return getJinjavaConfig() - .getMethodValidator() + return getAllowlistMethodValidator() .validateMethod(super.getReadMethod(base, property)); } - private static JinjavaConfig getJinjavaConfig() { - return Objects - .requireNonNull( - JinjavaInterpreter.getCurrent(), - "JinjavaInterpreter.closeablePushCurrent must be used if using a JinjavaBeanELResolver directly" - ) - .getConfig(); + private static AllowlistMethodValidator getAllowlistMethodValidator() { + return JinjavaInterpreter + .getCurrentMaybe() + .map(interpreter -> interpreter.getConfig().getMethodValidator()) + .orElse(AllowlistMethodValidator.DEFAULT); } private static boolean checkAssignableParameterTypes(Object[] params, Method method) { From 4b2551f08c1939c6493d0fba9c8221659cce23b6 Mon Sep 17 00:00:00 2001 From: jasmith-hs Date: Thu, 5 Mar 2026 16:33:06 -0500 Subject: [PATCH 7/8] Add ReverseFilter tests for primitive and object arrays; fix ReverseFilter for primitive arrays Co-Authored-By: Claude Sonnet 4.6 --- .../jinjava/lib/filter/ReverseFilter.java | 21 +++++++------ .../objects/collections/ArrayBacked.java | 4 +-- .../jinjava/util/EagerExpressionResolver.java | 2 +- .../jinjava/lib/filter/ReverseFilterTest.java | 31 +++++++++++++++++++ 4 files changed, 45 insertions(+), 13 deletions(-) create mode 100644 src/test/java/com/hubspot/jinjava/lib/filter/ReverseFilterTest.java diff --git a/src/main/java/com/hubspot/jinjava/lib/filter/ReverseFilter.java b/src/main/java/com/hubspot/jinjava/lib/filter/ReverseFilter.java index 25fc8a22d..effe5a31c 100644 --- a/src/main/java/com/hubspot/jinjava/lib/filter/ReverseFilter.java +++ b/src/main/java/com/hubspot/jinjava/lib/filter/ReverseFilter.java @@ -20,6 +20,7 @@ import com.hubspot.jinjava.doc.annotations.JinjavaSnippet; import com.hubspot.jinjava.interpret.JinjavaInterpreter; import com.hubspot.jinjava.objects.collections.ArrayBacked; +import java.lang.reflect.Array; import java.util.Collection; import java.util.Iterator; import java.util.NoSuchElementException; @@ -54,7 +55,7 @@ public Object filter(Object object, JinjavaInterpreter interpreter, String... ar } // array if (object.getClass().isArray()) { - return ReverseArrayIterator.create((Object[]) object); + return ReverseArrayIterator.create(object); } // string if (object instanceof String) { @@ -76,26 +77,26 @@ public String getName() { return "reverse"; } - static class ReverseArrayIterator implements Iterator, ArrayBacked { + static class ReverseArrayIterator implements Iterator, ArrayBacked { - private final T[] array; + private final Object array; private int index; - static ReverseArrayIterator create(T[] array) { - return new ReverseArrayIterator<>(array); + static ReverseArrayIterator create(Object array) { + return new ReverseArrayIterator(array); } - private ReverseArrayIterator(T[] array) { + private ReverseArrayIterator(Object array) { this.array = array; - index = array.length - 1; + index = Array.getLength(array) - 1; } @Override - public T next() { + public Object next() { if (index < 0) { throw new NoSuchElementException(); } - return array[index--]; + return Array.get(array, index--); } @Override @@ -104,7 +105,7 @@ public boolean hasNext() { } @Override - public T[] backingArray() { + public Object backingArray() { return array; } } diff --git a/src/main/java/com/hubspot/jinjava/objects/collections/ArrayBacked.java b/src/main/java/com/hubspot/jinjava/objects/collections/ArrayBacked.java index 34a3586f4..05937c1ed 100644 --- a/src/main/java/com/hubspot/jinjava/objects/collections/ArrayBacked.java +++ b/src/main/java/com/hubspot/jinjava/objects/collections/ArrayBacked.java @@ -1,5 +1,5 @@ package com.hubspot.jinjava.objects.collections; -public interface ArrayBacked { - T[] backingArray(); +public interface ArrayBacked { + Object backingArray(); } diff --git a/src/main/java/com/hubspot/jinjava/util/EagerExpressionResolver.java b/src/main/java/com/hubspot/jinjava/util/EagerExpressionResolver.java index 1a17c5017..cd3b640d4 100644 --- a/src/main/java/com/hubspot/jinjava/util/EagerExpressionResolver.java +++ b/src/main/java/com/hubspot/jinjava/util/EagerExpressionResolver.java @@ -329,7 +329,7 @@ private static boolean isResolvableObjectRec( if (isPrimitive(val)) { return true; } - if (val instanceof ArrayBacked arrayBacked) { + if (val instanceof ArrayBacked arrayBacked) { val = arrayBacked.backingArray(); } try { diff --git a/src/test/java/com/hubspot/jinjava/lib/filter/ReverseFilterTest.java b/src/test/java/com/hubspot/jinjava/lib/filter/ReverseFilterTest.java new file mode 100644 index 000000000..7ded09ad8 --- /dev/null +++ b/src/test/java/com/hubspot/jinjava/lib/filter/ReverseFilterTest.java @@ -0,0 +1,31 @@ +package com.hubspot.jinjava.lib.filter; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.hubspot.jinjava.BaseJinjavaTest; +import java.util.HashMap; +import java.util.Map; +import org.junit.Test; + +public class ReverseFilterTest extends BaseJinjavaTest { + + @Test + public void itReversesPrimitiveIntArray() { + Map context = new HashMap<>(); + context.put("arr", new int[] { 1, 2, 3 }); + assertThat( + jinjava.render("{% for item in arr|reverse %}{{ item }}{% endfor %}", context) + ) + .isEqualTo("321"); + } + + @Test + public void itReversesObjectArray() { + Map context = new HashMap<>(); + context.put("arr", new String[] { "a", "b", "c" }); + assertThat( + jinjava.render("{% for item in arr|reverse %}{{ item }}{% endfor %}", context) + ) + .isEqualTo("cba"); + } +} From da2b67b28f9eaae67f2e27779aea44e1f3d1b4fa Mon Sep 17 00:00:00 2001 From: jasmith-hs Date: Fri, 6 Mar 2026 12:48:34 -0500 Subject: [PATCH 8/8] Address Sidekick review feedback: null check ordering in SizeLimitingPySet Co-Authored-By: Claude Sonnet 4.6 --- .../com/hubspot/jinjava/el/ext/JinjavaBeanELResolver.java | 2 +- .../jinjava/objects/collections/SizeLimitingPySet.java | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/hubspot/jinjava/el/ext/JinjavaBeanELResolver.java b/src/main/java/com/hubspot/jinjava/el/ext/JinjavaBeanELResolver.java index 51db256bf..dad811583 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/JinjavaBeanELResolver.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/JinjavaBeanELResolver.java @@ -168,7 +168,7 @@ protected void coerceValue( Class type ) { if (type.equals(JinjavaInterpreter.class)) { - Array.set(array, index, JinjavaInterpreter.getCurrent()); + Array.set(array, index, JinjavaInterpreter.getCurrent()); // Could be null if there's no current interpreter } else { super.coerceValue(array, index, factory, value, type); } diff --git a/src/main/java/com/hubspot/jinjava/objects/collections/SizeLimitingPySet.java b/src/main/java/com/hubspot/jinjava/objects/collections/SizeLimitingPySet.java index ac12e871c..b3c25a2fe 100644 --- a/src/main/java/com/hubspot/jinjava/objects/collections/SizeLimitingPySet.java +++ b/src/main/java/com/hubspot/jinjava/objects/collections/SizeLimitingPySet.java @@ -7,6 +7,7 @@ import com.hubspot.jinjava.interpret.TemplateError.ErrorType; import com.hubspot.jinjava.objects.PyWrapper; import java.util.Collection; +import java.util.Objects; import java.util.Set; import javax.annotation.Nonnull; @@ -16,10 +17,7 @@ public class SizeLimitingPySet extends PySet implements PyWrapper { private boolean hasWarned; public SizeLimitingPySet(Set set, int maxSize) { - super(set); - if (set == null) { - throw new IllegalArgumentException("set is null"); - } + super(Objects.requireNonNull(set, "set is null")); if (maxSize <= 0) { throw new IllegalArgumentException("maxSize must be >= 1"); }