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

Author: jasmith-hs

3.0-changes.md

@@ -0,0 +1,6 @@
+- Migrated JinjavaConfig to immutable
+- Removed whitespaceRequiredWithinTokens LegacyOverride and converted to BuiltinFeatures
+- Nested interpretation disabled by default
+- Removed restricted methods and properties from JinjavaConfig
+- Turn on all current LegacyOverrides by default
+- Use MethodValidator and ReturnTypeValidator to enforce method invocations and objects returned by ELResolver

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<Library, Set<String>> getDisabled();
-  Set<String> getRestrictedMethods();
-  Set<String> 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()

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<String> 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, "");
+  }
 }

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();
+}

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) {

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<Object>) value,
+        interpreter.getConfig().getMaxListSize()
+      );
+    }
     if (Map.class.isAssignableFrom(value.getClass())) {
       // FIXME: ensure keys are actually strings, if not, convert them
       return new SizeLimitingPyMap(

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();
+  }
 }

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<FeatureDescriptor> 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);
+  }
+}