Skip to content

Commit 5bd7929

Browse files
MoonWormMoonWorm
committed
#137: Fix how 'OutboundAccessControlForCustomClass' treats Java enums + self-ref guard
1 parent 6d289a1 commit 5bd7929

6 files changed

Lines changed: 133 additions & 29 deletions

File tree

‎examples/pom.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
</modules>
2020

2121
<properties>
22-
<jsonapi4j.version>1.8.3</jsonapi4j.version>
22+
<jsonapi4j.version>1.8.4</jsonapi4j.version>
2323

2424
<jakarta.servlet-api.version>6.1.0</jakarta.servlet-api.version>
2525

‎jsonapi4j-base/src/main/java/pro/api4/jsonapi4j/util/ReflectionUtils.java‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -264,12 +264,17 @@ private static boolean isLeafType(Class<?> clazz) {
264264
}
265265

266266
/**
267-
* Treats platform (JDK) types as opaque leaves so traversal never descends into them. JDK value
268-
* types such as {@code java.time.LocalDate}, {@code java.util.UUID} or {@code java.math.BigDecimal}
269-
* carry no domain-meaningful field paths, and the module system forbids making their internal
270-
* fields accessible (e.g. {@code java.base} does not open {@code java.time} to the unnamed module).
267+
* Determines whether a type is a platform (JDK) type that field-graph traversals should treat as an
268+
* opaque leaf rather than descending into. JDK value types such as {@code java.time.LocalDate},
269+
* {@code java.util.UUID} or {@code java.math.BigDecimal} carry no domain-meaningful field structure,
270+
* and the module system forbids making their internal fields accessible (e.g. {@code java.base} does
271+
* not open {@code java.time} to the unnamed module, which otherwise yields an
272+
* {@link java.lang.reflect.InaccessibleObjectException}).
273+
*
274+
* @param clazz target type
275+
* @return {@code true} if the type belongs to a JDK package, {@code false} otherwise (including arrays)
271276
*/
272-
private static boolean isJdkType(Class<?> clazz) {
277+
public static boolean isJdkType(Class<?> clazz) {
273278
if (clazz.isArray()) {
274279
return false;
275280
}

‎jsonapi4j-base/src/test/java/pro/api4/jsonapi4j/plugin/utils/ReflectionUtilsTests.java‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,33 @@ public void getAllFieldPaths_jdkValueTypeAttributes_treatedAsLeavesAndNotTravers
6161
);
6262
}
6363

64+
@Test
65+
public void isJdkType_jdkPackages_returnTrue() {
66+
assertThat(ReflectionUtils.isJdkType(String.class)).isTrue();
67+
assertThat(ReflectionUtils.isJdkType(java.time.LocalDate.class)).isTrue();
68+
assertThat(ReflectionUtils.isJdkType(java.util.UUID.class)).isTrue();
69+
assertThat(ReflectionUtils.isJdkType(java.math.BigDecimal.class)).isTrue();
70+
assertThat(ReflectionUtils.isJdkType(javax.security.auth.Subject.class)).isTrue();
71+
}
72+
73+
@Test
74+
public void isJdkType_customClass_returnsFalse() {
75+
assertThat(ReflectionUtils.isJdkType(Nested.class)).isFalse();
76+
}
77+
78+
@Test
79+
public void isJdkType_arrays_returnFalse() {
80+
assertThat(ReflectionUtils.isJdkType(String[].class)).isFalse();
81+
assertThat(ReflectionUtils.isJdkType(int[].class)).isFalse();
82+
assertThat(ReflectionUtils.isJdkType(Nested[].class)).isFalse();
83+
}
84+
85+
@Test
86+
public void isJdkType_nullPackage_returnsFalse() {
87+
// primitives have no package
88+
assertThat(ReflectionUtils.isJdkType(int.class)).isFalse();
89+
}
90+
6491
@Test
6592
public void setFieldPathValueThrowing_nullObject_checkResult() {
6693
// given

‎jsonapi4j-plugins/jsonapi4j-ac-plugin/src/main/java/pro/api4/jsonapi4j/plugin/ac/model/outbound/OutboundAccessControlForCustomClass.java‎

Lines changed: 44 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,9 @@
1515

1616
import java.util.Collections;
1717
import java.util.HashMap;
18+
import java.util.HashSet;
1819
import java.util.Map;
20+
import java.util.Set;
1921
import java.util.stream.Stream;
2022

2123
@EqualsAndHashCode
@@ -73,31 +75,51 @@ public static OutboundAccessControlForCustomClass fromClassAnnotationsOf(Object
7375
}
7476

7577
private static Map<String, OutboundAccessControlForCustomClass> extractNestedRecursively(Class<?> clazz) {
78+
return extractNestedRecursively(clazz, new HashSet<>());
79+
}
80+
81+
private static Map<String, OutboundAccessControlForCustomClass> extractNestedRecursively(Class<?> clazz,
82+
Set<Class<?>> visited) {
7683
Map<String, OutboundAccessControlForCustomClass> result = new HashMap<>();
7784

78-
Map<String, Class<?>> fields = ReflectionUtils.fetchFieldTypes(clazz);
79-
80-
fields.forEach((fieldName, fieldClass) -> {
81-
if (!fieldClass.getPackageName().startsWith("java.")) {
82-
AccessControlModel classLevelAccessControl
83-
= AccessControlModel.fromClassAnnotation(fieldClass);
84-
Map<String, AccessControlModel> fieldLevelAccessControl
85-
= AccessControlModel.fromFieldsAnnotations(fieldClass);
86-
Map<String, OutboundAccessControlForCustomClass> nested
87-
= extractNestedRecursively(fieldClass);
88-
if (classLevelAccessControl != null
89-
|| MapUtils.isNotEmpty(fieldLevelAccessControl)
90-
|| MapUtils.isNotEmpty(nested)) {
91-
result.put(
92-
fieldName,
93-
OutboundAccessControlForCustomClass.builder()
94-
.classLevel(classLevelAccessControl)
95-
.fieldLevel(fieldLevelAccessControl)
96-
.nested(nested).build()
97-
);
85+
// Guard against self-referential / cyclic class graphs (e.g. a tree node referencing its own
86+
// type) that would otherwise recurse until the stack overflows. Scoped to the current branch
87+
// so diamond-shaped graphs are still fully explored.
88+
if (!visited.add(clazz)) {
89+
return result;
90+
}
91+
try {
92+
Map<String, Class<?>> fields = ReflectionUtils.fetchFieldTypes(clazz);
93+
94+
fields.forEach((fieldName, fieldClass) -> {
95+
if (!ReflectionUtils.isJdkType(fieldClass)) {
96+
AccessControlModel classLevelAccessControl
97+
= AccessControlModel.fromClassAnnotation(fieldClass);
98+
Map<String, AccessControlModel> fieldLevelAccessControl
99+
= AccessControlModel.fromFieldsAnnotations(fieldClass);
100+
// Enums are value types: their compiler-generated constant fields are static and
101+
// self-referential (each constant is a field typed as the enum itself), so recursing
102+
// into them never terminates. Capture the enum's own class/field-level access control
103+
// but do not descend into its constants.
104+
Map<String, OutboundAccessControlForCustomClass> nested = fieldClass.isEnum()
105+
? Collections.emptyMap()
106+
: extractNestedRecursively(fieldClass, visited);
107+
if (classLevelAccessControl != null
108+
|| MapUtils.isNotEmpty(fieldLevelAccessControl)
109+
|| MapUtils.isNotEmpty(nested)) {
110+
result.put(
111+
fieldName,
112+
OutboundAccessControlForCustomClass.builder()
113+
.classLevel(classLevelAccessControl)
114+
.fieldLevel(fieldLevelAccessControl)
115+
.nested(nested).build()
116+
);
117+
}
98118
}
99-
}
100-
});
119+
});
120+
} finally {
121+
visited.remove(clazz);
122+
}
101123

102124
return result;
103125
}

‎jsonapi4j-plugins/jsonapi4j-ac-plugin/src/test/java/pro/api4/jsonapi4j/plugin/ac/model/outbound/OutboundAccessControlForCustomClassTests.java‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,56 @@ public void merge_twoEqual_resultIsTheSame() {
7979
assertThat(actualResult).isNotNull().isEqualTo(higherPrecedence);
8080
}
8181

82+
@Test
83+
public void fromObjectClass_enumTypedField_doesNotRecurseIntoConstants_noStackOverflow() {
84+
// given
85+
ClassWithEnumField target = new ClassWithEnumField();
86+
87+
// when - must not throw StackOverflowError from recursing into the enum's self-referential constants
88+
OutboundAccessControlForCustomClass actualResult
89+
= OutboundAccessControlForCustomClass.fromClassAnnotationsOf(target);
90+
91+
// then - the enum's own class-level access control is captured, but it is treated as a leaf
92+
assertThat(actualResult).isNotNull();
93+
assertThat(actualResult.getNested()).containsKey("status");
94+
OutboundAccessControlForCustomClass statusAc = actualResult.getNested().get("status");
95+
assertThat(statusAc.getClassLevel()).isNotNull();
96+
assertThat(statusAc.getClassLevel().getRequiredScopes().getRequiredScopes()).isEqualTo(Set.of("Status"));
97+
assertThat(statusAc.getNested()).isEmpty();
98+
}
99+
100+
@Test
101+
public void fromObjectClass_selfReferentialField_doesNotRecurseInfinitely_noStackOverflow() {
102+
// given
103+
SelfReferential target = new SelfReferential();
104+
105+
// when - a class referencing its own type must not blow the stack
106+
OutboundAccessControlForCustomClass actualResult
107+
= OutboundAccessControlForCustomClass.fromClassAnnotationsOf(target);
108+
109+
// then
110+
assertThat(actualResult).isNotNull();
111+
assertThat(actualResult.getNested().get("child")).isNotNull();
112+
}
113+
114+
@AccessControl(scopes = @AccessControlScopes(requiredScopes = "Status"))
115+
private enum Status {
116+
ACTIVE, INACTIVE
117+
}
118+
119+
@Data
120+
private static class ClassWithEnumField {
121+
@AccessControl(scopes = @AccessControlScopes(requiredScopes = "status"))
122+
private Status status;
123+
}
124+
125+
@AccessControl(scopes = @AccessControlScopes(requiredScopes = "SelfReferential"))
126+
@Data
127+
private static class SelfReferential {
128+
@AccessControl(scopes = @AccessControlScopes(requiredScopes = "child"))
129+
private SelfReferential child;
130+
}
131+
82132
@AccessControl(scopes = @AccessControlScopes(requiredScopes = "TargetClass"))
83133
@Data
84134
private static class TargetClass extends ParentClass {

‎pom.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@
5252

5353
<properties>
5454
<!--project revision-->
55-
<revision>1.8.3</revision>
55+
<revision>1.8.4</revision>
5656

5757
<!--spting/web-->
5858
<spring.version>5.3.14</spring.version>

0 commit comments

Comments
 (0)