Skip to content

Commit a95a126

Browse files
mandreanchaokunyang
mandrean
and
chaokunyang
authored
fix(java): ignore non-Scala/Lombok-style default helper methods (#3733)
## Why? When Scala is present on the runtime classpath, Java compatible serializers enable Scala default-value support globally. Plain Java classes generated with Lombok `@Builder.Default` can contain private helper methods named like `$default$imageRelations()`, which previously matched the broad Scala default-method scan and caused warning noise during deserialization. ## What does this PR do? - Restricts Scala default-value method detection to known Scala constructor/apply helper prefixes with numeric parameter suffixes. - Ignores non-Scala/Lombok-style `$default$` helper names and out-of-range parameter indexes without invoking them. - Keeps Scala case-class and regular-class constructor defaults working, including nested/default-value regression coverage. - Adds Java fixtures and Scala serializer tests covering Lombok-like helper methods. ## Related issues Closes #3720 ## AI Contribution Checklist - [ ] Substantial AI assistance was used in this PR: `yes` - [ ] If `yes`, I included a completed [AI Contribution Checklist](https://github.com/apache/fory/blob/main/AI_POLICY.md#9-contributor-checklist-for-ai-assisted-prs) in this PR description and the required `AI Usage Disclosure`. - [ ] If `yes`, my PR description includes the required `ai_review` summary and screenshot evidence of the final clean AI review results from both fresh reviewers on the current PR diff or current HEAD after the latest code changes. ## Does this PR introduce any user-facing change? - [ ] Does this PR introduce any public API change? - [ ] Does this PR introduce any binary protocol compatibility change? ## Benchmark N/A. This is a targeted default-method filtering fix and does not change serialization format or hot-path serialization logic. ## Validation - `mvn -T16 -pl fory-core -am install -DskipTests -Dmaven.javadoc.skip=true -Dmaven.source.skip=true` - `mvn -pl fory-core -Dtest=org.apache.fory.platform.AndroidSupportStaticCheckTest#testScalaDefaultValuesDoNotUseTrustedLookupOnAndroid test` - `sbt "+testOnly org.apache.fory.util.ScalaDefaultValueUtilsTest org.apache.fory.serializer.scala.ScalaDefaultValueTest"` - `sbt +test` - `mvn -T16 spotless:check checkstyle:check` Co-authored-by: Shawn Yang <shawn.ck.yang@gmail.com>
1 parent 075421f commit a95a126

7 files changed

Lines changed: 375 additions & 78 deletions

File tree

‎java/fory-core/src/main/java/org/apache/fory/util/DefaultValueUtils.java‎

Lines changed: 117 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,9 @@
5050
@Internal
5151
public class DefaultValueUtils {
5252
private static final Logger LOG = LoggerFactory.getLogger(DefaultValueUtils.class);
53+
private static final String SCALA_APPLY_DEFAULT_METHOD_PREFIX = "apply$default$";
54+
private static final String SCALA_CONSTRUCTOR_DEFAULT_METHOD_PREFIX =
55+
"$lessinit$greater$default$";
5356

5457
private static final ClassValueCache<Map<Integer, Object>> cachedCtrDefaultValues =
5558
ClassValueCache.newClassKeySoftCache(32);
@@ -226,8 +229,8 @@ public Map<String, Object> getAllDefaultValues(Class<?> cls) {
226229
}
227230
Preconditions.checkNotNull(
228231
primaryConstructor, "Primary constructor not found for class " + cls.getName());
229-
Map<Integer, Object> defaultValues = getDefaultValuesForClass(cls);
230232
int paramCount = primaryConstructor.getParameterCount();
233+
Map<Integer, Object> defaultValues = getDefaultValuesForClass(cls, paramCount);
231234
for (int i = 0; i < paramCount; i++) {
232235
String paramName = primaryConstructor.getParameters()[i].getName();
233236
Object defaultValue = defaultValues.get(i + 1); // +1 because default values are 1-indexed
@@ -245,75 +248,42 @@ public Map<String, Object> getAllDefaultValues(Class<?> cls) {
245248
* @param cls the Scala class
246249
* @return a map from parameter index to method handle
247250
*/
248-
private Map<Integer, Object> getDefaultValuesForClass(Class<?> cls) {
251+
private Map<Integer, Object> getDefaultValuesForClass(Class<?> cls, int paramCount) {
249252
if (cachedCtrDefaultValues.getIfPresent(cls) != null) {
250253
return cachedCtrDefaultValues.getIfPresent(cls);
251254
}
252255
Map<Integer, Object> defaultValueMethods;
253256
if (ScalaTypes.isScalaProductType(cls)) {
254-
defaultValueMethods = getDefaultValuesForCaseClass(cls);
257+
defaultValueMethods = getDefaultValuesForCaseClass(cls, paramCount);
255258
} else {
256-
defaultValueMethods = getDefaultValuesForRegularScalaClass(cls);
259+
defaultValueMethods = getDefaultValuesForRegularScalaClass(cls, paramCount);
257260
}
258261
cachedCtrDefaultValues.put(cls, defaultValueMethods);
259262
return defaultValueMethods;
260263
}
261264

262-
private static Map<Integer, Object> getDefaultValuesForCaseClass(Class<?> cls) {
265+
private static Map<Integer, Object> getDefaultValuesForCaseClass(Class<?> cls, int paramCount) {
263266
Map<Integer, Object> values = new HashMap<>();
264-
String companionClassName = cls.getName() + "$";
265-
Class<?> companionClass = null;
266-
Object companionInstance = null;
267-
try {
268-
companionClass = Class.forName(companionClassName, false, cls.getClassLoader());
269-
companionInstance = companionClass.getField("MODULE$").get(null);
270-
} catch (Exception e) {
271-
// For nested case classes, try to find the companion object in the enclosing class
272-
Class<?> enclosingClass = cls.getEnclosingClass();
273-
if (enclosingClass != null) {
274-
// Look for a companion object field in the enclosing class
275-
for (java.lang.reflect.Field field : enclosingClass.getDeclaredFields()) {
276-
if (field.getType().getName().equals(companionClassName)) {
277-
field.setAccessible(true);
278-
try {
279-
companionInstance = field.get(null);
280-
} catch (Exception e1) {
281-
LOG.warn(
282-
"Error {} accessing companion object for {}, default values support is disabled when deserializing object of type {}",
283-
e1.getMessage(),
284-
cls.getName(),
285-
cls.getName());
286-
return values;
287-
}
288-
if (companionInstance != null) {
289-
companionClass = companionInstance.getClass();
290-
break;
291-
}
292-
}
293-
}
294-
}
295-
}
296-
if (companionClass == null) {
297-
LOG.warn(
298-
"Companion class not found for {}, default values support is disabled when deserializing object of type {}",
299-
cls.getName(),
300-
cls.getName());
267+
ScalaCompanionObject companionObject = getScalaCompanionObject(cls, true);
268+
if (companionObject == null) {
301269
return values;
302270
}
303271
MethodHandles.Lookup lookup =
304-
AndroidSupport.IS_ANDROID ? null : _JDKAccess._trustedLookup(companionClass);
272+
AndroidSupport.IS_ANDROID ? null : _JDKAccess._trustedLookup(companionObject.cls);
305273

306-
// Look for methods named `apply$default$1`, `apply$default$2`, etc.
307-
Method[] companionMethods = companionClass.getDeclaredMethods();
274+
Method[] companionMethods = companionObject.cls.getDeclaredMethods();
308275
for (Method method : companionMethods) {
309-
String methodName = method.getName();
310-
if (methodName.contains("$default$")) {
276+
int paramIndex =
277+
getScalaDefaultParameterIndex(method, SCALA_APPLY_DEFAULT_METHOD_PREFIX, paramCount);
278+
if (paramIndex < 0) {
279+
paramIndex =
280+
getScalaDefaultParameterIndex(
281+
method, SCALA_CONSTRUCTOR_DEFAULT_METHOD_PREFIX, paramCount);
282+
}
283+
if (paramIndex > 0) {
311284
try {
312-
// Extract the parameter index from the method name
313-
String indexStr =
314-
methodName.substring(methodName.lastIndexOf("$default$") + "$default$".length());
315-
int paramIndex = Integer.parseInt(indexStr);
316-
Object defaultValue = invokeDefaultValueMethod(lookup, method, companionInstance);
285+
Object defaultValue =
286+
invokeDefaultValueMethod(lookup, method, companionObject.instance);
317287
values.put(paramIndex, defaultValue);
318288
} catch (Throwable e) {
319289
LOG.warn(
@@ -328,23 +298,25 @@ private static Map<Integer, Object> getDefaultValuesForCaseClass(Class<?> cls) {
328298
return values;
329299
}
330300

331-
private static Map<Integer, Object> getDefaultValuesForRegularScalaClass(Class<?> cls) {
301+
private static Map<Integer, Object> getDefaultValuesForRegularScalaClass(
302+
Class<?> cls, int paramCount) {
332303
Map<Integer, Object> values = new HashMap<>();
304+
ScalaCompanionObject companionObject = getScalaCompanionObject(cls, false);
305+
if (companionObject == null) {
306+
return values;
307+
}
333308
try {
334309
MethodHandles.Lookup lookup =
335-
AndroidSupport.IS_ANDROID ? null : _JDKAccess._trustedLookup(cls);
336-
Method[] classMethods = cls.getDeclaredMethods();
337-
for (Method method : classMethods) {
338-
String methodName = method.getName();
339-
if (methodName.contains("$default$")) {
310+
AndroidSupport.IS_ANDROID ? null : _JDKAccess._trustedLookup(companionObject.cls);
311+
Method[] companionMethods = companionObject.cls.getDeclaredMethods();
312+
for (Method method : companionMethods) {
313+
int paramIndex =
314+
getScalaDefaultParameterIndex(
315+
method, SCALA_CONSTRUCTOR_DEFAULT_METHOD_PREFIX, paramCount);
316+
if (paramIndex > 0) {
340317
try {
341-
// Extract the parameter index from the method name
342-
String indexStr =
343-
methodName.substring(methodName.lastIndexOf("$default$") + "$default$".length());
344-
int paramIndex = Integer.parseInt(indexStr);
345-
// For regular Scala classes, we need to create an instance to call instance methods
346-
// Since these are default value methods, we can try to call them as static methods
347-
Object defaultValue = invokeDefaultValueMethod(lookup, method, null);
318+
Object defaultValue =
319+
invokeDefaultValueMethod(lookup, method, companionObject.instance);
348320
values.put(paramIndex, defaultValue);
349321
} catch (Throwable e) {
350322
LOG.warn(
@@ -367,6 +339,48 @@ private static Map<Integer, Object> getDefaultValuesForRegularScalaClass(Class<?
367339
return values;
368340
}
369341

342+
private static ScalaCompanionObject getScalaCompanionObject(
343+
Class<?> cls, boolean warnIfMissing) {
344+
String companionClassName = cls.getName() + "$";
345+
try {
346+
Class<?> companionClass = Class.forName(companionClassName, false, cls.getClassLoader());
347+
Object companionInstance = companionClass.getField("MODULE$").get(null);
348+
if (companionInstance != null) {
349+
return new ScalaCompanionObject(companionClass, companionInstance);
350+
}
351+
} catch (Exception e) {
352+
// For nested case classes, try to find the companion object in the enclosing class.
353+
Class<?> enclosingClass = cls.getEnclosingClass();
354+
if (enclosingClass != null) {
355+
for (java.lang.reflect.Field field : enclosingClass.getDeclaredFields()) {
356+
if (field.getType().getName().equals(companionClassName)) {
357+
field.setAccessible(true);
358+
try {
359+
Object companionInstance = field.get(null);
360+
if (companionInstance != null) {
361+
return new ScalaCompanionObject(companionInstance.getClass(), companionInstance);
362+
}
363+
} catch (Exception e1) {
364+
LOG.warn(
365+
"Error {} accessing companion object for {}, default values support is disabled when deserializing object of type {}",
366+
e1.getMessage(),
367+
cls.getName(),
368+
cls.getName());
369+
return null;
370+
}
371+
}
372+
}
373+
}
374+
}
375+
if (warnIfMissing) {
376+
LOG.warn(
377+
"Companion class not found for {}, default values support is disabled when deserializing object of type {}",
378+
cls.getName(),
379+
cls.getName());
380+
}
381+
return null;
382+
}
383+
370384
private static Object invokeDefaultValueMethod(
371385
MethodHandles.Lookup lookup, Method method, Object target) throws Throwable {
372386
if (AndroidSupport.IS_ANDROID) {
@@ -383,6 +397,44 @@ private static Object invokeDefaultValueMethod(
383397
}
384398
return methodHandle.invoke(target);
385399
}
400+
401+
private static int getScalaDefaultParameterIndex(
402+
Method method, String methodPrefix, int maxParamIndex) {
403+
if (method.getParameterCount() != 0) {
404+
return -1;
405+
}
406+
String methodName = method.getName();
407+
if (!methodName.startsWith(methodPrefix)) {
408+
return -1;
409+
}
410+
int indexStart = methodPrefix.length();
411+
if (indexStart == methodName.length()) {
412+
return -1;
413+
}
414+
int paramIndex = 0;
415+
for (int i = indexStart; i < methodName.length(); i++) {
416+
char c = methodName.charAt(i);
417+
if (c < '0' || c > '9') {
418+
return -1;
419+
}
420+
int digit = c - '0';
421+
if (paramIndex > (Integer.MAX_VALUE - digit) / 10) {
422+
return -1;
423+
}
424+
paramIndex = paramIndex * 10 + digit;
425+
}
426+
return paramIndex > 0 && paramIndex <= maxParamIndex ? paramIndex : -1;
427+
}
428+
429+
private static final class ScalaCompanionObject {
430+
private final Class<?> cls;
431+
private final Object instance;
432+
433+
private ScalaCompanionObject(Class<?> cls, Object instance) {
434+
this.cls = cls;
435+
this.instance = instance;
436+
}
437+
}
386438
}
387439

388440
/** Convert value to correct type based on dispatchId. */

‎java/fory-core/src/test/java/org/apache/fory/platform/AndroidSupportStaticCheckTest.java‎

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import java.nio.file.Paths;
2929
import java.util.ArrayList;
3030
import java.util.List;
31+
import java.util.regex.Matcher;
3132
import java.util.regex.Pattern;
3233
import java.util.stream.Stream;
3334
import org.testng.annotations.Test;
@@ -42,15 +43,12 @@ public class AndroidSupportStaticCheckTest {
4243
private static final Pattern ANDROID_GATED_FIELD_GET_ANNOTATED_TYPE =
4344
Pattern.compile(
4445
"AndroidSupport\\.IS_ANDROID[\\s\\S]{0,160}\\.getAnnotatedType\\s*\\(", Pattern.DOTALL);
45-
private static final Pattern ANDROID_GATED_SCALA_COMPANION_LOOKUP =
46+
private static final Pattern SCALA_TRUSTED_LOOKUP =
47+
Pattern.compile("_JDKAccess\\._trustedLookup\\(");
48+
private static final Pattern ANDROID_GATED_SCALA_TRUSTED_LOOKUP =
4649
Pattern.compile(
4750
"AndroidSupport\\.IS_ANDROID\\s*\\?\\s*null\\s*:\\s*"
48-
+ "_JDKAccess\\._trustedLookup\\(companionClass\\)",
49-
Pattern.DOTALL);
50-
private static final Pattern ANDROID_GATED_SCALA_CLASS_LOOKUP =
51-
Pattern.compile(
52-
"AndroidSupport\\.IS_ANDROID\\s*\\?\\s*null\\s*:\\s*"
53-
+ "_JDKAccess\\._trustedLookup\\(cls\\)",
51+
+ "_JDKAccess\\._trustedLookup\\([^)]*\\)",
5452
Pattern.DOTALL);
5553
private static final Pattern ANDROID_REFLECTIVE_SCALA_DEFAULT_INVOKE =
5654
Pattern.compile(
@@ -155,12 +153,19 @@ public void testAndroidLoadedRuntimePathsDoNotReferenceAnnotatedType() throws IO
155153
public void testScalaDefaultValuesDoNotUseTrustedLookupOnAndroid() throws IOException {
156154
Path sourcePath = Paths.get("src/main/java/org/apache/fory/util/DefaultValueUtils.java");
157155
String source = new String(Files.readAllBytes(sourcePath), StandardCharsets.UTF_8);
156+
List<String> violations = new ArrayList<>();
157+
Matcher matcher = SCALA_TRUSTED_LOOKUP.matcher(source);
158+
while (matcher.find()) {
159+
int start = Math.max(0, matcher.start() - 120);
160+
int end = Math.min(source.length(), matcher.end() + 160);
161+
String context = source.substring(start, end);
162+
if (!ANDROID_GATED_SCALA_TRUSTED_LOOKUP.matcher(context).find()) {
163+
violations.add(context);
164+
}
165+
}
158166
assertTrue(
159-
ANDROID_GATED_SCALA_COMPANION_LOOKUP.matcher(source).find(),
160-
"Scala companion default values must not use _JDKAccess trusted lookup on Android");
161-
assertTrue(
162-
ANDROID_GATED_SCALA_CLASS_LOOKUP.matcher(source).find(),
163-
"Regular Scala default values must not use _JDKAccess trusted lookup on Android");
167+
violations.isEmpty(),
168+
"Scala default values must not use _JDKAccess trusted lookup on Android: " + violations);
164169
assertTrue(
165170
ANDROID_REFLECTIVE_SCALA_DEFAULT_INVOKE.matcher(source).find(),
166171
"Android Scala default values must invoke default methods through direct reflection");

‎scala/src/test/java/org/apache/fory/serializer/scala/LombokDefaultLikeJavaPojo.java‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.apache.fory.serializer.scala;
21+
22+
import java.util.Collections;
23+
import java.util.List;
24+
import java.util.Objects;
25+
26+
public class LombokDefaultLikeJavaPojo {
27+
private String id;
28+
private List<String> imageRelations;
29+
30+
public LombokDefaultLikeJavaPojo() {
31+
this(null, Collections.emptyList());
32+
}
33+
34+
public LombokDefaultLikeJavaPojo(String id, List<String> imageRelations) {
35+
this.id = id;
36+
this.imageRelations = imageRelations;
37+
}
38+
39+
public String getId() {
40+
return id;
41+
}
42+
43+
public List<String> getImageRelations() {
44+
return imageRelations;
45+
}
46+
47+
private static List<String> $default$imageRelations() {
48+
return Collections.emptyList();
49+
}
50+
51+
private static List<String> $default$1() {
52+
return Collections.emptyList();
53+
}
54+
55+
private List<String> $default$2() {
56+
return Collections.emptyList();
57+
}
58+
59+
private static List<String> $default$99() {
60+
return Collections.emptyList();
61+
}
62+
63+
@Override
64+
public boolean equals(Object o) {
65+
if (this == o) {
66+
return true;
67+
}
68+
if (!(o instanceof LombokDefaultLikeJavaPojo)) {
69+
return false;
70+
}
71+
LombokDefaultLikeJavaPojo that = (LombokDefaultLikeJavaPojo) o;
72+
return Objects.equals(id, that.id) && Objects.equals(imageRelations, that.imageRelations);
73+
}
74+
75+
@Override
76+
public int hashCode() {
77+
return Objects.hash(id, imageRelations);
78+
}
79+
}

0 commit comments

Comments
 (0)