Skip to content

Commit f72754a

Browse files
jasmith-hsclaude
andcommitted
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 <noreply@anthropic.com>
1 parent e3d1669 commit f72754a

5 files changed

Lines changed: 13 additions & 6 deletions

File tree

src/main/java/com/hubspot/jinjava/el/ExpressionResolver.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,6 @@ public Object wrap(Object object) {
352352

353353
public String getAsString(Object object) {
354354
if (interpreter.getConfig().getLegacyOverrides().isUsePyishObjectMapper()) {
355-
// resolver.
356355
return PyishObjectMapper.getAsUnquotedPyishString(object);
357356
}
358357
return Objects.toString(object, "");

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,9 @@ public Method validateMethod(Method m) {
5151
m1 -> {
5252
Class<?> clazz = m1.getDeclaringClass();
5353
String canonicalClassName = clazz.getCanonicalName();
54+
if (canonicalClassName == null) {
55+
return false;
56+
}
5457
return (
5558
allowedMethods.contains(m1) ||
5659
allowedDeclaredMethodsFromCanonicalClassNames.contains(canonicalClassName) ||

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,10 @@ public Object validateReturnType(Object o) {
5252
return o;
5353
}
5454
String canonicalClassName = clazz.getCanonicalName();
55+
if (canonicalClassName == null) {
56+
onRejectedClass.accept(clazz);
57+
return null;
58+
}
5559
boolean isAllowedClassName = allowedReturnTypesCache.computeIfAbsent(
5660
canonicalClassName,
5761
c ->

src/main/java/com/hubspot/jinjava/el/ext/BannedAllowlistOptions.java

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@
77
import java.util.List;
88
import java.util.Set;
99
import java.util.stream.Stream;
10-
import org.immutables.value.Value;
11-
1210
public class BannedAllowlistOptions {
1311

1412
// These aren't required, but they prevent someone from misconfiguring Jinjava to allow sandbox bypass unintentionally
@@ -42,7 +40,6 @@ public class BannedAllowlistOptions {
4240
)
4341
.collect(ImmutableSet.toImmutableSet());
4442

45-
@Value.Check
4643
public static List<String> findBannedPrefixes(Stream<String> prefixes) {
4744
return prefixes
4845
.filter(prefixOrName ->

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ protected static final class BeanMethod {
8484

8585
private final MethodDescriptor descriptor;
8686

87-
private Method method;
87+
private volatile Method method;
8888

8989
public BeanMethod(MethodDescriptor descriptor) {
9090
this.descriptor = descriptor;
@@ -200,7 +200,11 @@ protected Method findMethod(
200200

201201
List<Method> potentialMethods = new LinkedList<>();
202202

203-
for (BeanMethods.BeanMethod bm : beanMethods.getBeanMethods(name)) {
203+
List<BeanMethods.BeanMethod> methodsForName = beanMethods.getBeanMethods(name);
204+
if (methodsForName == null) {
205+
methodsForName = List.of();
206+
}
207+
for (BeanMethods.BeanMethod bm : methodsForName) {
204208
Method m = bm.getMethod();
205209
int formalParamCount = m.getParameterTypes().length;
206210
if (m.isVarArgs() && paramCount >= formalParamCount - 1) {

0 commit comments

Comments
 (0)