Skip to content

Commit 7492be2

Browse files
jasmith-hsjasmith-hs
committed
Extract separate MethodValidator
1 parent f469eef commit 7492be2

7 files changed

Lines changed: 236 additions & 138 deletions

File tree

‎src/main/java/com/hubspot/jinjava/JinjavaConfig.java‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,6 @@
2222
import com.fasterxml.jackson.datatype.jdk8.Jdk8Module;
2323
import com.google.common.collect.ImmutableMap;
2424
import com.google.common.collect.ImmutableSet;
25-
import com.hubspot.immutable.collection.encoding.ImmutableListEncodingEnabled;
26-
import com.hubspot.immutable.collection.encoding.ImmutableMapEncodingEnabled;
27-
import com.hubspot.immutable.collection.encoding.ImmutableSetEncodingEnabled;
2825
import com.hubspot.jinjava.el.JinjavaInterpreterResolver;
2926
import com.hubspot.jinjava.el.JinjavaObjectUnwrapper;
3027
import com.hubspot.jinjava.el.JinjavaProcessors;
@@ -54,13 +51,7 @@
5451
import org.immutables.value.Value;
5552

5653
@Value.Immutable(singleton = true)
57-
@Value.Style(
58-
init = "with*",
59-
get = { "is*", "get*" } // Detect 'get' and 'is' prefixes in accessor methods
60-
)
61-
@ImmutableSetEncodingEnabled
62-
@ImmutableListEncodingEnabled
63-
@ImmutableMapEncodingEnabled
54+
@JinjavaImmutableStyle
6455
public interface JinjavaConfig {
6556
@Value.Default
6657
default Charset getCharset() {

‎src/main/java/com/hubspot/jinjava/JinjavaImmutableStyle.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
package com.hubspot.jinjava;
2+
3+
import com.hubspot.immutable.collection.encoding.ImmutableListEncodingEnabled;
4+
import com.hubspot.immutable.collection.encoding.ImmutableMapEncodingEnabled;
5+
import com.hubspot.immutable.collection.encoding.ImmutableSetEncodingEnabled;
6+
import org.immutables.value.Value;
7+
8+
@Value.Style(
9+
init = "with*",
10+
get = { "is*", "get*" } // Detect 'get' and 'is' prefixes in accessor methods
11+
)
12+
@ImmutableSetEncodingEnabled
13+
@ImmutableListEncodingEnabled
14+
@ImmutableMapEncodingEnabled
15+
public @interface JinjavaImmutableStyle {
16+
}

‎src/main/java/com/hubspot/jinjava/el/JinjavaInterpreterResolver.java‎

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
import com.hubspot.jinjava.el.ext.AbstractCallableMethod;
77
import com.hubspot.jinjava.el.ext.DeferredParsingException;
88
import com.hubspot.jinjava.el.ext.ExtendedParser;
9-
import com.hubspot.jinjava.el.ext.ImmutableJinjavaBeanELResolverConfig;
109
import com.hubspot.jinjava.el.ext.JinjavaBeanELResolver;
1110
import com.hubspot.jinjava.el.ext.JinjavaListELResolver;
1211
import com.hubspot.jinjava.el.ext.NamedParameter;
@@ -57,11 +56,14 @@ public class JinjavaInterpreterResolver extends SimpleResolver {
5756
add(new JinjavaListELResolver(true));
5857
add(new TypeConvertingMapELResolver(true));
5958
add(new ResourceBundleELResolver());
60-
add(new JinjavaBeanELResolver(
61-
new JinjavaBeanELResolver.JinjavaBeanELResolverConfig.Builder()
62-
.readOnly(true)
63-
.build()
64-
));
59+
add(
60+
new JinjavaBeanELResolver(
61+
JinjavaBeanELResolver.JinjavaBeanELResolverConfig
62+
.builder()
63+
.withReadOnly(true)
64+
.build()
65+
)
66+
);
6567
}
6668
};
6769

@@ -71,11 +73,14 @@ public class JinjavaInterpreterResolver extends SimpleResolver {
7173
add(new JinjavaListELResolver(false));
7274
add(new TypeConvertingMapELResolver(false));
7375
add(new ResourceBundleELResolver());
74-
add(new JinjavaBeanELResolver(
75-
new JinjavaBeanELResolver.JinjavaBeanELResolverConfig.Builder()
76-
.readOnly(false)
77-
.build()
78-
));
76+
add(
77+
new JinjavaBeanELResolver(
78+
JinjavaBeanELResolver.JinjavaBeanELResolverConfig
79+
.builder()
80+
.withReadOnly(false)
81+
.build()
82+
)
83+
);
7984
}
8085
};
8186

‎src/main/java/com/hubspot/jinjava/el/ext/JinjavaBeanELResolver.java‎

Lines changed: 24 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import com.fasterxml.jackson.databind.ObjectMapper;
44
import com.google.common.base.CaseFormat;
55
import com.google.common.collect.ImmutableSet;
6+
import com.hubspot.jinjava.JinjavaImmutableStyle;
67
import com.hubspot.jinjava.interpret.DeferredValueException;
78
import com.hubspot.jinjava.interpret.JinjavaInterpreter;
89
import com.hubspot.jinjava.util.EagerReconstructionUtils;
@@ -66,12 +67,12 @@ public class JinjavaBeanELResolver extends BeanELResolver {
6667
.add("set")
6768
.add("merge")
6869
.build();
69-
private final JinjavaBeanELResolverConfig jinjavaBeanELResolverConfig;
7070

7171
@Value.Immutable(singleton = true)
72+
@JinjavaImmutableStyle
7273
public interface JinjavaBeanELResolverConfig {
73-
ImmutableSet<Method> allowMethods();
74-
ImmutableSet<Class<?>> allowDeclaredMethodsFromClasses();
74+
ImmutableSet<Method> allowedMethods();
75+
ImmutableSet<Class<?>> allowedDeclaredMethodsFromClasses();
7576

7677
@Value.Default
7778
default boolean readOnly() {
@@ -81,7 +82,7 @@ default boolean readOnly() {
8182
@Value.Check
8283
default void banClassesAndMethods() {
8384
if (
84-
allowMethods()
85+
allowedMethods()
8586
.stream()
8687
.anyMatch(method ->
8788
Class.class.equals(method.getDeclaringClass()) ||
@@ -95,7 +96,7 @@ default void banClassesAndMethods() {
9596
);
9697
}
9798
if (
98-
allowDeclaredMethodsFromClasses()
99+
allowedDeclaredMethodsFromClasses()
99100
.stream()
100101
.anyMatch(clazz ->
101102
Class.class.equals(clazz) ||
@@ -114,9 +115,14 @@ static JinjavaBeanELResolverConfig.Builder builder() {
114115
return new Builder();
115116
}
116117

117-
class Builder extends ImmutableJinjavaBeanELResolverConfig.Builder {}
118+
class Builder extends ImmutableJinjavaBeanELResolverConfig.Builder {
119+
120+
Builder() {}
121+
}
118122
}
119123

124+
private final MethodValidator methodValidator;
125+
120126
public JinjavaBeanELResolver() {
121127
this(JinjavaBeanELResolverConfig.builder().build());
122128
}
@@ -125,7 +131,12 @@ public JinjavaBeanELResolver() {
125131
* Creates a new read/write {@link JinjavaBeanELResolver}.
126132
*/
127133
public JinjavaBeanELResolver(JinjavaBeanELResolverConfig jinjavaBeanELResolverConfig) {
128-
this.jinjavaBeanELResolverConfig = jinjavaBeanELResolverConfig;
134+
super(jinjavaBeanELResolverConfig.readOnly());
135+
this.methodValidator =
136+
new MethodValidator(
137+
jinjavaBeanELResolverConfig.allowedMethods(),
138+
jinjavaBeanELResolverConfig.allowedDeclaredMethodsFromClasses()
139+
);
129140
}
130141

131142
@Override
@@ -221,10 +232,7 @@ protected Method findMethod(
221232
List<Method> potentialMethods = new LinkedList<>();
222233

223234
for (Method m : methods) {
224-
if (
225-
m.getName().equals(name) &&
226-
(isDeclaringClassAllowed(m.getDeclaringClass()) || methodAllowed(m))
227-
) {
235+
if (m.getName().equals(name)) {
228236
int formalParamCount = m.getParameterTypes().length;
229237
if (m.isVarArgs() && paramCount >= formalParamCount - 1) {
230238
varArgsMethod = m;
@@ -250,17 +258,13 @@ protected Method findMethod(
250258
)
251259
);
252260
}
253-
return method;
261+
return methodValidator.validateMethod(method);
254262
}
255263

256-
private boolean methodAllowed(Method m) {
257-
return jinjavaBeanELResolverConfig.allowMethods().contains(m);
258-
}
259-
260-
private boolean isDeclaringClassAllowed(Class<?> declaringClass) {
261-
return jinjavaBeanELResolverConfig
262-
.allowDeclaredMethodsFromClasses()
263-
.contains(declaringClass);
264+
@Override
265+
protected Method getReadMethod(Object base, Object property) {
266+
Method method = super.getReadMethod(base, property);
267+
return methodValidator.validateMethod(method);
264268
}
265269

266270
private static boolean checkAssignableParameterTypes(Object[] params, Method method) {

‎src/main/java/com/hubspot/jinjava/el/ext/MethodAllowlists.java‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
package com.hubspot.jinjava.el.ext;
2+
3+
import com.google.common.collect.ImmutableSet;
4+
import com.hubspot.jinjava.JinjavaImmutableStyle;
5+
import com.hubspot.jinjava.lib.filter.Filter;
6+
import com.hubspot.jinjava.lib.filter.FilterLibrary;
7+
import java.lang.reflect.Method;
8+
import java.util.Arrays;
9+
import java.util.stream.Stream;
10+
import org.immutables.value.Value;
11+
12+
public class MethodAllowlists {
13+
14+
@Value.Immutable
15+
@JinjavaImmutableStyle
16+
interface MethodAllowlist {
17+
ImmutableSet<Method> allowCustomMethods();
18+
ImmutableSet<Class<?>> allowCustomDeclaredMethodsFromClasses();
19+
ImmutableSet<AllowlistGroup> allowlistGroups();
20+
}
21+
22+
enum AllowlistGroup {
23+
JavaString {
24+
private static final Class<?>[] ARRAY = { String.class };
25+
26+
@Override
27+
Class<?>[] allowDeclaredMethodsFromClasses() {
28+
return ARRAY;
29+
}
30+
},
31+
JinjavaFilters {
32+
private static final Class<?>[] ARRAY = Stream
33+
.concat(Stream.of(Filter.class), Arrays.stream(FilterLibrary.DEFAULT_FILTERS))
34+
.toArray(Class<?>[]::new);
35+
36+
@Override
37+
Class<?>[] allowDeclaredMethodsFromClasses() {
38+
return ARRAY;
39+
}
40+
},
41+
ReadOnlyCollectionsg,
42+
WritableCollections,
43+
JinjavaFunctions;
44+
45+
Method[] allowMethods() {
46+
return new Method[0];
47+
}
48+
49+
Class<?>[] allowDeclaredMethodsFromClasses() {
50+
return new Class[0];
51+
}
52+
}
53+
}

‎src/main/java/com/hubspot/jinjava/el/ext/MethodValidator.java‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
package com.hubspot.jinjava.el.ext;
2+
3+
import com.google.common.collect.ImmutableSet;
4+
import java.lang.reflect.Method;
5+
6+
public class MethodValidator {
7+
8+
private final ImmutableSet<Method> allowedMethods;
9+
private final ImmutableSet<Class<?>> allowedDeclaredMethodsFromClasses;
10+
11+
public MethodValidator(
12+
ImmutableSet<Method> allowedMethods,
13+
ImmutableSet<Class<?>> allowedDeclaredMethodsFromClasses
14+
) {
15+
this.allowedMethods = allowedMethods;
16+
this.allowedDeclaredMethodsFromClasses = allowedDeclaredMethodsFromClasses;
17+
}
18+
19+
public Method validateMethod(Method m) {
20+
return (
21+
allowedMethods.contains(m) ||
22+
allowedDeclaredMethodsFromClasses.contains(m.getDeclaringClass())
23+
)
24+
? m
25+
: null;
26+
}
27+
}

0 commit comments

Comments
 (0)