diff --git a/java/ql/integration-tests/java/query-suite/java-code-quality-extended.qls.expected b/java/ql/integration-tests/java/query-suite/java-code-quality-extended.qls.expected index 5c82bd5e3498..00b8a91fb576 100644 --- a/java/ql/integration-tests/java/query-suite/java-code-quality-extended.qls.expected +++ b/java/ql/integration-tests/java/query-suite/java-code-quality-extended.qls.expected @@ -77,6 +77,7 @@ ql/java/ql/src/Violations of Best Practice/Naming Conventions/ConfusingMethodNam ql/java/ql/src/Violations of Best Practice/Naming Conventions/ConfusingOverloading.ql ql/java/ql/src/Violations of Best Practice/Naming Conventions/LocalShadowsFieldConfusing.ql ql/java/ql/src/Violations of Best Practice/Naming Conventions/SameNameAsSuper.ql +ql/java/ql/src/Violations of Best Practice/Records/UselessMembersOfTheRecordsClass.ql ql/java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToStringToString.ql ql/java/ql/src/Violations of Best Practice/Undesirable Calls/DefaultToString.ql ql/java/ql/src/Violations of Best Practice/Undesirable Calls/DoNotCallFinalize.ql diff --git a/java/ql/integration-tests/java/query-suite/java-code-quality.qls.expected b/java/ql/integration-tests/java/query-suite/java-code-quality.qls.expected index e558cf3ffc48..df6ec8fab813 100644 --- a/java/ql/integration-tests/java/query-suite/java-code-quality.qls.expected +++ b/java/ql/integration-tests/java/query-suite/java-code-quality.qls.expected @@ -75,6 +75,7 @@ ql/java/ql/src/Violations of Best Practice/Naming Conventions/ConfusingMethodNam ql/java/ql/src/Violations of Best Practice/Naming Conventions/ConfusingOverloading.ql ql/java/ql/src/Violations of Best Practice/Naming Conventions/LocalShadowsFieldConfusing.ql ql/java/ql/src/Violations of Best Practice/Naming Conventions/SameNameAsSuper.ql +ql/java/ql/src/Violations of Best Practice/Records/UselessMembersOfTheRecordsClass.ql ql/java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToStringToString.ql ql/java/ql/src/Violations of Best Practice/Undesirable Calls/DefaultToString.ql ql/java/ql/src/Violations of Best Practice/Undesirable Calls/DoNotCallFinalize.ql diff --git a/java/ql/src/Violations of Best Practice/Records/UselessMembersOfTheRecordsClass.md b/java/ql/src/Violations of Best Practice/Records/UselessMembersOfTheRecordsClass.md new file mode 100644 index 000000000000..27a1d25cecc9 --- /dev/null +++ b/java/ql/src/Violations of Best Practice/Records/UselessMembersOfTheRecordsClass.md @@ -0,0 +1,54 @@ +## Overview + +Record types were introduced in Java 16 as a mechanism to provide simpler data handling that is an alternative to regular classes. Record classes behave slightly differently during serialization however, namely any `writeObject`, `readObject`, `readObjectNoData`, `writeExternal`, and `readExternal` methods and `serialPersistentFields` fields declared in these classes cannot be used to affect the serialization process of any `Record` data type. + +## Recommendation + +Some level of serialization customization is offered by the Java 16 Record feature; the `writeReplace` and `readResolve` methods in a record that implements `java.io.Serializable` can be used to replace the object to be serialized. Otherwise no further customization of serialization of records is possible, and it is better to consider using a regular class implementing `java.io.Serializable` or `java.io.Externalizable` when customization is needed. + +## Example + +```java +record T1() implements Serializable { + + @Serial + private static final ObjectStreamField[] serialPersistentFields = new ObjectStreamField[0]; // NON_COMPLIANT + + @Serial + private void writeObject(ObjectOutputStream out) throws IOException {} // NON_COMPLIANT + + @Serial + private void readObject(ObjectOutputStream out) throws IOException {}// NON_COMPLIANT + + @Serial + private void readObjectNoData(ObjectOutputStream out) throws IOException { // NON_COMPLIANT + } +} + +record T2() implements Externalizable { + + @Override + public void writeExternal(ObjectOutput out) throws IOException { // NON_COMPLIANT + } + + @Override + public void readExternal(ObjectInput in) throws IOException, ClassNotFoundException { // NON_COMPLIANT + } +} + +record T3() implements Serializable { + + public Object writeReplace(ObjectOutput out) throws ObjectStreamException { // COMPLIANT + return new Object(); + } + + public Object readResolve(ObjectInput in) throws ObjectStreamException { // COMPLIANT + return new Object(); + } +} +``` + +## References + +- Oracle Serialization Documentation: [Serialization of Records](https://docs.oracle.com/en/java/javase/16/docs/specs/serialization/serial-arch.html#serialization-of-records) +- Java Record: [Feature Specification](https://openjdk.org/jeps/395) diff --git a/java/ql/src/Violations of Best Practice/Records/UselessMembersOfTheRecordsClass.ql b/java/ql/src/Violations of Best Practice/Records/UselessMembersOfTheRecordsClass.ql new file mode 100644 index 000000000000..a55b283d6e70 --- /dev/null +++ b/java/ql/src/Violations of Best Practice/Records/UselessMembersOfTheRecordsClass.ql @@ -0,0 +1,24 @@ +/** + * @id java/useless-member-of-the-record-class + * @name Useless serialization member of record class + * @description Using certain members of a record class during serialization will result in + * those members being ignored. + * @previous-id java/useless-members-of-the-records-class + * @kind problem + * @precision very-high + * @problem.severity warning + * @tags quality + * reliability + * correctness + */ + +import java + +from Record record, Member m +where + record.getAMember() = m and + m.hasName([ + "writeObject", "readObject", "readObjectNoData", "writeExternal", "readExternal", + "serialPersistentFields" + ]) +select m, "Useless serialization member found in record class $@.", record, record.getName() diff --git a/java/ql/test/query-tests/UselessMembersOfTheRecordsClass/Test.java b/java/ql/test/query-tests/UselessMembersOfTheRecordsClass/Test.java new file mode 100644 index 000000000000..bab1780de0ec --- /dev/null +++ b/java/ql/test/query-tests/UselessMembersOfTheRecordsClass/Test.java @@ -0,0 +1,42 @@ +import java.io.*; + +public class Test { + record T1() implements Serializable { + + @Serial + private static final ObjectStreamField[] serialPersistentFields = new ObjectStreamField[0]; // $ Alert + + @Serial + private void writeObject(ObjectOutputStream out) throws IOException {} // $ Alert + + @Serial + private void readObject(ObjectOutputStream out) throws IOException {} // $ Alert + + @Serial + private void readObjectNoData(ObjectOutputStream out) throws IOException { // $ Alert + } + +} + + record T2() implements Externalizable { + + @Override + public void writeExternal(ObjectOutput out) throws IOException { // $ Alert + } + + @Override + public void readExternal(ObjectInput in) throws IOException, ClassNotFoundException { // $ Alert + } + + } + + record T3() implements Serializable { + + public Object writeReplace(ObjectOutput out) throws ObjectStreamException { // COMPLIANT + return new Object(); + } + + public Object readResolve(ObjectInput in) throws ObjectStreamException { // COMPLIANT + return new Object(); + } +}} diff --git a/java/ql/test/query-tests/UselessMembersOfTheRecordsClass/UselessMembersOfTheRecordsClass.expected b/java/ql/test/query-tests/UselessMembersOfTheRecordsClass/UselessMembersOfTheRecordsClass.expected new file mode 100644 index 000000000000..27195b3f5080 --- /dev/null +++ b/java/ql/test/query-tests/UselessMembersOfTheRecordsClass/UselessMembersOfTheRecordsClass.expected @@ -0,0 +1,6 @@ +| Test.java:7:46:7:67 | serialPersistentFields | Useless serialization member found in record class $@. | Test.java:4:12:4:13 | T1 | T1 | +| Test.java:10:18:10:28 | writeObject | Useless serialization member found in record class $@. | Test.java:4:12:4:13 | T1 | T1 | +| Test.java:13:18:13:27 | readObject | Useless serialization member found in record class $@. | Test.java:4:12:4:13 | T1 | T1 | +| Test.java:16:18:16:33 | readObjectNoData | Useless serialization member found in record class $@. | Test.java:4:12:4:13 | T1 | T1 | +| Test.java:24:17:24:29 | writeExternal | Useless serialization member found in record class $@. | Test.java:21:12:21:13 | T2 | T2 | +| Test.java:28:17:28:28 | readExternal | Useless serialization member found in record class $@. | Test.java:21:12:21:13 | T2 | T2 | diff --git a/java/ql/test/query-tests/UselessMembersOfTheRecordsClass/UselessMembersOfTheRecordsClass.qlref b/java/ql/test/query-tests/UselessMembersOfTheRecordsClass/UselessMembersOfTheRecordsClass.qlref new file mode 100644 index 000000000000..5b4ab63ba4bd --- /dev/null +++ b/java/ql/test/query-tests/UselessMembersOfTheRecordsClass/UselessMembersOfTheRecordsClass.qlref @@ -0,0 +1,2 @@ +query: Violations of Best Practice/Records/UselessMembersOfTheRecordsClass.ql +postprocess: utils/test/InlineExpectationsTestQuery.ql diff --git a/java/ql/test/query-tests/UselessMembersOfTheRecordsClass/options b/java/ql/test/query-tests/UselessMembersOfTheRecordsClass/options new file mode 100644 index 000000000000..5465e6337b0e --- /dev/null +++ b/java/ql/test/query-tests/UselessMembersOfTheRecordsClass/options @@ -0,0 +1 @@ +//semmle-extractor-options: --javac-args -source 16 -target 16 \ No newline at end of file