Skip to content

fix: honor serialize false in JSONCompiled writers #7661

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
hutiefang76 wants to merge 10 commits into from
Closed

fix: honor serialize false in JSONCompiled writers #7661

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
0cc63ee
fix: honor serialize false in JSONCompiled writers
hutiefang76 Jun 21, 2026
0a2d3b7
fix: honor JSONField on JSONCompiled accessors
hutiefang76 Jun 23, 2026
92e9e28
fix: preserve JSONCompiled annotation disables
hutiefang76 Jun 23, 2026
81ad5e6
fix: remove redundant JSONField method rename branch
hutiefang76 Jun 23, 2026
68cedd3
test: cover JSONCompiled setter deserialize false
hutiefang76 Jun 23, 2026
2b5b826
test: locate javac outside JDK8 jre home
hutiefang76 Jun 23, 2026
4dac8fd
test: guard JSONCompiled javac module args on JDK8
hutiefang76 Jun 30, 2026
4e9ef3f
fix: keep JSONCompiled writer ignores with FieldBased
hutiefang76 Jun 30, 2026
050a2a7
fix: preserve JSONCompiled record writer order
hutiefang76 Jul 1, 2026
a756388
test: locate Windows javac executable in codegen tests
hutiefang76 Jul 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 31 additions & 5 deletions codegen/src/main/java/com/alibaba/fastjson2/internal/processor/Analysis.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,19 +127,27 @@ private void findRelatedReferences() {
continue;
}

FieldInfo fieldInfo = new FieldInfo();
FieldInfo readerFieldInfo = new FieldInfo();
FieldInfo writerFieldInfo = new FieldInfo();

String name = field.getSimpleName().toString();
JSONField[] annotations = field.getAnnotationsByType(JSONField.class);

@wenshao wenshao Jun 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] Follow-up: the method-processing loop (lines 149-189) also never reads @JSONField from getter/setter methods. If a user places @JSONField(serialize = false) on a getter (common JavaBeans pattern for private fields), the generated writer will still serialize the field — the same class of bug, on a different annotation target.

Consider adding method.getAnnotationsByType(JSONField.class) processing in the method loop to mirror the field-level fix:

JSONField[] methodAnnotations = method.getAnnotationsByType(JSONField.class);
for (JSONField annotation : methodAnnotations) {
    FieldInfo readerFI = new FieldInfo();
    FieldInfo writerFI = new FieldInfo();
    CodeGenUtils.getFieldInfo(readerFI, annotation, false);
    CodeGenUtils.getFieldInfo(writerFI, annotation, true);
    if (readerFI.ignore) attr.reader = false;
    if (writerFI.ignore) attr.writer = false;
}

— qwen3.7-max via Qwen Code /review

for (JSONField annotation : annotations) {
CodeGenUtils.getFieldInfo(fieldInfo, annotation, false);
CodeGenUtils.getFieldInfo(readerFieldInfo, annotation, false);
CodeGenUtils.getFieldInfo(writerFieldInfo, annotation, true);

@wenshao wenshao Jun 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] serialize=false is bypassed when deserializeFeatures includes FieldBased

The new call getFieldInfo(writerFieldInfo, annotation, true) reuses CodeGenUtils.getFieldInfo, which contains reader-specific reset logic (lines 542–547) that sets fieldInfo.ignore = false when JSONReader.Feature.FieldBased is present — regardless of the serialize parameter. This silently overrides the ignore=true that serialize=false just set.

Verified empirically: @JSONField(serialize = false, deserializeFeatures = {JSONReader.Feature.FieldBased}) leaks the field into JSON output. This is a realistic combination — FieldBased for private-field deserialization + serialize=false to prevent leakage.

Suggested change
CodeGenUtils.getFieldInfo(writerFieldInfo, annotation, true);

Fix should be in CodeGenUtils.getFieldInfo — gate the FieldBased override on !serialize:

if (!serialize && fieldInfo.ignore && feature == JSONReader.Feature.FieldBased) {
    fieldInfo.ignore = false;
}

— glm-5.2 via Qwen Code /review

}

if (fieldInfo.fieldName != null) {
name = fieldInfo.fieldName;
if (readerFieldInfo.fieldName != null) {
name = readerFieldInfo.fieldName;
}

info.getAttributeByField(name, field);
AttributeInfo attr = info.getAttributeByField(name, field);
if (readerFieldInfo.ignore) {
attr.reader = false;
}
if (writerFieldInfo.ignore) {
attr.writer = false;
}
}

for (ExecutableElement method : ElementFilter.methodsIn(inheritance.getEnclosedElements())) {
Expand Down Expand Up @@ -185,7 +193,25 @@ private void findRelatedReferences() {
} else {
continue;
}

FieldInfo readerFieldInfo = new FieldInfo();

@wenshao wenshao Jun 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] Duplicated annotation-processing logic

The reader/writer FieldInfo construction, annotation iteration, name override, and ignore-to-flag mapping is duplicated verbatim between the field loop (lines 130–150) and this method loop (lines 193–211). The copies have already drifted (the redundant guard above exists in only one). Consider extracting a helper to keep them in sync.

— glm-5.2 via Qwen Code /review

FieldInfo writerFieldInfo = new FieldInfo();
JSONField[] annotations = method.getAnnotationsByType(JSONField.class);
for (JSONField annotation : annotations) {
CodeGenUtils.getFieldInfo(readerFieldInfo, annotation, false);
CodeGenUtils.getFieldInfo(writerFieldInfo, annotation, true);
}
if (readerFieldInfo.fieldName != null) {
name = readerFieldInfo.fieldName;
}

AttributeInfo attr = info.getAttributeByMethod(name, type, getter, setter);

@wenshao wenshao Jun 23, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] Dead code: the else if (writerFieldInfo.fieldName != null) branch is unreachable. CodeGenUtils.getFieldInfo sets fieldName from JSONField.name() regardless of the serialize parameter, so readerFieldInfo.fieldName and writerFieldInfo.fieldName are always identical. When readerFieldInfo.fieldName is null, writerFieldInfo.fieldName is guaranteed null too. The field loop (line 140) correctly omits this redundant check, making the two code paths inconsistent.

Suggested change
AttributeInfo attr = info.getAttributeByMethod(name, type, getter, setter);
if (readerFieldInfo.fieldName != null) {
name = readerFieldInfo.fieldName;
}

— qwen3.7-max via Qwen Code /review

if (readerFieldInfo.ignore) {

@wenshao wenshao Jul 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] Split-attribute bypass: @JSONField(name, serialize=false) can leak supposedly-hidden fields

When a field has @JSONField(name = "alias", serialize = false) and a getter exists for the same Java property, the field loop creates an AttributeInfo under "alias" with writer=false, but this method loop creates a separate AttributeInfo under the Java property name (e.g., "internalCode") with writer=true (default). getWriterAttributes() then includes the second entry, leaking the supposedly-hidden value into serialized output.

The same vulnerability exists for deserialize=false: a field with @JSONField(name = "alias", deserialize = false) plus a setter creates two entries, and the generated reader accepts the value under the Java property name.

Consider propagating the field-level disable to the method-resolved attribute when the names diverge, or unifying disable tracking by Java property name rather than JSON name.

— qwen3.7-max via Qwen Code /review

attr.reader = false;
}
if (writerFieldInfo.ignore) {
attr.writer = false;
}
}
}
}
Expand Down
8 changes: 7 additions & 1 deletion codegen/src/main/java/com/alibaba/fastjson2/internal/processor/AttributeInfo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ public class AttributeInfo
ExecutableElement setMethod;
VariableElement argument;
long readerFeatures;
boolean reader = true;
boolean writer = true;

public AttributeInfo(
String name,
Expand All @@ -35,7 +37,11 @@ public AttributeInfo(
}

public boolean supportSet() {
return field != null || setMethod != null;
return reader && (field != null || setMethod != null);

@wenshao wenshao Jul 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] Record canonical constructor arity mismatch with @JSONField(deserialize=false)

For records, field != null (set by the field loop for record components), so the new reader guard changes supportSet() behavior. When @JSONField(deserialize = false) is on a record component, reader becomes false, the component is excluded from getReaderAttributes(), and genCreateRecordInstance / genReadRecordObject generate a constructor call with fewer arguments than the canonical constructor requires.

Example: record Person(@JSONField(deserialize=false) String secret, String name) would generate new Person((String) null) instead of new Person(null, null) — a compile error.

Fix: for records, the constructor call sites need ALL components regardless of the reader filter. Components with reader=false should contribute a default value but not be read from JSON.

— qwen3.7-max via Qwen Code /review

}

public boolean supportGet() {
return writer && (field != null || getMethod != null);
}

@Override
Expand Down
2 changes: 1 addition & 1 deletion codegen/src/main/java/com/alibaba/fastjson2/internal/processor/CodeGenUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,7 @@ public static void getFieldInfo(FieldInfo fieldInfo, JSONField jsonField, boolea

for (JSONReader.Feature feature : jsonField.deserializeFeatures()) {
fieldInfo.features |= feature.mask;
if (fieldInfo.ignore && feature == JSONReader.Feature.FieldBased) {
if (!serialize && fieldInfo.ignore && feature == JSONReader.Feature.FieldBased) {
fieldInfo.ignore = false;
}
}
Expand Down
40 changes: 23 additions & 17 deletions ...c/main/java/com/alibaba/fastjson2/internal/processor/JSONCompiledAnnotationProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,13 @@ public boolean process(Set<? extends TypeElement> annotations, RoundEnvironment
Set<ReflectionMetadata> reflects = new HashSet<>(elementsAnnotatedWith.size() * 2);
elementsAnnotatedWith.forEach(element -> {
StructInfo structInfo = structs.get(element.toString());
java.util.List<AttributeInfo> attributeInfos = structInfo.getReaderAttributes();
int fieldsSize = attributeInfos.size();
java.util.List<AttributeInfo> readerAttributeInfos = structInfo.getReaderAttributes();
java.util.List<AttributeInfo> writerAttributeInfos = structInfo.getWriterAttributes();
int readerFieldsSize = readerAttributeInfos.size();
int writerFieldsSize = writerAttributeInfos.size();
boolean record = structInfo.record;
Class readSuperClass = getReadSuperClass(fieldsSize);
Class writeSuperClass = getWriteSuperClass(fieldsSize);
Class readSuperClass = getReadSuperClass(readerFieldsSize);
Class writeSuperClass = getWriteSuperClass(writerFieldsSize);

JCTree tree = javacTrees.getTree(element);
pos(tree.pos);
Expand Down Expand Up @@ -129,31 +131,32 @@ public void visitClassDef(JCTree.JCClassDecl beanClassDecl) {
JCTree.JCClassDecl innerReadClass = genInnerClass(innerReadClassName, readSuperClass);

// generate fields if necessary
final boolean generatedFields = fieldsSize < 128;
if (generatedFields) {
innerReadClass.defs = innerReadClass.defs.prependList(genFields(attributeInfos, readSuperClass));
final boolean generatedReadFields = readerFieldsSize < 128;
if (generatedReadFields) {
innerReadClass.defs = innerReadClass.defs.prependList(genFields(readerAttributeInfos, readSuperClass));
}

// generate constructor
innerReadClass.defs = innerReadClass.defs.append(genReadConstructor(beanType, beanNew, attributeInfos, readSuperClass, generatedFields, record));
innerReadClass.defs = innerReadClass.defs.append(genReadConstructor(
beanType, beanNew, readerAttributeInfos, readSuperClass, generatedReadFields, record));

// generate createInstance
innerReadClass.defs = innerReadClass.defs.append(
record
? genCreateRecordInstance(objectType, beanType, attributeInfos)
? genCreateRecordInstance(objectType, beanType, readerAttributeInfos)
: genCreateInstance(objectType, beanNew));

// generate readObject
innerReadClass.defs = innerReadClass.defs.append(
record
? genReadRecordObject(objectType, beanType, attributeInfos, structInfo, false)
: genReadObject(objectType, beanType, beanNew, attributeInfos, structInfo, false));
? genReadRecordObject(objectType, beanType, readerAttributeInfos, structInfo, false)
: genReadObject(objectType, beanType, beanNew, readerAttributeInfos, structInfo, false));

// generate readJSONBObject
innerReadClass.defs = innerReadClass.defs.append(
record
? genReadRecordObject(objectType, beanType, attributeInfos, structInfo, true)
: genReadObject(objectType, beanType, beanNew, attributeInfos, structInfo, true));
? genReadRecordObject(objectType, beanType, readerAttributeInfos, structInfo, true)
: genReadObject(objectType, beanType, beanNew, readerAttributeInfos, structInfo, true));

// link with inner class
beanClassDecl.defs = beanClassDecl.defs.append(innerReadClass);
Expand All @@ -169,15 +172,18 @@ public void visitClassDef(JCTree.JCClassDecl beanClassDecl) {
JCTree.JCClassDecl innerWriteClass = genInnerClass(innerWriteClassName, writeSuperClass);

// generate fields if necessary
if (generatedFields) {
innerWriteClass.defs = innerWriteClass.defs.prependList(genFields(attributeInfos, writeSuperClass));
final boolean generatedWriteFields = writerFieldsSize < 128;
if (generatedWriteFields) {
innerWriteClass.defs = innerWriteClass.defs.prependList(genFields(writerAttributeInfos, writeSuperClass));
}

// generate constructor
innerWriteClass.defs = innerWriteClass.defs.append(genWriteConstructor(beanType, beanNew, attributeInfos, writeSuperClass, generatedFields));
innerWriteClass.defs = innerWriteClass.defs.append(genWriteConstructor(
beanType, beanNew, writerAttributeInfos, writeSuperClass, generatedWriteFields));

// generate write
innerWriteClass.defs = innerWriteClass.defs.append(genWrite(objectType, beanType, beanNew, attributeInfos, structInfo, false));
innerWriteClass.defs = innerWriteClass.defs.append(
genWrite(objectType, beanType, beanNew, writerAttributeInfos, structInfo, false));

// link with inner class
beanClassDecl.defs = beanClassDecl.defs.append(innerWriteClass);
Expand Down
15 changes: 15 additions & 0 deletions codegen/src/main/java/com/alibaba/fastjson2/internal/processor/StructInfo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,4 +137,19 @@ public List<AttributeInfo> getReaderAttributes() {
.sorted()
.collect(Collectors.toList());
}

public List<AttributeInfo> getWriterAttributes() {
if (record) {
return attributes.values()
.stream()
.filter(AttributeInfo::supportGet)
.collect(Collectors.toList());
}

return attributes.values()
.stream()
.filter(AttributeInfo::supportGet)
.sorted()
.collect(Collectors.toList());
}
}
Loading
Loading