diff --git a/docs/src/orchid/resources/changelog/v3_1_4.md b/docs/src/orchid/resources/changelog/v3_1_4.md index 4249b17ee..fc49e6f67 100644 --- a/docs/src/orchid/resources/changelog/v3_1_4.md +++ b/docs/src/orchid/resources/changelog/v3_1_4.md @@ -5,5 +5,6 @@ version: '3.1.4' - Slice filter: Use collection size when toIndex is greater than collection size (#504) - Adjust spring boot doc (#509) - Build with jdk14 (#508) -- Set proxyBeanMethods to false (#507) -- Add access to Spring Beans/request/session and response when using Pebble with WebFlux (#512) \ No newline at end of file +- Set proxyBeanMethods to false and build with spring boot 2.3 (#507) +- Add access to Spring Beans/request/session and response when using Pebble with WebFlux (#512) +- Remove allowUnsafeMethods property and replace it with methodAccessValidator. Default one is BlacklistMethodAccessValidtor (#511) \ No newline at end of file diff --git a/docs/src/orchid/resources/wiki/guide/installation.md b/docs/src/orchid/resources/wiki/guide/installation.md index cee1b8133..f46f6c1ea 100644 --- a/docs/src/orchid/resources/wiki/guide/installation.md +++ b/docs/src/orchid/resources/wiki/guide/installation.md @@ -84,7 +84,7 @@ All the settings are set during the construction of the `PebbleEngine` object. | `executorService` | An `ExecutorService` that allows the usage of some advanced multithreading features, such as the `parallel` tag. | `null` | | `loader` | An implementation of the `Loader` interface which is used to find templates. | An implementation of the `DelegatingLoader` which uses a `ClasspathLoader` and a `FileLoader` behind the scenes. | | `strictVariables` | If set to true, Pebble will throw an exception if you try to access a variable or attribute that does not exist (or an attribute of a null variable). If set to false, your template will treat non-existing variables/attributes as null without ever skipping a beat. | `false` | -| `allowUnsafeMethods` | If set to false, Pebble will throw an exception if you try to access unsafe methods. Unsafe methods are defined [here](https://github.com/PebbleTemplates/pebble/tree/master/pebble/src/main/resources/unsafeMethods.properties) | `true` in 2.x, 'false' in v3.x | +| `methodAccessValidator` | Pebble provides two implementations. NoOpMethodAccessValidator which do nothing and BlacklistMethodAccessValidator which checks that the method being called is not blacklisted. | `BlacklistMethodAccessValidator` | `literalDecimalTreatedAsInteger` | option for toggling to enable/disable literal decimal treated as integer | `false` | | `literalNumbersAsBigDecimals` | option for toggling to enable/disable literal numbers treated as BigDecimals | `false` | | `greedyMatchMethod` | option for toggling to enable/disable greedy matching mode for finding java method. Reduce the limit of the parameter type, try to find other method which has compatible parameter types. | `false` | diff --git a/docs/src/orchid/resources/wiki/guide/spring-boot-integration.md b/docs/src/orchid/resources/wiki/guide/spring-boot-integration.md index 5cc1095f8..eaf63f53e 100644 --- a/docs/src/orchid/resources/wiki/guide/spring-boot-integration.md +++ b/docs/src/orchid/resources/wiki/guide/spring-boot-integration.md @@ -54,6 +54,7 @@ A number of properties can be defined in Spring Boot externalized configuration, * ``pebble.exposeSessionAttributes``: defines whether all session attributes should be added to the model prior to merging with the template for the ViewResolver. Defaults to ``false`` * ``pebble.defaultLocale``: defines the default locale that will be used to configure the PebbleEngine. Defaults to ``null`` * ``pebble.strictVariables``: enable or disable the strict variable checking in the PebbleEngine. Defaults to ``false`` +* ``pebble.greedyMatchMethod``: enable or disable the greedy matching mode for finding java method in the PebbleEngine. Defaults to ``false`` ## Examples There is the spring petclinic example which has been migrated to [pebble](https://github.com/PebbleTemplates/spring-petclinic) @@ -86,7 +87,7 @@ public Loader pebbleLoader() { return new MyCustomLoader(); } ``` -PLEASE NOTE: this loader's prefix and suffix will be both overwritten when the ViewResolver is configured. You should use the externalized configuration for changing these properties. +**PLEASE NOTE**: this loader's prefix and suffix will be both overwritten when the ViewResolver is configured. You should use the externalized configuration for changing these properties. ### Customizing the PebbleEngine Likewise, you can build a custom engine and make it the default by using the bean name ``pebbleEngine``: @@ -97,8 +98,17 @@ public PebbleEngine pebbleEngine() { } ``` +### Customizing the MethodAccessValidator +You can provide your own MethodAccessValidator or switch to NoOpMethodAccessValidator by providing a MethodAccessValidator Bean +```java +@Bean +public MethodAccessValidator methodAccessValidator() { + return new NoOpMethodAccessValidator(); +} +``` + ### Customizing the ViewResolver -And the same goes for the ViewResolver, using the bean name ``pebbleViewResolver``: +And the same goes for the ViewResolver ```java @Bean public PebbleViewResolver pebbleViewResolver() { @@ -106,7 +116,7 @@ public PebbleViewResolver pebbleViewResolver() { } ``` -For reactive app, you need to use the bean name ``pebbleReactiveViewResolver``: +For reactive app ```java @Bean public PebbleReactiveViewResolver pebbleReactiveViewResolver() { diff --git a/pebble-spring/pebble-spring-boot-starter/pom.xml b/pebble-spring/pebble-spring-boot-starter/pom.xml index 9c90a6551..e23b1d67d 100644 --- a/pebble-spring/pebble-spring-boot-starter/pom.xml +++ b/pebble-spring/pebble-spring-boot-starter/pom.xml @@ -10,11 +10,11 @@ pebble-spring-boot-starter Pebble Spring Boot 2 Starter - Spring Boot 2 starter or Pebble Template Engine + Spring Boot 2 starter for Pebble Template Engine http://pebbletemplates.io - 2.2.6.RELEASE + 2.3.0.RELEASE diff --git a/pebble-spring/pebble-spring-boot-starter/src/main/java/com/mitchellbosecke/pebble/boot/autoconfigure/PebbleAutoConfiguration.java b/pebble-spring/pebble-spring-boot-starter/src/main/java/com/mitchellbosecke/pebble/boot/autoconfigure/PebbleAutoConfiguration.java index 076271af1..1e56d8c9b 100644 --- a/pebble-spring/pebble-spring-boot-starter/src/main/java/com/mitchellbosecke/pebble/boot/autoconfigure/PebbleAutoConfiguration.java +++ b/pebble-spring/pebble-spring-boot-starter/src/main/java/com/mitchellbosecke/pebble/boot/autoconfigure/PebbleAutoConfiguration.java @@ -1,6 +1,7 @@ package com.mitchellbosecke.pebble.boot.autoconfigure; import com.mitchellbosecke.pebble.PebbleEngine; +import com.mitchellbosecke.pebble.attributes.methodaccess.MethodAccessValidator; import com.mitchellbosecke.pebble.extension.Extension; import com.mitchellbosecke.pebble.loader.ClasspathLoader; import com.mitchellbosecke.pebble.loader.Loader; @@ -34,7 +35,7 @@ public Loader pebbleLoader(PebbleProperties properties) { @Bean @ConditionalOnMissingBean - public SpringExtension pebbleSpringExtension(MessageSource messageSource) { + public SpringExtension springExtension(MessageSource messageSource) { return new SpringExtension(messageSource); } @@ -43,7 +44,8 @@ public SpringExtension pebbleSpringExtension(MessageSource messageSource) { public PebbleEngine pebbleEngine(PebbleProperties properties, Loader pebbleLoader, SpringExtension springExtension, - @Nullable List extensions) { + @Nullable List extensions, + @Nullable MethodAccessValidator methodAccessValidator) { PebbleEngine.Builder builder = new PebbleEngine.Builder(); builder.loader(pebbleLoader); builder.extension(springExtension); @@ -58,6 +60,9 @@ public PebbleEngine pebbleEngine(PebbleProperties properties, } builder.strictVariables(properties.isStrictVariables()); builder.greedyMatchMethod(properties.isGreedyMatchMethod()); + if (methodAccessValidator != null) { + builder.methodAccessValidator(methodAccessValidator); + } return builder.build(); } } diff --git a/pebble-spring/pebble-spring-boot-starter/src/main/java/com/mitchellbosecke/pebble/boot/autoconfigure/PebbleReactiveWebConfiguration.java b/pebble-spring/pebble-spring-boot-starter/src/main/java/com/mitchellbosecke/pebble/boot/autoconfigure/PebbleReactiveWebConfiguration.java index 5979d6f72..4d40b79ff 100644 --- a/pebble-spring/pebble-spring-boot-starter/src/main/java/com/mitchellbosecke/pebble/boot/autoconfigure/PebbleReactiveWebConfiguration.java +++ b/pebble-spring/pebble-spring-boot-starter/src/main/java/com/mitchellbosecke/pebble/boot/autoconfigure/PebbleReactiveWebConfiguration.java @@ -14,7 +14,7 @@ class PebbleReactiveWebConfiguration extends AbstractPebbleConfiguration { @Bean - @ConditionalOnMissingBean(name = "pebbleReactiveViewResolver") + @ConditionalOnMissingBean PebbleReactiveViewResolver pebbleReactiveViewResolver(PebbleProperties properties, PebbleEngine pebbleEngine) { String prefix = properties.getPrefix(); diff --git a/pebble-spring/pebble-spring-boot-starter/src/main/java/com/mitchellbosecke/pebble/boot/autoconfigure/PebbleServletWebConfiguration.java b/pebble-spring/pebble-spring-boot-starter/src/main/java/com/mitchellbosecke/pebble/boot/autoconfigure/PebbleServletWebConfiguration.java index 122cdb680..b42b16a4e 100644 --- a/pebble-spring/pebble-spring-boot-starter/src/main/java/com/mitchellbosecke/pebble/boot/autoconfigure/PebbleServletWebConfiguration.java +++ b/pebble-spring/pebble-spring-boot-starter/src/main/java/com/mitchellbosecke/pebble/boot/autoconfigure/PebbleServletWebConfiguration.java @@ -14,7 +14,7 @@ class PebbleServletWebConfiguration extends AbstractPebbleConfiguration { @Bean - @ConditionalOnMissingBean(name = "pebbleViewResolver") + @ConditionalOnMissingBean PebbleViewResolver pebbleViewResolver(PebbleProperties properties, PebbleEngine pebbleEngine) { PebbleViewResolver pvr = new PebbleViewResolver(pebbleEngine); diff --git a/pebble-spring/pebble-spring-boot-starter/src/test/java/com/mitchellbosecke/pebble/boot/autoconfigure/PebbleAutoConfigurationTest.java b/pebble-spring/pebble-spring-boot-starter/src/test/java/com/mitchellbosecke/pebble/boot/autoconfigure/PebbleAutoConfigurationTest.java index 0938cc030..9ed885476 100644 --- a/pebble-spring/pebble-spring-boot-starter/src/test/java/com/mitchellbosecke/pebble/boot/autoconfigure/PebbleAutoConfigurationTest.java +++ b/pebble-spring/pebble-spring-boot-starter/src/test/java/com/mitchellbosecke/pebble/boot/autoconfigure/PebbleAutoConfigurationTest.java @@ -1,9 +1,13 @@ package com.mitchellbosecke.pebble.boot.autoconfigure; import static java.util.Locale.CHINESE; +import static java.util.Locale.FRENCH; import static org.assertj.core.api.Assertions.assertThat; import com.mitchellbosecke.pebble.PebbleEngine; +import com.mitchellbosecke.pebble.attributes.methodaccess.BlacklistMethodAccessValidator; +import com.mitchellbosecke.pebble.attributes.methodaccess.MethodAccessValidator; +import com.mitchellbosecke.pebble.attributes.methodaccess.NoOpMethodAccessValidator; import com.mitchellbosecke.pebble.loader.Loader; import com.mitchellbosecke.pebble.spring.extension.SpringExtension; import com.mitchellbosecke.pebble.spring.reactive.PebbleReactiveViewResolver; @@ -21,6 +25,7 @@ class PebbleAutoConfigurationTest { private static final Locale DEFAULT_LOCALE = CHINESE; + private static final Locale CUSTOM_LOCALE = FRENCH; private AnnotationConfigServletWebApplicationContext webContext; private AnnotationConfigReactiveWebApplicationContext reactiveWebContext; @@ -31,6 +36,15 @@ void registerBeansForServletApp() { assertThat(this.webContext.getBeansOfType(Loader.class)).hasSize(1); assertThat(this.webContext.getBeansOfType(SpringExtension.class)).hasSize(1); assertThat(this.webContext.getBeansOfType(PebbleEngine.class)).hasSize(1); + assertThat(this.webContext.getBean(PebbleEngine.class).getDefaultLocale()) + .isEqualTo(DEFAULT_LOCALE); + assertThat(this.webContext.getBean(PebbleEngine.class).isStrictVariables()).isTrue(); + assertThat( + this.webContext.getBean(PebbleEngine.class).getEvaluationOptions().isGreedyMatchMethod()) + .isTrue(); + assertThat(this.webContext.getBean(PebbleEngine.class).getEvaluationOptions() + .getMethodAccessValidator()).isInstanceOf( + BlacklistMethodAccessValidator.class); assertThat(this.webContext.getBeansOfType(PebbleViewResolver.class)).hasSize(1); } @@ -40,7 +54,34 @@ void registerCompilerForServletApp() { assertThat(this.webContext.getBeansOfType(Loader.class)).hasSize(1); assertThat(this.webContext.getBeansOfType(SpringExtension.class)).hasSize(1); assertThat(this.webContext.getBeansOfType(PebbleEngine.class)).hasSize(1); - assertThat(this.webContext.getBean(PebbleEngine.class).getDefaultLocale()).isEqualTo(DEFAULT_LOCALE); + assertThat(this.webContext.getBean(PebbleEngine.class).getDefaultLocale()) + .isEqualTo(CUSTOM_LOCALE); + assertThat(this.webContext.getBean(PebbleEngine.class).isStrictVariables()).isFalse(); + assertThat( + this.webContext.getBean(PebbleEngine.class).getEvaluationOptions().isGreedyMatchMethod()) + .isFalse(); + assertThat(this.webContext.getBean(PebbleEngine.class).getEvaluationOptions() + .getMethodAccessValidator()).isInstanceOf( + BlacklistMethodAccessValidator.class); + assertThat(this.webContext.getBeansOfType(PebbleViewResolver.class)).hasSize(1); + assertThat(this.webContext.getBeansOfType(PebbleReactiveViewResolver.class)).isEmpty(); + } + + @Test + void registerCustomMethodAccessValidatorForServletApp() { + this.loadWithServlet(CustomMethodAccessValidatorConfiguration.class); + assertThat(this.webContext.getBeansOfType(Loader.class)).hasSize(1); + assertThat(this.webContext.getBeansOfType(SpringExtension.class)).hasSize(1); + assertThat(this.webContext.getBeansOfType(PebbleEngine.class)).hasSize(1); + assertThat(this.webContext.getBean(PebbleEngine.class).getDefaultLocale()) + .isEqualTo(DEFAULT_LOCALE); + assertThat(this.webContext.getBean(PebbleEngine.class).isStrictVariables()).isTrue(); + assertThat( + this.webContext.getBean(PebbleEngine.class).getEvaluationOptions().isGreedyMatchMethod()) + .isTrue(); + assertThat(this.webContext.getBean(PebbleEngine.class).getEvaluationOptions() + .getMethodAccessValidator()).isInstanceOf( + NoOpMethodAccessValidator.class); assertThat(this.webContext.getBeansOfType(PebbleViewResolver.class)).hasSize(1); assertThat(this.webContext.getBeansOfType(PebbleReactiveViewResolver.class)).isEmpty(); } @@ -51,6 +92,14 @@ void registerBeansForReactiveApp() { assertThat(this.reactiveWebContext.getBeansOfType(Loader.class)).hasSize(1); assertThat(this.reactiveWebContext.getBeansOfType(SpringExtension.class)).hasSize(1); assertThat(this.reactiveWebContext.getBeansOfType(PebbleEngine.class)).hasSize(1); + assertThat(this.reactiveWebContext.getBean(PebbleEngine.class).getDefaultLocale()) + .isEqualTo(DEFAULT_LOCALE); + assertThat(this.reactiveWebContext.getBean(PebbleEngine.class).isStrictVariables()).isTrue(); + assertThat(this.reactiveWebContext.getBean(PebbleEngine.class).getEvaluationOptions() + .isGreedyMatchMethod()).isTrue(); + assertThat(this.reactiveWebContext.getBean(PebbleEngine.class).getEvaluationOptions() + .getMethodAccessValidator()).isInstanceOf( + BlacklistMethodAccessValidator.class); assertThat(this.reactiveWebContext.getBeansOfType(PebbleViewResolver.class)).isEmpty(); assertThat(this.reactiveWebContext.getBeansOfType(PebbleReactiveViewResolver.class)).hasSize(1); assertThat(this.reactiveWebContext.getBeansOfType(PebbleViewResolver.class)).isEmpty(); @@ -62,7 +111,33 @@ void registerCompilerForReactiveApp() { assertThat(this.reactiveWebContext.getBeansOfType(Loader.class)).hasSize(1); assertThat(this.reactiveWebContext.getBeansOfType(SpringExtension.class)).hasSize(1); assertThat(this.reactiveWebContext.getBeansOfType(PebbleEngine.class)).hasSize(1); - assertThat(this.reactiveWebContext.getBean(PebbleEngine.class).getDefaultLocale()).isEqualTo(DEFAULT_LOCALE); + assertThat(this.reactiveWebContext.getBean(PebbleEngine.class).getDefaultLocale()) + .isEqualTo(CUSTOM_LOCALE); + assertThat(this.reactiveWebContext.getBean(PebbleEngine.class).isStrictVariables()).isFalse(); + assertThat(this.reactiveWebContext.getBean(PebbleEngine.class).getEvaluationOptions() + .isGreedyMatchMethod()).isFalse(); + assertThat(this.reactiveWebContext.getBean(PebbleEngine.class).getEvaluationOptions() + .getMethodAccessValidator()).isInstanceOf( + BlacklistMethodAccessValidator.class); + assertThat(this.reactiveWebContext.getBeansOfType(PebbleReactiveViewResolver.class)).hasSize(1); + assertThat(this.reactiveWebContext.getBeansOfType(PebbleViewResolver.class)).isEmpty(); + } + + @Test + void registerCustomMethodAccessValidatorForReactiveApp() { + this.loadWithReactive(CustomMethodAccessValidatorConfiguration.class); + assertThat(this.reactiveWebContext.getBeansOfType(Loader.class)).hasSize(1); + assertThat(this.reactiveWebContext.getBeansOfType(SpringExtension.class)).hasSize(1); + assertThat(this.reactiveWebContext.getBeansOfType(PebbleEngine.class)).hasSize(1); + assertThat(this.reactiveWebContext.getBean(PebbleEngine.class).getDefaultLocale()) + .isEqualTo(DEFAULT_LOCALE); + assertThat(this.reactiveWebContext.getBean(PebbleEngine.class).isStrictVariables()).isTrue(); + assertThat(this.reactiveWebContext.getBean(PebbleEngine.class).getEvaluationOptions() + .isGreedyMatchMethod()).isTrue(); + assertThat(this.reactiveWebContext.getBean(PebbleEngine.class).getEvaluationOptions() + .getMethodAccessValidator()).isInstanceOf( + NoOpMethodAccessValidator.class); + assertThat(this.reactiveWebContext.getBeansOfType(PebbleViewResolver.class)).isEmpty(); assertThat(this.reactiveWebContext.getBeansOfType(PebbleReactiveViewResolver.class)).hasSize(1); assertThat(this.reactiveWebContext.getBeansOfType(PebbleViewResolver.class)).isEmpty(); } @@ -70,6 +145,9 @@ void registerCompilerForReactiveApp() { private void loadWithServlet(Class config) { this.webContext = new AnnotationConfigServletWebApplicationContext(); TestPropertyValues.of("pebble.prefix=classpath:/templates/").applyTo(this.webContext); + TestPropertyValues.of("pebble.defaultLocale=zh").applyTo(this.webContext); + TestPropertyValues.of("pebble.strictVariables=true").applyTo(this.webContext); + TestPropertyValues.of("pebble.greedyMatchMethod=true").applyTo(this.webContext); if (config != null) { this.webContext.register(config); } @@ -80,6 +158,9 @@ private void loadWithServlet(Class config) { private void loadWithReactive(Class config) { this.reactiveWebContext = new AnnotationConfigReactiveWebApplicationContext(); TestPropertyValues.of("pebble.prefix=classpath:/templates/").applyTo(this.reactiveWebContext); + TestPropertyValues.of("pebble.defaultLocale=zh").applyTo(this.reactiveWebContext); + TestPropertyValues.of("pebble.strictVariables=true").applyTo(this.reactiveWebContext); + TestPropertyValues.of("pebble.greedyMatchMethod=true").applyTo(this.reactiveWebContext); if (config != null) { this.reactiveWebContext.register(config); } @@ -98,7 +179,7 @@ protected static class CustomPebbleEngineCompilerConfiguration { @Bean public PebbleEngine pebbleEngine() { - return new PebbleEngine.Builder().defaultLocale(DEFAULT_LOCALE).build(); + return new PebbleEngine.Builder().defaultLocale(CUSTOM_LOCALE).build(); } @Bean @@ -107,4 +188,13 @@ public SpringExtension customSpringExtension(MessageSource messageSource) { } } + @Configuration(proxyBeanMethods = false) + protected static class CustomMethodAccessValidatorConfiguration { + + @Bean + public MethodAccessValidator methodAccessValidator() { + return new NoOpMethodAccessValidator(); + } + } + } \ No newline at end of file diff --git a/pebble-spring/pebble-spring5/pom.xml b/pebble-spring/pebble-spring5/pom.xml index dd8fa7d2b..f15d40f69 100644 --- a/pebble-spring/pebble-spring5/pom.xml +++ b/pebble-spring/pebble-spring5/pom.xml @@ -15,7 +15,7 @@ 3.1.0 - 5.2.1.RELEASE + 5.2.6.RELEASE 3.1.0 5.5.2 diff --git a/pebble/src/main/java/com/mitchellbosecke/pebble/PebbleEngine.java b/pebble/src/main/java/com/mitchellbosecke/pebble/PebbleEngine.java index 55b15c1f1..0393fabec 100644 --- a/pebble/src/main/java/com/mitchellbosecke/pebble/PebbleEngine.java +++ b/pebble/src/main/java/com/mitchellbosecke/pebble/PebbleEngine.java @@ -9,6 +9,8 @@ package com.mitchellbosecke.pebble; +import com.mitchellbosecke.pebble.attributes.methodaccess.BlacklistMethodAccessValidator; +import com.mitchellbosecke.pebble.attributes.methodaccess.MethodAccessValidator; import com.mitchellbosecke.pebble.cache.CacheKey; import com.mitchellbosecke.pebble.cache.PebbleCache; import com.mitchellbosecke.pebble.cache.tag.ConcurrentMapTagCache; @@ -39,14 +41,13 @@ import com.mitchellbosecke.pebble.template.EvaluationOptions; import com.mitchellbosecke.pebble.template.PebbleTemplate; import com.mitchellbosecke.pebble.template.PebbleTemplateImpl; - import java.io.IOException; import java.io.Reader; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Locale; import java.util.concurrent.ExecutorService; - import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -155,12 +156,12 @@ private PebbleTemplate getPebbleTemplate(String templateName, Loader loader, Obj Reader templateReader = loader.getReader(cacheKey); try { - logger.debug("Tokenizing template named {}", templateName); + this.logger.debug("Tokenizing template named {}", templateName); LexerImpl lexer = new LexerImpl(this.syntax, this.extensionRegistry.getUnaryOperators().values(), this.extensionRegistry.getBinaryOperators().values()); TokenStream tokenStream = lexer.tokenize(templateReader, templateName); - logger.trace("TokenStream: {}", tokenStream); + this.logger.trace("TokenStream: {}", tokenStream); Parser parser = new ParserImpl(this.extensionRegistry.getUnaryOperators(), this.extensionRegistry.getBinaryOperators(), this.extensionRegistry.getTokenParsers(), @@ -263,7 +264,7 @@ public static class Builder { private Loader loader; - private List userProvidedExtensions = new ArrayList<>(); + private final List userProvidedExtensions = new ArrayList<>(); private Syntax syntax; @@ -281,9 +282,7 @@ public static class Builder { private PebbleCache tagCache; - private EscaperExtension escaperExtension = new EscaperExtension(); - - private boolean allowUnsafeMethods; + private final EscaperExtension escaperExtension = new EscaperExtension(); private boolean literalDecimalTreatedAsInteger = false; @@ -293,6 +292,8 @@ public static class Builder { private boolean literalNumbersAsBigDecimals = false; + private MethodAccessValidator methodAccessValidator = new BlacklistMethodAccessValidator(); + /** * Creates the builder. */ @@ -318,9 +319,7 @@ public Builder loader(Loader loader) { * @return This builder object */ public Builder extension(Extension... extensions) { - for (Extension extension : extensions) { - this.userProvidedExtensions.add(extension); - } + Collections.addAll(this.userProvidedExtensions, extensions); return this; } @@ -479,13 +478,13 @@ public Builder cacheActive(boolean cacheActive) { } /** - * Enable/disable unsafe methods access for attributes + * Validator that can be used to validate object/method access * - * @param allowUnsafeMethods toggle to enable/disable unsafe methods access + * @param methodAccessValidator Validator that can be used to validate object/method access * @return This builder object */ - public Builder allowUnsafeMethods(boolean allowUnsafeMethods) { - this.allowUnsafeMethods = allowUnsafeMethods; + public Builder methodAccessValidator(MethodAccessValidator methodAccessValidator) { + this.methodAccessValidator = methodAccessValidator; return this; } @@ -584,10 +583,8 @@ public PebbleEngine build() { parserOptions.setLiteralDecimalTreatedAsInteger(this.literalDecimalTreatedAsInteger); parserOptions.setLiteralNumbersAsBigDecimals(this.literalNumbersAsBigDecimals); - EvaluationOptions evaluationOptions = new EvaluationOptions(); - evaluationOptions.setAllowUnsafeMethods(this.allowUnsafeMethods); - evaluationOptions.setGreedyMatchMethod(this.greedyMatchMethod); - + EvaluationOptions evaluationOptions = new EvaluationOptions(this.greedyMatchMethod, + this.methodAccessValidator); return new PebbleEngine(this.loader, this.syntax, this.strictVariables, this.defaultLocale, this.tagCache, this.templateCache, this.executorService, extensionRegistry, parserOptions, evaluationOptions); diff --git a/pebble/src/main/java/com/mitchellbosecke/pebble/attributes/DefaultAttributeResolver.java b/pebble/src/main/java/com/mitchellbosecke/pebble/attributes/DefaultAttributeResolver.java index f6559cc7f..f36211954 100644 --- a/pebble/src/main/java/com/mitchellbosecke/pebble/attributes/DefaultAttributeResolver.java +++ b/pebble/src/main/java/com/mitchellbosecke/pebble/attributes/DefaultAttributeResolver.java @@ -95,9 +95,9 @@ private Object invokeMember(Object object, Member member, Object[] argumentValue Object result = null; try { if (member instanceof Method) { - argumentValues = TypeUtils - .compatibleCast(argumentValues, ((Method) member).getParameterTypes()); - result = ((Method) member).invoke(object, argumentValues); + Method method = (Method) member; + argumentValues = TypeUtils.compatibleCast(argumentValues, method.getParameterTypes()); + result = method.invoke(object, argumentValues); } else if (member instanceof Field) { result = ((Field) member).get(object); } diff --git a/pebble/src/main/java/com/mitchellbosecke/pebble/attributes/MemberCacheUtils.java b/pebble/src/main/java/com/mitchellbosecke/pebble/attributes/MemberCacheUtils.java index 0068eee0b..79a14daaa 100644 --- a/pebble/src/main/java/com/mitchellbosecke/pebble/attributes/MemberCacheUtils.java +++ b/pebble/src/main/java/com/mitchellbosecke/pebble/attributes/MemberCacheUtils.java @@ -3,7 +3,6 @@ import com.mitchellbosecke.pebble.error.ClassAccessException; import com.mitchellbosecke.pebble.template.EvaluationContextImpl; import com.mitchellbosecke.pebble.template.EvaluationOptions; - import java.lang.reflect.AccessibleObject; import java.lang.reflect.Member; import java.lang.reflect.Method; @@ -16,7 +15,6 @@ import java.util.concurrent.ConcurrentHashMap; class MemberCacheUtils { - private final UnsafeMethods unsafeMethods = new UnsafeMethods(); private final ConcurrentHashMap memberCache = new ConcurrentHashMap<>(100, 0.9f, 1); @@ -30,10 +28,11 @@ Member cacheMember(Object instance, EvaluationContextImpl context, String filename, int lineNumber) { - Member member = this.reflect(instance, attributeName, argumentTypes, filename, lineNumber, - context.getEvaluationOptions()); + Member member = this.reflect(instance, attributeName, argumentTypes, + filename, lineNumber, context.getEvaluationOptions()); if (member != null) { - this.memberCache.put(new MemberCacheKey(instance.getClass(), attributeName, argumentTypes), member); + this.memberCache + .put(new MemberCacheKey(instance.getClass(), attributeName, argumentTypes), member); } return member; } @@ -49,7 +48,7 @@ private Member reflect(Object object, String attributeName, Class[] parameter // capitalize first letter of attribute for the following attempts String attributeCapitalized = Character.toUpperCase(attributeName.charAt(0)) + attributeName.substring(1); - + // search well known super classes first to avoid illegal reflective access List> agenda = Arrays.asList( List.class, @@ -67,26 +66,27 @@ private Member reflect(Object object, String attributeName, Class[] parameter // check get method Member result = this - .findMethod(type, "get" + attributeCapitalized, parameterTypes, filename, lineNumber, - evaluationOptions); + .findMethod(object, type, "get" + attributeCapitalized, parameterTypes, filename, + lineNumber, evaluationOptions); // check is method if (result == null) { result = this - .findMethod(type, "is" + attributeCapitalized, parameterTypes, filename, lineNumber, - evaluationOptions); + .findMethod(object, type, "is" + attributeCapitalized, parameterTypes, filename, + lineNumber, evaluationOptions); } // check has method if (result == null) { result = this - .findMethod(type, "has" + attributeCapitalized, parameterTypes, filename, lineNumber, + .findMethod(object, type, "has" + attributeCapitalized, parameterTypes, filename, + lineNumber, evaluationOptions); } // check if attribute is a public method if (result == null) { - result = this.findMethod(type, attributeName, parameterTypes, filename, lineNumber, + result = this.findMethod(object, type, attributeName, parameterTypes, filename, lineNumber, evaluationOptions); } @@ -110,8 +110,8 @@ private Member reflect(Object object, String attributeName, Class[] parameter * Finds an appropriate method by comparing if parameter types are compatible. This is more * relaxed than class.getMethod. */ - private Method findMethod(Class clazz, String name, Class[] requiredTypes, String filename, - int lineNumber, EvaluationOptions evaluationOptions) { + private Method findMethod(Object object, Class clazz, String name, Class[] requiredTypes, + String filename, int lineNumber, EvaluationOptions evaluationOptions) { List candidates = this.getCandidates(clazz, name, requiredTypes); // perfect match @@ -146,7 +146,7 @@ private Method findMethod(Class clazz, String name, Class[] requiredTypes, } } if(bestMatch != null) { - this.verifyUnsafeMethod(filename, lineNumber, evaluationOptions, bestMatch); + this.verifyUnsafeMethod(filename, lineNumber, evaluationOptions, object, bestMatch); return bestMatch; } @@ -163,7 +163,7 @@ private Method findMethod(Class clazz, String name, Class[] requiredTypes, } if (compatibleTypes) { - this.verifyUnsafeMethod(filename, lineNumber, evaluationOptions, candidate); + this.verifyUnsafeMethod(filename, lineNumber, evaluationOptions, object, candidate); return candidate; } } @@ -172,12 +172,16 @@ private Method findMethod(Class clazz, String name, Class[] requiredTypes, return null; } - private void verifyUnsafeMethod(String filename, int lineNumber, EvaluationOptions evaluationOptions, Method method) { - if (!evaluationOptions.isAllowUnsafeMethods() && this.unsafeMethods.isUnsafeMethod(method)) { - throw new ClassAccessException(lineNumber, filename); + private void verifyUnsafeMethod(String filename, int lineNumber, + EvaluationOptions evaluationOptions, Object object, Method method) { + boolean methodAccessAllowed = evaluationOptions.getMethodAccessValidator() + .isMethodAccessAllowed(object, method); + if (!methodAccessAllowed) { + throw new ClassAccessException(method, filename, lineNumber); } } + /** * Performs a widening conversion (primitive to boxed type) */ diff --git a/pebble/src/main/java/com/mitchellbosecke/pebble/attributes/methodaccess/BlacklistMethodAccessValidator.java b/pebble/src/main/java/com/mitchellbosecke/pebble/attributes/methodaccess/BlacklistMethodAccessValidator.java new file mode 100644 index 000000000..c5083da68 --- /dev/null +++ b/pebble/src/main/java/com/mitchellbosecke/pebble/attributes/methodaccess/BlacklistMethodAccessValidator.java @@ -0,0 +1,41 @@ +package com.mitchellbosecke.pebble.attributes.methodaccess; + +import java.lang.reflect.AccessibleObject; +import java.lang.reflect.Method; + +public class BlacklistMethodAccessValidator implements MethodAccessValidator { + + private static final String[] FORBIDDEN_METHODS = {"getClass", + "wait", + "notify", + "notifyAll"}; + + @Override + public boolean isMethodAccessAllowed(Object object, Method method) { + boolean methodForbidden = object instanceof Class + || object instanceof Runtime + || object instanceof Thread + || object instanceof ThreadGroup + || object instanceof System + || object instanceof AccessibleObject + || this.isUnsafeMethod(method); + return !methodForbidden; + } + + private boolean isUnsafeMethod(Method member) { + return this.isAnyOfMethods(member, FORBIDDEN_METHODS); + } + + private boolean isAnyOfMethods(Method member, String... methods) { + for (String method : methods) { + if (this.isMethodWithName(member, method)) { + return true; + } + } + return false; + } + + private boolean isMethodWithName(Method member, String method) { + return member.getName().equals(method); + } +} diff --git a/pebble/src/main/java/com/mitchellbosecke/pebble/attributes/methodaccess/MethodAccessValidator.java b/pebble/src/main/java/com/mitchellbosecke/pebble/attributes/methodaccess/MethodAccessValidator.java new file mode 100644 index 000000000..c5187f5d1 --- /dev/null +++ b/pebble/src/main/java/com/mitchellbosecke/pebble/attributes/methodaccess/MethodAccessValidator.java @@ -0,0 +1,8 @@ +package com.mitchellbosecke.pebble.attributes.methodaccess; + +import java.lang.reflect.Method; + +public interface MethodAccessValidator { + + boolean isMethodAccessAllowed(Object object, Method method); +} diff --git a/pebble/src/main/java/com/mitchellbosecke/pebble/attributes/methodaccess/NoOpMethodAccessValidator.java b/pebble/src/main/java/com/mitchellbosecke/pebble/attributes/methodaccess/NoOpMethodAccessValidator.java new file mode 100644 index 000000000..7aa43ce9d --- /dev/null +++ b/pebble/src/main/java/com/mitchellbosecke/pebble/attributes/methodaccess/NoOpMethodAccessValidator.java @@ -0,0 +1,11 @@ +package com.mitchellbosecke.pebble.attributes.methodaccess; + +import java.lang.reflect.Method; + +public class NoOpMethodAccessValidator implements MethodAccessValidator { + + @Override + public boolean isMethodAccessAllowed(Object object, Method method) { + return true; + } +} diff --git a/pebble/src/main/java/com/mitchellbosecke/pebble/error/ClassAccessException.java b/pebble/src/main/java/com/mitchellbosecke/pebble/error/ClassAccessException.java index d01faaa52..4f336ab97 100644 --- a/pebble/src/main/java/com/mitchellbosecke/pebble/error/ClassAccessException.java +++ b/pebble/src/main/java/com/mitchellbosecke/pebble/error/ClassAccessException.java @@ -8,12 +8,15 @@ */ package com.mitchellbosecke.pebble.error; +import java.lang.reflect.Method; + public class ClassAccessException extends PebbleException { private static final long serialVersionUID = 5109892021088141417L; - public ClassAccessException(Integer lineNumber, String filename) { - super(null, "For security reasons access to class/getClass attribute is denied.", lineNumber, + public ClassAccessException(Method method, String filename, Integer lineNumber) { + super(null, String.format("For security reasons access to %s method is denied.", method), + lineNumber, filename); } } diff --git a/pebble/src/main/java/com/mitchellbosecke/pebble/template/EvaluationOptions.java b/pebble/src/main/java/com/mitchellbosecke/pebble/template/EvaluationOptions.java index 412ce052f..c200d318d 100644 --- a/pebble/src/main/java/com/mitchellbosecke/pebble/template/EvaluationOptions.java +++ b/pebble/src/main/java/com/mitchellbosecke/pebble/template/EvaluationOptions.java @@ -1,5 +1,7 @@ package com.mitchellbosecke.pebble.template; +import com.mitchellbosecke.pebble.attributes.methodaccess.MethodAccessValidator; + /** * Evaluation options. * @@ -8,30 +10,26 @@ public class EvaluationOptions { /** - * toggle to enable/disable unsafe methods access + * toggle to enable/disable greedy matching mode for finding java method */ - private boolean allowUnsafeMethods; + private final boolean greedyMatchMethod; /** - * toggle to enable/disable greedy matching mode for finding java method + * Validator that can be used to validate object/method access */ - private boolean greedyMatchMethod; + private final MethodAccessValidator methodAccessValidator; - public boolean isAllowUnsafeMethods() { - return this.allowUnsafeMethods; - } - - public EvaluationOptions setAllowUnsafeMethods(boolean allowUnsafeMethods) { - this.allowUnsafeMethods = allowUnsafeMethods; - return this; + public EvaluationOptions(boolean greedyMatchMethod, + MethodAccessValidator methodAccessValidator) { + this.greedyMatchMethod = greedyMatchMethod; + this.methodAccessValidator = methodAccessValidator; } public boolean isGreedyMatchMethod() { return this.greedyMatchMethod; } - public EvaluationOptions setGreedyMatchMethod(boolean greedyMatchMethod) { - this.greedyMatchMethod = greedyMatchMethod; - return this; + public MethodAccessValidator getMethodAccessValidator() { + return this.methodAccessValidator; } } diff --git a/pebble/src/test/java/com/mitchellbosecke/pebble/GetAttributeTest.java b/pebble/src/test/java/com/mitchellbosecke/pebble/GetAttributeTest.java index 61c4fab0e..dd8f66561 100644 --- a/pebble/src/test/java/com/mitchellbosecke/pebble/GetAttributeTest.java +++ b/pebble/src/test/java/com/mitchellbosecke/pebble/GetAttributeTest.java @@ -8,15 +8,17 @@ */ package com.mitchellbosecke.pebble; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.fail; + +import com.mitchellbosecke.pebble.attributes.methodaccess.NoOpMethodAccessValidator; import com.mitchellbosecke.pebble.error.AttributeNotFoundException; import com.mitchellbosecke.pebble.error.ClassAccessException; import com.mitchellbosecke.pebble.error.PebbleException; import com.mitchellbosecke.pebble.error.RootAttributeNotFoundException; import com.mitchellbosecke.pebble.loader.StringLoader; import com.mitchellbosecke.pebble.template.PebbleTemplate; - -import org.junit.jupiter.api.Test; - import java.io.IOException; import java.io.StringWriter; import java.io.Writer; @@ -25,10 +27,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.fail; +import org.junit.jupiter.api.Test; class GetAttributeTest { @@ -229,7 +228,7 @@ void testMethodAttribute() throws PebbleException, IOException { void testAccessingClass_AllowUnsafeMethodsOn_StrictVariableOff_Property() throws PebbleException, IOException { PebbleEngine pebble = new PebbleEngine.Builder().loader(new StringLoader()) - .allowUnsafeMethods(true) + .methodAccessValidator(new NoOpMethodAccessValidator()) .strictVariables(false) .build(); @@ -246,7 +245,7 @@ void testAccessingClass_AllowUnsafeMethodsOn_StrictVariableOff_Property() void testAccessingClass_AllowUnsafeMethodsOn_StrictVariableOff_Method() throws PebbleException, IOException { PebbleEngine pebble = new PebbleEngine.Builder().loader(new StringLoader()) - .allowUnsafeMethods(true) + .methodAccessValidator(new NoOpMethodAccessValidator()) .strictVariables(false) .build(); @@ -263,7 +262,7 @@ void testAccessingClass_AllowUnsafeMethodsOn_StrictVariableOff_Method() void testAccessingClass_AllowUnsafeMethodsOn_StrictVariableOn_Property() throws PebbleException, IOException { PebbleEngine pebble = new PebbleEngine.Builder().loader(new StringLoader()) - .allowUnsafeMethods(true) + .methodAccessValidator(new NoOpMethodAccessValidator()) .strictVariables(true) .build(); @@ -280,7 +279,7 @@ void testAccessingClass_AllowUnsafeMethodsOn_StrictVariableOn_Property() void testAccessingClass_AllowUnsafeMethodsOn_StrictVariableOn_Method() throws PebbleException, IOException { PebbleEngine pebble = new PebbleEngine.Builder().loader(new StringLoader()) - .allowUnsafeMethods(true) + .methodAccessValidator(new NoOpMethodAccessValidator()) .strictVariables(true) .build(); @@ -298,7 +297,6 @@ void testAccessingClass_AllowUnsafeMethodsOff_StrictVariableOff_Property() throws PebbleException, IOException { assertThrows(ClassAccessException.class, () -> { PebbleEngine pebble = new PebbleEngine.Builder().loader(new StringLoader()) - .allowUnsafeMethods(false) .strictVariables(false) .build(); @@ -316,7 +314,6 @@ void testAccessingClass_AllowUnsafeMethodsOff_StrictVariableOff_Method() throws PebbleException, IOException { assertThrows(ClassAccessException.class, () -> { PebbleEngine pebble = new PebbleEngine.Builder().loader(new StringLoader()) - .allowUnsafeMethods(false) .strictVariables(false) .build(); @@ -334,7 +331,6 @@ void testAccessingClass_AllowUnsafeMethodsOff_StrictVariableOn_Property() throws PebbleException, IOException { assertThrows(ClassAccessException.class, () -> { PebbleEngine pebble = new PebbleEngine.Builder().loader(new StringLoader()) - .allowUnsafeMethods(false) .strictVariables(true) .build(); @@ -352,7 +348,6 @@ void testAccessingClass_AllowUnsafeMethodsOff_StrictVariableOn_Method() throws PebbleException, IOException { assertThrows(ClassAccessException.class, () -> { PebbleEngine pebble = new PebbleEngine.Builder().loader(new StringLoader()) - .allowUnsafeMethods(false) .strictVariables(true) .build(); @@ -370,7 +365,7 @@ void testAccessingClass_AllowUnsafeMethodsOnIsCaseInsensitive_Property() throws PebbleException, IOException { PebbleEngine pebble = new PebbleEngine.Builder() .loader(new StringLoader()) - .allowUnsafeMethods(true) + .methodAccessValidator(new NoOpMethodAccessValidator()) .build(); PebbleTemplate template = pebble.getTemplate("hello [{{ object.ClAsS }}]"); @@ -388,7 +383,6 @@ void testAccessingClass_AllowUnsafeMethodsOffIsCaseInsensitive_Property() assertThrows(ClassAccessException.class, () -> { PebbleEngine pebble = new PebbleEngine.Builder() .loader(new StringLoader()) - .allowUnsafeMethods(false) .build(); PebbleTemplate template = pebble.getTemplate("hello [{{ object.ClAsS }}]"); @@ -405,7 +399,7 @@ void testAccessingClass_AllowUnsafeMethodsOnIsCaseInsensitive_Method() throws PebbleException, IOException { PebbleEngine pebble = new PebbleEngine.Builder() .loader(new StringLoader()) - .allowUnsafeMethods(true) + .methodAccessValidator(new NoOpMethodAccessValidator()) .build(); PebbleTemplate template = pebble.getTemplate("hello [{{ object.GeTcLAsS() }}]"); @@ -423,7 +417,6 @@ void testAccessingClass_AllowUnsafeMethodsOffIsCaseInsensitive_Method() assertThrows(ClassAccessException.class, () -> { PebbleEngine pebble = new PebbleEngine.Builder() .loader(new StringLoader()) - .allowUnsafeMethods(false) .build(); PebbleTemplate template = pebble.getTemplate("hello [{{ object.GeTcLAsS() }}]"); @@ -441,7 +434,6 @@ void testAccessingClass_AllowUnsafeMethodsOffForMethodNotify_thenThrowException( assertThrows(ClassAccessException.class, () -> { PebbleEngine pebble = new PebbleEngine.Builder() .loader(new StringLoader()) - .allowUnsafeMethods(false) .build(); PebbleTemplate template = pebble.getTemplate("hello [{{ object.notify() }}]"); diff --git a/pebble/src/test/java/com/mitchellbosecke/pebble/MethodAccessTemplateTest.java b/pebble/src/test/java/com/mitchellbosecke/pebble/MethodAccessTemplateTest.java new file mode 100644 index 000000000..0a5bfe99b --- /dev/null +++ b/pebble/src/test/java/com/mitchellbosecke/pebble/MethodAccessTemplateTest.java @@ -0,0 +1,180 @@ +package com.mitchellbosecke.pebble; + +import static org.junit.jupiter.api.Assertions.assertThrows; + +import com.mitchellbosecke.pebble.attributes.methodaccess.NoOpMethodAccessValidator; +import com.mitchellbosecke.pebble.error.ClassAccessException; +import com.mitchellbosecke.pebble.loader.StringLoader; +import com.mitchellbosecke.pebble.template.PebbleTemplate; +import java.io.IOException; +import java.io.StringWriter; +import java.io.Writer; +import java.lang.reflect.Constructor; +import java.lang.reflect.Method; +import java.util.HashMap; +import java.util.Map; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.function.Executable; + +class MethodAccessTemplateTest { + + @Nested + class ClassTest { + + @Test + void testIfAccessIsForbiddenWhenAllowUnsafeMethodsIsFalse() { + PebbleEngine pebble = MethodAccessTemplateTest.this.pebbleEngine(); + assertThrows(ClassAccessException.class, this.templateEvaluation(pebble)); + } + + @Test + void testIfAccessIsAllowedWhenAllowUnsafeMethodsIsTrue() throws Throwable { + PebbleEngine pebble = MethodAccessTemplateTest.this.unsafePebbleEngine(); + this.templateEvaluation(pebble).execute(); + } + + private Executable templateEvaluation(PebbleEngine pebble) { + return () -> { + String source = "{{clazz.getPackage()}}"; + PebbleTemplate template = pebble.getTemplate(source); + Map context = new HashMap<>(); + context.put("clazz", Object.class); + MethodAccessTemplateTest.this.evaluateTemplate(template, context); + }; + } + } + + @Nested + class RuntimeTest { + + @Test + void testIfAccessIsForbiddenWhenAllowUnsafeMethodsIsFalse() { + PebbleEngine pebble = MethodAccessTemplateTest.this.pebbleEngine(); + assertThrows(ClassAccessException.class, this.templateEvaluation(pebble)); + } + + @Test + void testIfAccessIsAllowedWhenAllowUnsafeMethodsIsTrue() throws Throwable { + PebbleEngine pebble = MethodAccessTemplateTest.this.unsafePebbleEngine(); + this.templateEvaluation(pebble).execute(); + } + + private Executable templateEvaluation(PebbleEngine pebble) { + return () -> { + String source = "{{runtime.availableProcessors()}}"; + PebbleTemplate template = pebble.getTemplate(source); + Map context = new HashMap<>(); + context.put("runtime", Runtime.getRuntime()); + MethodAccessTemplateTest.this.evaluateTemplate(template, context); + }; + } + } + + @Nested + class ThreadTest { + + @Test + void testIfAccessIsForbiddenWhenAllowUnsafeMethodsIsFalse() { + PebbleEngine pebble = MethodAccessTemplateTest.this.pebbleEngine(); + assertThrows(ClassAccessException.class, this.templateEvaluation(pebble)); + } + + @Test + void testIfAccessIsAllowedWhenAllowUnsafeMethodsIsTrue() throws Throwable { + PebbleEngine pebble = MethodAccessTemplateTest.this.unsafePebbleEngine(); + this.templateEvaluation(pebble).execute(); + } + + private Executable templateEvaluation(PebbleEngine pebble) { + return () -> { + String source = "{{thread.sleep(500)}}"; + PebbleTemplate template = pebble.getTemplate(source); + Map context = new HashMap<>(); + context.put("thread", new Thread()); + MethodAccessTemplateTest.this.evaluateTemplate(template, context); + }; + } + } + + @Nested + class SystemTest { + + @Test + void testIfAccessIsForbiddenWhenAllowUnsafeMethodsIsFalse() { + PebbleEngine pebble = MethodAccessTemplateTest.this.pebbleEngine(); + assertThrows(ClassAccessException.class, this.templateEvaluation(pebble)); + } + + @Test + void testIfAccessIsAllowedWhenAllowUnsafeMethodsIsTrue() throws Throwable { + PebbleEngine pebble = MethodAccessTemplateTest.this.unsafePebbleEngine(); + this.templateEvaluation(pebble).execute(); + } + + private Executable templateEvaluation(PebbleEngine pebble) { + return () -> { + Class systemClass = Class.forName("java.lang.System"); + systemClass.getMethod("gc").setAccessible(true); + + Constructor constructor = systemClass.getDeclaredConstructor(); + constructor.setAccessible(true); + System system = (System) constructor.newInstance(); + + String source = "{{system.gc()}}"; + PebbleTemplate template = pebble.getTemplate(source); + Map context = new HashMap<>(); + context.put("system", system); + MethodAccessTemplateTest.this.evaluateTemplate(template, context); + }; + } + } + + @Nested + class MethodTest { + + @Test + void testIfAccessIsForbiddenWhenAllowUnsafeMethodsIsFalse() { + PebbleEngine pebble = MethodAccessTemplateTest.this.pebbleEngine(); + assertThrows(ClassAccessException.class, this.templateEvaluation(pebble)); + } + + @Test + void testIfAccessIsAllowedWhenAllowUnsafeMethodsIsTrue() throws Throwable { + PebbleEngine pebble = MethodAccessTemplateTest.this.unsafePebbleEngine(); + this.templateEvaluation(pebble).execute(); + } + + private Executable templateEvaluation(PebbleEngine pebble) { + return () -> { + Class systemClass = Class.forName("java.lang.System"); + Method gcMethod = systemClass.getMethod("gc"); + gcMethod.setAccessible(true); + gcMethod.invoke(null); + + String source = "{{gc.invoke(null, null)}}"; + PebbleTemplate template = pebble.getTemplate(source); + Map context = new HashMap<>(); + context.put("gc", gcMethod); + + MethodAccessTemplateTest.this.evaluateTemplate(template, context); + }; + } + } + + private PebbleEngine unsafePebbleEngine() { + return new PebbleEngine.Builder().loader(new StringLoader()) + .methodAccessValidator(new NoOpMethodAccessValidator()) + .build(); + } + + private PebbleEngine pebbleEngine() { + return new PebbleEngine.Builder().loader(new StringLoader()).build(); + } + + private void evaluateTemplate(PebbleTemplate template, Map context) + throws IOException { + Writer writer = new StringWriter(); + template.evaluate(writer, context); + } +} \ No newline at end of file diff --git a/pebble/src/test/java/com/mitchellbosecke/pebble/attributes/methodaccess/BlacklistMethodAccessValidatorTest.java b/pebble/src/test/java/com/mitchellbosecke/pebble/attributes/methodaccess/BlacklistMethodAccessValidatorTest.java new file mode 100644 index 000000000..877d2661f --- /dev/null +++ b/pebble/src/test/java/com/mitchellbosecke/pebble/attributes/methodaccess/BlacklistMethodAccessValidatorTest.java @@ -0,0 +1,46 @@ +package com.mitchellbosecke.pebble.attributes.methodaccess; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.lang.reflect.Method; +import java.util.stream.Stream; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; + + +class BlacklistMethodAccessValidatorTest { + + private final InstanceProvider instanceProvider = new InstanceProvider(); + private final MethodAccessValidator underTest = new BlacklistMethodAccessValidator(); + + @ParameterizedTest + @MethodSource("provideUnsafeMethod") + void checkIfAccessIsForbidden(Method unsafeMethod) + throws NoSuchFieldException, NoSuchMethodException { + Class declaringClass = unsafeMethod.getDeclaringClass(); + Object instance = this.instanceProvider.createObject(declaringClass); + + boolean methodAccessAllowed = this.underTest.isMethodAccessAllowed(instance, unsafeMethod); + + assertThat(methodAccessAllowed).isFalse(); + } + + @ParameterizedTest + @MethodSource("provideAllowedMethod") + void checkIfAccessIsAllowed(Method allowedMethod) throws Throwable { + Class declaringClass = allowedMethod.getDeclaringClass(); + Object instance = this.instanceProvider.createObject(declaringClass); + + boolean methodAccessAllowed = this.underTest.isMethodAccessAllowed(instance, allowedMethod); + + assertThat(methodAccessAllowed).isTrue(); + } + + private static Stream provideUnsafeMethod() { + return MethodsProvider.UNSAFE_METHODS.stream(); + } + + private static Stream provideAllowedMethod() { + return MethodsProvider.ALLOWED_METHODS.stream(); + } +} \ No newline at end of file diff --git a/pebble/src/test/java/com/mitchellbosecke/pebble/attributes/methodaccess/Foo.java b/pebble/src/test/java/com/mitchellbosecke/pebble/attributes/methodaccess/Foo.java new file mode 100644 index 000000000..ea236f60a --- /dev/null +++ b/pebble/src/test/java/com/mitchellbosecke/pebble/attributes/methodaccess/Foo.java @@ -0,0 +1,16 @@ +package com.mitchellbosecke.pebble.attributes.methodaccess; + +public class Foo { + + private String x; + + public void getX() { + } + + private Foo() { + } + + public void setX(String x) { + this.x = x; + } +} \ No newline at end of file diff --git a/pebble/src/test/java/com/mitchellbosecke/pebble/attributes/methodaccess/InstanceProvider.java b/pebble/src/test/java/com/mitchellbosecke/pebble/attributes/methodaccess/InstanceProvider.java new file mode 100644 index 000000000..b45d8db59 --- /dev/null +++ b/pebble/src/test/java/com/mitchellbosecke/pebble/attributes/methodaccess/InstanceProvider.java @@ -0,0 +1,30 @@ +package com.mitchellbosecke.pebble.attributes.methodaccess; + +import java.lang.reflect.Constructor; + +public class InstanceProvider { + + Object createObject(Class declaringClass) throws NoSuchFieldException, NoSuchMethodException { + try { + Constructor constructor = declaringClass.getDeclaredConstructor(); + constructor.setAccessible(true); + return constructor.newInstance(); + } catch (Exception e) { + switch (declaringClass.getName()) { + case "java.lang.reflect.Field": + return Foo.class.getDeclaredField("x"); + case "java.lang.reflect.Method": + return Foo.class.getDeclaredMethod("getX", (Class[]) null); + case "java.lang.Class": + return Foo.class; + case "java.lang.reflect.Constructor": + return Foo.class.getDeclaredConstructor(); + case "java.lang.Integer": + return Integer.valueOf(1); + default: + throw new RuntimeException( + String.format("No object instance defined for class %s", declaringClass.getName())); + } + } + } +} \ No newline at end of file diff --git a/pebble/src/main/java/com/mitchellbosecke/pebble/attributes/UnsafeMethods.java b/pebble/src/test/java/com/mitchellbosecke/pebble/attributes/methodaccess/MethodsProvider.java similarity index 67% rename from pebble/src/main/java/com/mitchellbosecke/pebble/attributes/UnsafeMethods.java rename to pebble/src/test/java/com/mitchellbosecke/pebble/attributes/methodaccess/MethodsProvider.java index 3bafa54d3..4189f99fd 100644 --- a/pebble/src/main/java/com/mitchellbosecke/pebble/attributes/UnsafeMethods.java +++ b/pebble/src/test/java/com/mitchellbosecke/pebble/attributes/methodaccess/MethodsProvider.java @@ -1,4 +1,4 @@ -package com.mitchellbosecke.pebble.attributes; +package com.mitchellbosecke.pebble.attributes.methodaccess; import java.io.IOException; @@ -11,20 +11,28 @@ import java.util.Set; import java.util.StringTokenizer; -class UnsafeMethods { +class MethodsProvider { - private static final String UNSAFE_METHODS_PROPERTIES = "/unsafeMethods.properties"; - private static final Set UNSAFE_METHODS = createUnsafeMethodsSet(); + private static final String UNSAFE_METHODS_PROPERTIES = "/security/unsafeMethods.properties"; + private static final String ALLOWED_METHODS_PROPERTIES = "/security/allowedMethods.properties"; - UnsafeMethods() { } + static final Set UNSAFE_METHODS = createUnsafeMethodsSet(); + static final Set ALLOWED_METHODS = createAllowedMethodsSet(); - boolean isUnsafeMethod(Method method) { - return UNSAFE_METHODS.contains(method); + MethodsProvider() { } - private static final Set createUnsafeMethodsSet() { + private static Set createAllowedMethodsSet() { + return createMethodsSet(ALLOWED_METHODS_PROPERTIES); + } + + private static Set createUnsafeMethodsSet() { + return createMethodsSet(UNSAFE_METHODS_PROPERTIES); + } + + private static Set createMethodsSet(String filename) { try { - Properties props = loadProperties(UNSAFE_METHODS_PROPERTIES); + Properties props = loadProperties(filename); Set set = new HashSet<>(props.size() * 4 / 3, 1f); Map primClasses = createPrimitiveClassesMap(); for (Object key : props.keySet()) { @@ -32,7 +40,8 @@ private static final Set createUnsafeMethodsSet() { } return set; } catch (Exception e) { - throw new RuntimeException("Could not load unsafe method set", e); + throw new RuntimeException(String.format("Could not load method set from file %s", filename), + e); } } @@ -72,14 +81,8 @@ private static Map createPrimitiveClassesMap() { private static Properties loadProperties(String resource) throws IOException { Properties props = new Properties(); - InputStream is = null; - try { - is = UnsafeMethods.class.getResourceAsStream(resource); + try (InputStream is = MethodsProvider.class.getResourceAsStream(resource)) { props.load(is); - } finally { - if (is != null) { - is.close(); - } } return props; } diff --git a/pebble/src/test/java/com/mitchellbosecke/pebble/attributes/methodaccess/NoOpMethodAccessValidatorTest.java b/pebble/src/test/java/com/mitchellbosecke/pebble/attributes/methodaccess/NoOpMethodAccessValidatorTest.java new file mode 100644 index 000000000..7a8bf0f44 --- /dev/null +++ b/pebble/src/test/java/com/mitchellbosecke/pebble/attributes/methodaccess/NoOpMethodAccessValidatorTest.java @@ -0,0 +1,17 @@ +package com.mitchellbosecke.pebble.attributes.methodaccess; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.jupiter.api.Test; + +public class NoOpMethodAccessValidatorTest { + + private final MethodAccessValidator underTest = new NoOpMethodAccessValidator(); + + @Test + void whenIsMethodAccessAllowed_thenReturnTrue() { + boolean methodAccessAllowed = this.underTest.isMethodAccessAllowed(null, null); + + assertThat(methodAccessAllowed).isTrue(); + } +} diff --git a/pebble/src/test/resources/security/allowedMethods.properties b/pebble/src/test/resources/security/allowedMethods.properties new file mode 100644 index 000000000..f59193424 --- /dev/null +++ b/pebble/src/test/resources/security/allowedMethods.properties @@ -0,0 +1,9 @@ +java.lang.Object.toString() +java.lang.Object.hashCode() +java.lang.Object.equals(java.lang.Object) + +com.mitchellbosecke.pebble.attributes.methodaccess.Foo.getX() +com.mitchellbosecke.pebble.attributes.methodaccess.Foo.setX(java.lang.String) +com.mitchellbosecke.pebble.attributes.methodaccess.Foo.toString() +com.mitchellbosecke.pebble.attributes.methodaccess.Foo.hashCode() +com.mitchellbosecke.pebble.attributes.methodaccess.Foo.equals(java.lang.Object) \ No newline at end of file diff --git a/pebble/src/main/resources/unsafeMethods.properties b/pebble/src/test/resources/security/unsafeMethods.properties similarity index 99% rename from pebble/src/main/resources/unsafeMethods.properties rename to pebble/src/test/resources/security/unsafeMethods.properties index 080bd60d4..e86ec4508 100644 --- a/pebble/src/main/resources/unsafeMethods.properties +++ b/pebble/src/test/resources/security/unsafeMethods.properties @@ -68,7 +68,6 @@ java.lang.ThreadGroup.resume() java.lang.ThreadGroup.setDaemon(boolean) java.lang.ThreadGroup.setMaxPriority(int) java.lang.ThreadGroup.stop() -java.lang.Thread.suspend() java.lang.Runtime.addShutdownHook(java.lang.Thread) java.lang.Runtime.exec(java.lang.String)