From bcc01d7a0f2d8b14b5a6018491613369c18d0a3b Mon Sep 17 00:00:00 2001 From: Shawn Yang Date: Thu, 12 Sep 2024 14:33:23 +0800 Subject: [PATCH] fix(java): fix long type name meta string encoding (#1837) ## What does this PR do? fix long type name meta string encoding ## Related issues Closes #1835 ## 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 --- docs/specification/java_serialization_spec.md | 6 ++-- .../org/apache/fury/meta/ClassDefDecoder.java | 26 +++++++--------- .../org/apache/fury/meta/ClassDefEncoder.java | 19 ++++++------ .../apache/fury/meta/ClassDefEncoderTest.java | 31 ++++++++++++++++++- .../serializer/MetaSharedCompatibleTest.java | 16 ++++++++++ 5 files changed, 71 insertions(+), 27 deletions(-) diff --git a/docs/specification/java_serialization_spec.md b/docs/specification/java_serialization_spec.md index a23572590a..2d48256531 100644 --- a/docs/specification/java_serialization_spec.md +++ b/docs/specification/java_serialization_spec.md @@ -155,11 +155,11 @@ Meta header is a 64 bits number value encoded in little endian order. fields info in meta for deserializing compatible fields. - Package name encoding(omitted when class is registered): - encoding algorithm: `UTF8/ALL_TO_LOWER_SPECIAL/LOWER_UPPER_DIGIT_SPECIAL` - - Header: `6 bits size | 2 bits encoding flags`. The `6 bits size: 0~63` will be used to indicate size `0~62`, - the value `63` the size need more byte to read, the encoding will encode `size - 62` as a varint next. + - Header: `6 bits size | 2 bits encoding flags`. The `6 bits size: 0~63` will be used to indicate size `0~63`, + the value `63` the size need more byte to read, the encoding will encode `size - 63` as a varint next. - Class name encoding(omitted when class is registered): - encoding algorithm: `UTF8/LOWER_UPPER_DIGIT_SPECIAL/FIRST_TO_LOWER_SPECIAL/ALL_TO_LOWER_SPECIAL` - - header: `6 bits size | 2 bits encoding flags`. The `6 bits size: 0~63` will be used to indicate size `1~64`, + - header: `6 bits size | 2 bits encoding flags`. The `6 bits size: 0~63` will be used to indicate size `0~63`, the value `63` the size need more byte to read, the encoding will encode `size - 63` as a varint next. - Field info: - header(8 diff --git a/java/fury-core/src/main/java/org/apache/fury/meta/ClassDefDecoder.java b/java/fury-core/src/main/java/org/apache/fury/meta/ClassDefDecoder.java index cde7a50cd3..c5d90d94d4 100644 --- a/java/fury-core/src/main/java/org/apache/fury/meta/ClassDefDecoder.java +++ b/java/fury-core/src/main/java/org/apache/fury/meta/ClassDefDecoder.java @@ -21,6 +21,7 @@ import static org.apache.fury.meta.ClassDef.COMPRESSION_FLAG; import static org.apache.fury.meta.ClassDef.SIZE_TWO_BYTES_FLAG; +import static org.apache.fury.meta.ClassDefEncoder.BIG_NAME_THRESHOLD; import static org.apache.fury.meta.Encoders.fieldNameEncodings; import static org.apache.fury.meta.Encoders.pkgEncodings; import static org.apache.fury.meta.Encoders.typeNameEncodings; @@ -128,13 +129,10 @@ private static String readPkgName(MemoryBuffer buffer) { // - Package name encoding(omitted when class is registered): // - encoding algorithm: `UTF8/ALL_TO_LOWER_SPECIAL/LOWER_UPPER_DIGIT_SPECIAL` // - Header: `6 bits size | 2 bits encoding flags`. - // The `6 bits size: 0~63` will be used to indicate size `0~62`, - // the value `63` the size need more byte to read, the encoding will encode `size - 62` as + // The `6 bits size: 0~63` will be used to indicate size `0~63`, + // the value `63` the size need more byte to read, the encoding will encode `size - 63` as // a varint next. - int header = buffer.readByte() & 0xff; - int encodingFlags = header & 0b11; - Encoding encoding = pkgEncodings[encodingFlags]; - return readName(Encoders.PACKAGE_DECODER, buffer, header, encoding, 62); + return readName(Encoders.PACKAGE_DECODER, buffer, pkgEncodings); } private static String readTypeName(MemoryBuffer buffer) { @@ -142,20 +140,20 @@ private static String readTypeName(MemoryBuffer buffer) { // - encoding algorithm: // `UTF8/LOWER_UPPER_DIGIT_SPECIAL/FIRST_TO_LOWER_SPECIAL/ALL_TO_LOWER_SPECIAL` // - header: `6 bits size | 2 bits encoding flags`. - // The `6 bits size: 0~63` will be used to indicate size `1~64`, + // The `6 bits size: 0~63` will be used to indicate size `0~63`, // the value `63` the size need more byte to read, the encoding will encode `size - 63` as // a varint next. - int header = buffer.readByte() & 0xff; - int encodingFlags = header & 0b11; - Encoding encoding = typeNameEncodings[encodingFlags]; - return readName(Encoders.TYPE_NAME_DECODER, buffer, header, encoding, 63); + return readName(Encoders.TYPE_NAME_DECODER, buffer, typeNameEncodings); } private static String readName( - MetaStringDecoder decoder, MemoryBuffer buffer, int header, Encoding encoding, int max) { + MetaStringDecoder decoder, MemoryBuffer buffer, Encoding[] encodings) { + int header = buffer.readByte() & 0xff; + int encodingFlags = header & 0b11; + Encoding encoding = encodings[encodingFlags]; int size = header >> 2; - if (size == max) { - size = buffer.readVarUint32Small7() + max; + if (size == BIG_NAME_THRESHOLD) { + size = buffer.readVarUint32Small7() + BIG_NAME_THRESHOLD; } return decoder.decode(buffer.readBytes(size), encoding); } diff --git a/java/fury-core/src/main/java/org/apache/fury/meta/ClassDefEncoder.java b/java/fury-core/src/main/java/org/apache/fury/meta/ClassDefEncoder.java index 46e2becf07..c1ac5ebdce 100644 --- a/java/fury-core/src/main/java/org/apache/fury/meta/ClassDefEncoder.java +++ b/java/fury-core/src/main/java/org/apache/fury/meta/ClassDefEncoder.java @@ -277,8 +277,7 @@ private static void writePkgName(MemoryBuffer buffer, String pkg) { // a varint next. MetaString pkgMetaString = Encoders.encodePackage(pkg); byte[] encoded = pkgMetaString.getBytes(); - int pkgHeader = (encoded.length << 2) | pkgEncodingsList.indexOf(pkgMetaString.getEncoding()); - writeName(buffer, encoded, pkgHeader, 62); + writeName(buffer, encoded, pkgEncodingsList.indexOf(pkgMetaString.getEncoding())); } private static void writeTypeName(MemoryBuffer buffer, String typeName) { @@ -291,17 +290,19 @@ private static void writeTypeName(MemoryBuffer buffer, String typeName) { // a varint next. MetaString metaString = Encoders.encodeTypeName(typeName); byte[] encoded = metaString.getBytes(); - int header = (encoded.length << 2) | typeNameEncodingsList.indexOf(metaString.getEncoding()); - writeName(buffer, encoded, header, 63); + writeName(buffer, encoded, typeNameEncodingsList.indexOf(metaString.getEncoding())); } - private static void writeName(MemoryBuffer buffer, byte[] encoded, int header, int max) { - boolean bigSize = encoded.length > max; + static final int BIG_NAME_THRESHOLD = 0b111111; + + private static void writeName(MemoryBuffer buffer, byte[] encoded, int encoding) { + boolean bigSize = encoded.length >= BIG_NAME_THRESHOLD; if (bigSize) { - header |= 0b11111100; - buffer.writeVarUint32Small7(header); - buffer.writeVarUint32Small7(encoded.length - max); + int header = (BIG_NAME_THRESHOLD << 2) | encoding; + buffer.writeByte(header); + buffer.writeVarUint32Small7(encoded.length - BIG_NAME_THRESHOLD); } else { + int header = (encoded.length << 2) | encoding; buffer.writeByte(header); } buffer.writeBytes(encoded); diff --git a/java/fury-core/src/test/java/org/apache/fury/meta/ClassDefEncoderTest.java b/java/fury-core/src/test/java/org/apache/fury/meta/ClassDefEncoderTest.java index 45d3075fc6..6565afdf6b 100644 --- a/java/fury-core/src/test/java/org/apache/fury/meta/ClassDefEncoderTest.java +++ b/java/fury-core/src/test/java/org/apache/fury/meta/ClassDefEncoderTest.java @@ -22,6 +22,7 @@ import static org.apache.fury.meta.ClassDefEncoder.buildFieldsInfo; import static org.apache.fury.meta.ClassDefEncoder.getClassFields; +import java.io.Serializable; import java.util.List; import lombok.Data; import org.apache.fury.Fury; @@ -78,7 +79,35 @@ public void testEmptySubClassSerializer() { ClassDef classDef1 = ClassDef.readClassDef( fury.getClassResolver(), MemoryBuffer.fromByteArray(classDef.getEncoded())); - Assert.assertEquals(classDef, classDef1); } + + @Test + public void testBigClassNameObject() { + Fury fury = Fury.builder().withMetaShare(true).build(); + ClassDef classDef = + ClassDef.buildClassDef( + fury, + TestClassLengthTestClassLengthTestClassLengthTestClassLengthTestClassLengthTestClassLengthTestClassLength + .InnerClassTestLengthInnerClassTestLengthInnerClassTestLength.class); + ClassDef classDef1 = + ClassDef.readClassDef( + fury.getClassResolver(), MemoryBuffer.fromByteArray(classDef.getEncoded())); + Assert.assertEquals(classDef1, classDef); + } + + @Data + public static + class TestClassLengthTestClassLengthTestClassLengthTestClassLengthTestClassLengthTestClassLengthTestClassLength + implements Serializable { + private String name; + private InnerClassTestLengthInnerClassTestLengthInnerClassTestLength innerClassTestLength; + + @Data + public static class InnerClassTestLengthInnerClassTestLengthInnerClassTestLength + implements Serializable { + private static final long serialVersionUID = -867612757789099089L; + private Long itemId; + } + } } diff --git a/java/fury-core/src/test/java/org/apache/fury/serializer/MetaSharedCompatibleTest.java b/java/fury-core/src/test/java/org/apache/fury/serializer/MetaSharedCompatibleTest.java index e9b7d082e9..8a4eb6834b 100644 --- a/java/fury-core/src/test/java/org/apache/fury/serializer/MetaSharedCompatibleTest.java +++ b/java/fury-core/src/test/java/org/apache/fury/serializer/MetaSharedCompatibleTest.java @@ -34,6 +34,7 @@ import org.apache.fury.config.CompatibleMode; import org.apache.fury.config.FuryBuilder; import org.apache.fury.config.Language; +import org.apache.fury.meta.ClassDefEncoderTest; import org.apache.fury.reflect.ReflectionUtils; import org.apache.fury.resolver.MetaContext; import org.apache.fury.serializer.collection.UnmodifiableSerializersTest; @@ -657,4 +658,19 @@ void testEmptySubClass(boolean referenceTracking, boolean compressNumber, boolea Assert.assertEquals(o.getClass(), o1.getClass()); Assert.assertTrue(ReflectionUtils.objectFieldsEquals(o, o1)); } + + @Test + public void testBigClassNameObject() { + Fury fury = + builder() + .withRefTracking(true) + .withCompatibleMode(CompatibleMode.COMPATIBLE) + .withScopedMetaShare(false) + .build(); + Object o = + new ClassDefEncoderTest + .TestClassLengthTestClassLengthTestClassLengthTestClassLengthTestClassLengthTestClassLengthTestClassLength + .InnerClassTestLengthInnerClassTestLengthInnerClassTestLength(); + serDeCheck(fury, o); + } }