Skip to content

Commit ab086b3

Browse files
jasmith-hsjasmith-hs
committed
Use BeanMethods and cache allowed methods and return types
1 parent e34f953 commit ab086b3

6 files changed

Lines changed: 171 additions & 161 deletions

File tree

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

Lines changed: 60 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -23,102 +23,105 @@
2323
import java.math.BigDecimal;
2424
import java.util.AbstractCollection;
2525
import java.util.ArrayList;
26+
import java.util.LinkedHashMap;
2627
import java.util.Map;
2728

2829
public enum AllowlistGroup {
2930
JavaPrimitives {
30-
private static final Class<?>[] ARRAY = {
31-
String.class,
32-
Long.class,
33-
Integer.class,
34-
Double.class,
35-
Byte.class,
36-
Character.class,
37-
Float.class,
38-
Boolean.class,
39-
Short.class,
40-
long.class,
41-
int.class,
42-
double.class,
43-
byte.class,
44-
char.class,
45-
float.class,
46-
boolean.class,
47-
short.class,
48-
BigDecimal.class,
31+
private static final String[] ARRAY = {
32+
String.class.getCanonicalName(),
33+
Long.class.getCanonicalName(),
34+
Integer.class.getCanonicalName(),
35+
Double.class.getCanonicalName(),
36+
Byte.class.getCanonicalName(),
37+
Character.class.getCanonicalName(),
38+
Float.class.getCanonicalName(),
39+
Boolean.class.getCanonicalName(),
40+
Short.class.getCanonicalName(),
41+
long.class.getCanonicalName(),
42+
int.class.getCanonicalName(),
43+
double.class.getCanonicalName(),
44+
byte.class.getCanonicalName(),
45+
char.class.getCanonicalName(),
46+
float.class.getCanonicalName(),
47+
boolean.class.getCanonicalName(),
48+
short.class.getCanonicalName(),
49+
BigDecimal.class.getCanonicalName(),
4950
};
5051

5152
@Override
52-
Class<?>[] allowedReturnTypeClasses() {
53+
String[] allowedReturnTypeClasses() {
5354
return ARRAY;
5455
}
5556

5657
@Override
57-
Class<?>[] allowedDeclaredMethodsFromClasses() {
58+
String[] allowedDeclaredMethodsFromClasses() {
5859
return ARRAY;
5960
}
6061
},
6162
JinjavaObjects {
62-
private static final Class<?>[] ARRAY = {
63-
PyList.class,
64-
PyMap.class,
65-
SizeLimitingPyMap.class,
66-
SizeLimitingPyList.class,
67-
SnakeCaseAccessibleMap.class,
68-
FormattedDate.class,
69-
PyishDate.class,
70-
DummyObject.class,
71-
Namespace.class,
72-
SafeString.class,
63+
private static final String[] ARRAY = {
64+
PyList.class.getCanonicalName(),
65+
PyMap.class.getCanonicalName(),
66+
SizeLimitingPyMap.class.getCanonicalName(),
67+
SizeLimitingPyList.class.getCanonicalName(),
68+
SnakeCaseAccessibleMap.class.getCanonicalName(),
69+
FormattedDate.class.getCanonicalName(),
70+
PyishDate.class.getCanonicalName(),
71+
DummyObject.class.getCanonicalName(),
72+
Namespace.class.getCanonicalName(),
73+
SafeString.class.getCanonicalName(),
7374
};
7475

7576
@Override
76-
Class<?>[] allowedReturnTypeClasses() {
77+
String[] allowedReturnTypeClasses() {
7778
return ARRAY;
7879
}
7980

8081
@Override
81-
Class<?>[] allowedDeclaredMethodsFromClasses() {
82+
String[] allowedDeclaredMethodsFromClasses() {
8283
return ARRAY;
8384
}
8485
},
8586
Collections {
86-
private static final Class<?>[] ARRAY = {
87-
Map.Entry.class,
88-
PyList.class,
89-
PyMap.class,
90-
PySet.class,
91-
SizeLimitingPyMap.class,
92-
SizeLimitingPyList.class,
93-
SizeLimitingPySet.class,
94-
ArrayList.class,
95-
ForwardingList.class,
96-
ForwardingMap.class,
97-
ForwardingSet.class,
98-
ForwardingCollection.class,
99-
AbstractCollection.class,
87+
private static final String[] ARRAY = {
88+
Map.Entry.class.getCanonicalName(),
89+
PyList.class.getCanonicalName(),
90+
PyMap.class.getCanonicalName(),
91+
PySet.class.getCanonicalName(),
92+
SizeLimitingPyMap.class.getCanonicalName(),
93+
SizeLimitingPyList.class.getCanonicalName(),
94+
SizeLimitingPySet.class.getCanonicalName(),
95+
ArrayList.class.getCanonicalName(),
96+
ForwardingList.class.getCanonicalName(),
97+
ForwardingMap.class.getCanonicalName(),
98+
ForwardingSet.class.getCanonicalName(),
99+
ForwardingCollection.class.getCanonicalName(),
100+
AbstractCollection.class.getCanonicalName(),
101+
LinkedHashMap.class.getCanonicalName(),
102+
"%s.Entry".formatted(LinkedHashMap.class.getCanonicalName()),
100103
};
101104

102105
@Override
103-
Class<?>[] allowedReturnTypeClasses() {
106+
String[] allowedReturnTypeClasses() {
104107
return ARRAY;
105108
}
106109

107110
@Override
108-
Class<?>[] allowedDeclaredMethodsFromClasses() {
111+
String[] allowedDeclaredMethodsFromClasses() {
109112
return ARRAY;
110113
}
111114
},
112115
JinjavaTagConstructs {
113-
private static final Class<?>[] ARRAY = { ForLoop.class };
116+
private static final String[] ARRAY = { ForLoop.class.getCanonicalName() };
114117

115118
@Override
116-
Class<?>[] allowedReturnTypeClasses() {
119+
String[] allowedReturnTypeClasses() {
117120
return ARRAY;
118121
}
119122

120123
@Override
121-
Class<?>[] allowedDeclaredMethodsFromClasses() {
124+
String[] allowedDeclaredMethodsFromClasses() {
122125
return ARRAY;
123126
}
124127
},
@@ -153,15 +156,15 @@ String[] allowedDeclaredMethodsFromCanonicalClassPrefixes() {
153156
return new String[0];
154157
}
155158

156-
Class<?>[] allowedDeclaredMethodsFromClasses() {
157-
return new Class[0];
159+
String[] allowedDeclaredMethodsFromClasses() {
160+
return new String[0];
158161
}
159162

160163
String[] allowedReturnTypeCanonicalClassPrefixes() {
161164
return new String[0];
162165
}
163166

164-
Class<?>[] allowedReturnTypeClasses() {
165-
return new Class[0];
167+
String[] allowedReturnTypeClasses() {
168+
return new String[0];
166169
}
167170
}

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

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,11 @@
33
import com.google.common.collect.ImmutableList;
44
import com.google.common.collect.ImmutableSet;
55
import java.lang.reflect.Method;
6+
import java.util.concurrent.ConcurrentHashMap;
67

78
public final class AllowlistMethodValidator {
89

10+
private final ConcurrentHashMap<Method, Boolean> allowedMethodsCache;
911
private final ImmutableSet<Method> allowedMethods;
1012
private final ImmutableSet<String> allowedDeclaredMethodsFromCanonicalClassPrefixes;
1113
private final ImmutableSet<String> allowedDeclaredMethodsFromCanonicalClassNames;
@@ -31,29 +33,37 @@ private AllowlistMethodValidator(
3133
this.allowedDeclaredMethodsFromCanonicalClassNames =
3234
methodValidatorConfig.allowedDeclaredMethodsFromCanonicalClassNames();
3335
this.additionalValidators = additionalValidators;
36+
this.allowedMethodsCache = new ConcurrentHashMap<>();
3437
}
3538

3639
public Method validateMethod(Method m) {
3740
if (m == null) {
3841
return null;
3942
}
40-
Class<?> clazz = m.getDeclaringClass();
41-
String canonicalClassName = clazz.getCanonicalName();
42-
if (
43-
allowedMethods.contains(m) ||
44-
allowedDeclaredMethodsFromCanonicalClassNames.contains(canonicalClassName) ||
45-
allowedDeclaredMethodsFromCanonicalClassPrefixes
46-
.stream()
47-
.anyMatch(canonicalClassName::startsWith)
48-
) {
49-
for (MethodValidator v : additionalValidators) {
50-
m = v.validateMethod(m);
51-
if (m == null) {
52-
return null;
53-
}
43+
boolean isAllowedMethod = allowedMethodsCache.computeIfAbsent(
44+
m,
45+
m1 -> {
46+
Class<?> clazz = m1.getDeclaringClass();
47+
String canonicalClassName = clazz.getCanonicalName();
48+
return (
49+
allowedMethods.contains(m1) ||
50+
allowedDeclaredMethodsFromCanonicalClassNames.contains(canonicalClassName) ||
51+
allowedDeclaredMethodsFromCanonicalClassPrefixes
52+
.stream()
53+
.anyMatch(canonicalClassName::startsWith)
54+
);
5455
}
55-
return m;
56+
);
57+
if (!isAllowedMethod) {
58+
return null;
5659
}
57-
return null;
60+
for (MethodValidator v : additionalValidators) {
61+
m = v.validateMethod(m);
62+
if (m == null) {
63+
return null;
64+
}
65+
}
66+
67+
return m;
5868
}
5969
}

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

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,12 @@
22

33
import com.google.common.collect.ImmutableList;
44
import com.google.common.collect.ImmutableSet;
5+
import java.util.concurrent.ConcurrentHashMap;
56

67
public final class AllowlistReturnTypeValidator {
78

9+
private final ConcurrentHashMap<String, Boolean> allowedReturnTypesCache;
10+
811
private final ImmutableSet<String> allowedCanonicalClassPrefixes;
912
private final ImmutableSet<String> allowedCanonicalClassNames;
1013
private final ImmutableList<ReturnTypeValidator> additionalValidators;
@@ -28,6 +31,7 @@ private AllowlistReturnTypeValidator(
2831
this.allowedCanonicalClassNames =
2932
returnTypeValidatorConfig.allowedCanonicalClassNames();
3033
this.additionalValidators = additionalValidators;
34+
this.allowedReturnTypesCache = new ConcurrentHashMap<>();
3135
}
3236

3337
public Object validateReturnType(Object o) {
@@ -36,18 +40,21 @@ public Object validateReturnType(Object o) {
3640
}
3741
Class<?> clazz = o.getClass();
3842
String canonicalClassName = clazz.getCanonicalName();
39-
if (
40-
allowedCanonicalClassNames.contains(canonicalClassName) ||
41-
allowedCanonicalClassPrefixes.stream().anyMatch(canonicalClassName::startsWith)
42-
) {
43-
for (ReturnTypeValidator v : additionalValidators) {
44-
o = v.validateReturnType(o);
45-
if (o == null) {
46-
return null;
47-
}
43+
boolean isAllowedClassName = allowedReturnTypesCache.computeIfAbsent(
44+
canonicalClassName,
45+
c ->
46+
allowedCanonicalClassNames.contains(canonicalClassName) ||
47+
allowedCanonicalClassPrefixes.stream().anyMatch(canonicalClassName::startsWith)
48+
);
49+
if (!isAllowedClassName) {
50+
return null;
51+
}
52+
for (ReturnTypeValidator v : additionalValidators) {
53+
o = v.validateReturnType(o);
54+
if (o == null) {
55+
return null;
4856
}
49-
return o;
5057
}
51-
return null;
58+
return o;
5259
}
5360
}

0 commit comments

Comments
 (0)