From e9440b54b5442c3b5ef7bffa936152ebbc7b7173 Mon Sep 17 00:00:00 2001 From: Matthias Andreas Benkard Date: Sun, 10 Dec 2023 15:28:16 +0100 Subject: Add Decoder#encode roundtrip tests and fix the bugs discovered. Change-Id: I21447306d9fc7768e07fafe5bed1d92a3eb42e53 --- .../main/java/eu/mulk/jgvariant/core/Decoder.java | 52 ++++++---- .../java/eu/mulk/jgvariant/core/DecoderTest.java | 106 ++++++++++++++++++++- 2 files changed, 138 insertions(+), 20 deletions(-) diff --git a/jgvariant-core/src/main/java/eu/mulk/jgvariant/core/Decoder.java b/jgvariant-core/src/main/java/eu/mulk/jgvariant/core/Decoder.java index a28d792..f605b09 100644 --- a/jgvariant-core/src/main/java/eu/mulk/jgvariant/core/Decoder.java +++ b/jgvariant-core/src/main/java/eu/mulk/jgvariant/core/Decoder.java @@ -5,6 +5,7 @@ package eu.mulk.jgvariant.core; import static java.lang.Math.max; +import static java.nio.ByteOrder.BIG_ENDIAN; import static java.nio.ByteOrder.LITTLE_ENDIAN; import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.Objects.requireNonNullElse; @@ -440,12 +441,16 @@ public abstract class Decoder { ArrayList framingOffsets = new ArrayList<>(value.size()); int startOffset = byteWriter.position(); for (var element : value) { + // Align the element. + var lastRelativeEnd = byteWriter.position() - startOffset; + byteWriter.write(new byte[align(lastRelativeEnd, alignment()) - lastRelativeEnd]); + + // Encode the element. elementDecoder.encode(element, byteWriter); + + // Record the framing offset of the element. var relativeEnd = byteWriter.position() - startOffset; framingOffsets.add(relativeEnd); - - // Align the next element. - byteWriter.write(new byte[align(relativeEnd, alignment()) - relativeEnd]); } // Write the framing offsets. @@ -710,21 +715,30 @@ public abstract class Decoder { @Override @SuppressWarnings("unchecked") void encode(Object[] value, ByteWriter byteWriter) { + // The unit type is encoded as a single zero byte. + if (value.length == 0) { + byteWriter.write((byte) 0); + return; + } + int startOffset = byteWriter.position(); ArrayList framingOffsets = new ArrayList<>(value.length); for (int i = 0; i < value.length; ++i) { var componentDecoder = (Decoder) componentDecoders[i]; - componentDecoder.encode(value[i], byteWriter); - var relativeEnd = byteWriter.position() - startOffset; + // Align the element. + var lastRelativeEnd = byteWriter.position() - startOffset; + byteWriter.write(new byte[align(lastRelativeEnd, componentDecoder.alignment()) - lastRelativeEnd]); + + // Encode the element. + componentDecoder.encode(value[i], byteWriter); + // Record the framing offset of the element if it is of variable size. var fixedComponentSize = componentDecoders[i].fixedSize(); if (fixedComponentSize == null && i < value.length - 1) { + var relativeEnd = byteWriter.position() - startOffset; framingOffsets.add(relativeEnd); } - - // Align the next element. - byteWriter.write(new byte[align(relativeEnd, alignment()) - relativeEnd]); } // Write the framing offsets in reverse order. @@ -732,6 +746,12 @@ public abstract class Decoder { for (int i = framingOffsets.size() - 1; i >= 0; --i) { byteWriter.writeIntN(framingOffsets.get(i), framingOffsetSize); } + + // Pad the structure to its alignment if it is of fixed size. + if (fixedSize() != null) { + var lastRelativeEnd = byteWriter.position() - startOffset; + byteWriter.write(new byte[align(lastRelativeEnd, alignment()) - lastRelativeEnd]); + } } } @@ -975,7 +995,7 @@ public abstract class Decoder { @Override void encode(String value, ByteWriter byteWriter) { - byteWriter.write(charset.encode(value)); + byteWriter.write(charset.encode(value).rewind()); byteWriter.write((byte) 0); } } @@ -1044,7 +1064,7 @@ public abstract class Decoder { var innerByteWriter = new ByteWriter(); Decoder.this.encode(value, innerByteWriter); var transformedBuffer = encodingFunction.apply(innerByteWriter.toByteBuffer()); - byteWriter.write(transformedBuffer); + byteWriter.write(transformedBuffer.rewind()); } } @@ -1136,7 +1156,7 @@ public abstract class Decoder { } private static class ByteWriter { - private ByteOrder byteOrder = ByteOrder.nativeOrder(); + private ByteOrder byteOrder = BIG_ENDIAN; private final ByteArrayOutputStream outputStream; ByteWriter() { @@ -1167,19 +1187,19 @@ public abstract class Decoder { } void write(int value) { - write(ByteBuffer.allocate(4).order(byteOrder).putInt(value)); + write(ByteBuffer.allocate(4).order(byteOrder).putInt(value).rewind()); } void write(long value) { - write(ByteBuffer.allocate(8).order(byteOrder).putLong(value)); + write(ByteBuffer.allocate(8).order(byteOrder).putLong(value).rewind()); } void write(short value) { - write(ByteBuffer.allocate(2).order(byteOrder).putShort(value)); + write(ByteBuffer.allocate(2).order(byteOrder).putShort(value).rewind()); } void write(double value) { - write(ByteBuffer.allocate(8).order(byteOrder).putDouble(value)); + write(ByteBuffer.allocate(8).order(byteOrder).putDouble(value).rewind()); } private void writeIntN(int value, int byteCount) { @@ -1195,7 +1215,7 @@ public abstract class Decoder { default -> throw new IllegalArgumentException("invalid byte count: %d".formatted(byteCount)); } - write(byteBuffer); + write(byteBuffer.rewind()); } ByteWriter duplicate() { diff --git a/jgvariant-core/src/test/java/eu/mulk/jgvariant/core/DecoderTest.java b/jgvariant-core/src/test/java/eu/mulk/jgvariant/core/DecoderTest.java index 068b051..d97cf88 100644 --- a/jgvariant-core/src/test/java/eu/mulk/jgvariant/core/DecoderTest.java +++ b/jgvariant-core/src/test/java/eu/mulk/jgvariant/core/DecoderTest.java @@ -14,9 +14,7 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import java.nio.ByteBuffer; import java.text.ParseException; -import java.util.List; -import java.util.Map; -import java.util.Optional; +import java.util.*; import org.junit.jupiter.api.Test; /** @@ -24,13 +22,14 @@ import org.junit.jupiter.api.Test; * href="https://people.gnome.org/~desrt/gvariant-serialisation.pdf">~desrt/gvariant-serialisation.pdf. */ @SuppressWarnings({ + "ByteBufferBackingArray", "ImmutableListOf", "ImmutableListOf1", "ImmutableListOf2", "ImmutableListOf3", "ImmutableListOf4", "ImmutableListOf5", - "ImmutableMapOf2" + "ImmutableMapOf2", }) class DecoderTest { @@ -39,6 +38,9 @@ class DecoderTest { var data = new byte[] {0x68, 0x65, 0x6C, 0x6C, 0x6F, 0x20, 0x77, 0x6F, 0x72, 0x6C, 0x64, 0x00}; var decoder = Decoder.ofString(UTF_8); assertEquals("hello world", decoder.decode(ByteBuffer.wrap(data))); + + var roundtripData = decoder.encode("hello world"); + assertArrayEquals(data, roundtripData.array()); } @Test @@ -47,6 +49,9 @@ class DecoderTest { new byte[] {0x68, 0x65, 0x6C, 0x6C, 0x6F, 0x20, 0x77, 0x6F, 0x72, 0x6C, 0x64, 0x00, 0x00}; var decoder = Decoder.ofMaybe(Decoder.ofString(UTF_8)); assertEquals(Optional.of("hello world"), decoder.decode(ByteBuffer.wrap(data))); + + var roundtripData = decoder.encode(Optional.of("hello world")); + assertArrayEquals(data, roundtripData.array()); } @Test @@ -54,6 +59,9 @@ class DecoderTest { var data = new byte[] {0x01, 0x00, 0x00, 0x01, 0x01}; var decoder = Decoder.ofArray(Decoder.ofBoolean()); assertEquals(List.of(true, false, false, true, true), decoder.decode(ByteBuffer.wrap(data))); + + var roundtripData = decoder.encode(List.of(true, false, false, true, true)); + assertArrayEquals(data, roundtripData.array()); } @Test @@ -67,6 +75,9 @@ class DecoderTest { var decoder = Decoder.ofStructure(TestRecord.class, Decoder.ofString(UTF_8), Decoder.ofInt()); assertEquals(new TestRecord("foo", -1), decoder.decode(ByteBuffer.wrap(data))); + + var roundtripData = decoder.encode(new TestRecord("foo", -1)); + assertArrayEquals(data, roundtripData.array()); } @Test @@ -109,6 +120,10 @@ class DecoderTest { assertEquals( List.of(new TestRecord("hi", -2), new TestRecord("bye", -1)), decoder.decode(ByteBuffer.wrap(data))); + + var roundtripData = + decoder.encode(List.of(new TestRecord("hi", -2), new TestRecord("bye", -1))); + assertArrayEquals(data, roundtripData.array()); } @Test @@ -143,6 +158,13 @@ class DecoderTest { var decoder = Decoder.ofDictionary(Decoder.ofString(UTF_8), Decoder.ofInt().withByteOrder(LITTLE_ENDIAN)); assertEquals(Map.of("hi", -2, "bye", -1), decoder.decode(ByteBuffer.wrap(data))); + + var entity = new LinkedHashMap(); + entity.put("hi", -2); + entity.put("bye", -1); + var roundtripData = decoder.encode(entity); + System.out.println(HexFormat.of().formatHex(roundtripData.array())); + assertArrayEquals(data, roundtripData.array()); } @Test @@ -154,6 +176,9 @@ class DecoderTest { }; var decoder = Decoder.ofArray(Decoder.ofString(UTF_8)); assertEquals(List.of("i", "can", "has", "strings?"), decoder.decode(ByteBuffer.wrap(data))); + + var roundtripData = decoder.encode(List.of("i", "can", "has", "strings?")); + assertArrayEquals(data, roundtripData.array()); } @Test @@ -176,6 +201,11 @@ class DecoderTest { assertEquals( new TestParent(new TestChild((byte) 0x69, "can"), List.of("has", "strings?")), decoder.decode(ByteBuffer.wrap(data))); + + var roundtripData = + decoder.encode( + new TestParent(new TestChild((byte) 0x69, "can"), List.of("has", "strings?"))); + assertArrayEquals(data, roundtripData.array()); } @Test @@ -195,6 +225,9 @@ class DecoderTest { () -> assertEquals(2, result.length), () -> assertArrayEquals(new Object[] {(byte) 0x69, "can"}, (Object[]) result[0]), () -> assertEquals(List.of("has", "strings?"), result[1])); + + var roundtripData = decoder.encode(variant); + assertArrayEquals(data, roundtripData.array()); } @Test @@ -210,6 +243,9 @@ class DecoderTest { Decoder.ofByte().withByteOrder(LITTLE_ENDIAN)); assertEquals(new TestRecord((byte) 0x60, (byte) 0x70), decoder.decode(ByteBuffer.wrap(data))); + + var roundtripData = decoder.encode(new TestRecord((byte) 0x60, (byte) 0x70)); + assertArrayEquals(data, roundtripData.array()); } @Test @@ -225,6 +261,9 @@ class DecoderTest { Decoder.ofByte().withByteOrder(LITTLE_ENDIAN)); assertEquals(new TestRecord(0x60, (byte) 0x70), decoder.decode(ByteBuffer.wrap(data))); + + var roundtripData = decoder.encode(new TestRecord(0x60, (byte) 0x70)); + assertArrayEquals(data, roundtripData.array()); } @Test @@ -240,6 +279,9 @@ class DecoderTest { Decoder.ofInt().withByteOrder(LITTLE_ENDIAN)); assertEquals(new TestRecord((byte) 0x60, 0x70), decoder.decode(ByteBuffer.wrap(data))); + + var roundtripData = decoder.encode(new TestRecord((byte) 0x60, 0x70)); + assertArrayEquals(data, roundtripData.array()); } @Test @@ -276,6 +318,10 @@ class DecoderTest { assertEquals( List.of(new TestRecord(96, (byte) 0x70), new TestRecord(648, (byte) 0xf7)), decoder.decode(ByteBuffer.wrap(data))); + + var roundtripData = + decoder.encode(List.of(new TestRecord(96, (byte) 0x70), new TestRecord(648, (byte) 0xf7))); + assertArrayEquals(data, roundtripData.array()); } @Test @@ -287,6 +333,9 @@ class DecoderTest { assertEquals( List.of((byte) 0x04, (byte) 0x05, (byte) 0x06, (byte) 0x07), decoder.decode(ByteBuffer.wrap(data))); + + var roundtripData = decoder.encode(List.of((byte) 0x04, (byte) 0x05, (byte) 0x06, (byte) 0x07)); + assertArrayEquals(data, roundtripData.array()); } @Test @@ -296,6 +345,9 @@ class DecoderTest { var decoder = Decoder.ofByteArray(); assertArrayEquals(data, decoder.decode(ByteBuffer.wrap(data))); + + var roundtripData = decoder.encode(data); + assertArrayEquals(data, roundtripData.array()); } @Test @@ -307,6 +359,9 @@ class DecoderTest { var decoder = Decoder.ofStructure(TestRecord.class, Decoder.ofByteArray()); assertArrayEquals(data, decoder.decode(ByteBuffer.wrap(data)).bytes()); + + var roundtripData = decoder.encode(new TestRecord(data)); + assertArrayEquals(data, roundtripData.array()); } @Test @@ -316,6 +371,9 @@ class DecoderTest { var decoder = Decoder.ofArray(Decoder.ofInt().withByteOrder(LITTLE_ENDIAN)); assertEquals(List.of(4, 258), decoder.decode(ByteBuffer.wrap(data))); + + var roundtripData = decoder.encode(List.of(4, 258)); + assertArrayEquals(data, roundtripData.array()); } @Test @@ -327,6 +385,9 @@ class DecoderTest { Decoder.ofDictionaryEntry( Decoder.ofString(UTF_8), Decoder.ofInt().withByteOrder(LITTLE_ENDIAN)); assertEquals(Map.entry("a key", 514), decoder.decode(ByteBuffer.wrap(data))); + + var roundtripData = decoder.encode(Map.entry("a key", 514)); + assertArrayEquals(data, roundtripData.array()); } @Test @@ -340,6 +401,9 @@ class DecoderTest { Decoder.ofStructure( TestEntry.class, Decoder.ofString(UTF_8), Decoder.ofInt().withByteOrder(LITTLE_ENDIAN)); assertEquals(new TestEntry("a key", 514), decoder.decode(ByteBuffer.wrap(data))); + + var roundtripData = decoder.encode(new TestEntry("a key", 514)); + assertArrayEquals(data, roundtripData.array()); } @Test @@ -359,6 +423,10 @@ class DecoderTest { Decoder.ofLong().withByteOrder(LITTLE_ENDIAN), Decoder.ofDouble()); assertEquals(new TestRecord((short) 1, 2, 3.25), decoder.decode(ByteBuffer.wrap(data))); + + var roundtripData = decoder.encode(new TestRecord((short) 1, 2, 3.25)); + + assertArrayEquals(data, roundtripData.array()); } @Test @@ -373,6 +441,9 @@ class DecoderTest { assertEquals( new TestRecord(Optional.of((byte) 1), Optional.empty()), decoder.decode(ByteBuffer.wrap(data))); + + var roundtripData = decoder.encode(new TestRecord(Optional.of((byte) 1), Optional.empty())); + assertArrayEquals(data, roundtripData.array()); } @Test @@ -393,6 +464,9 @@ class DecoderTest { var decoder = Decoder.ofStructure(TestRecord.class); assertEquals(new TestRecord(), decoder.decode(ByteBuffer.wrap(data))); + + var roundtripData = decoder.encode(new TestRecord()); + assertArrayEquals(data, roundtripData.array()); } @Test @@ -404,6 +478,9 @@ class DecoderTest { var decoder = Decoder.ofArray(Decoder.ofStructure(TestRecord.class)); assertEquals( List.of(new TestRecord(), new TestRecord()), decoder.decode(ByteBuffer.wrap(data))); + + var roundtripData = decoder.encode(List.of(new TestRecord(), new TestRecord())); + assertArrayEquals(data, roundtripData.array()); } @Test @@ -414,6 +491,9 @@ class DecoderTest { var decoder = Decoder.ofArray(Decoder.ofStructure(TestRecord.class)); assertEquals(List.of(new TestRecord()), decoder.decode(ByteBuffer.wrap(data))); + + var roundtripData = decoder.encode(List.of(new TestRecord())); + assertArrayEquals(data, roundtripData.array()); } @Test @@ -424,6 +504,9 @@ class DecoderTest { var decoder = Decoder.ofArray(Decoder.ofStructure(TestRecord.class)); assertEquals(List.of(), decoder.decode(ByteBuffer.wrap(data))); + + var roundtripData = decoder.encode(List.of()); + assertArrayEquals(data, roundtripData.array()); } @Test @@ -434,6 +517,9 @@ class DecoderTest { var decoder = Decoder.ofArray(Decoder.ofStructure(TestRecord.class)); assertEquals(List.of(), decoder.decode(ByteBuffer.wrap(data))); + + var roundtripData = decoder.encode(List.of()); + assertArrayEquals(data, roundtripData.array()); } @Test @@ -500,6 +586,9 @@ class DecoderTest { List.of(11, 12) }, (Object[]) decoder.decode(ByteBuffer.wrap(data)).value()); + + var roundtripData = decoder.encode(decoder.decode(ByteBuffer.wrap(data))); + assertArrayEquals(data, roundtripData.array()); } @Test @@ -610,5 +699,14 @@ class DecoderTest { new TestChild((short) 5, (short) 6), new TestChild((short) 7, (short) 8)), decoder.decode(ByteBuffer.wrap(data))); + + var roundtripData = + decoder.encode( + new TestParent( + new TestChild((short) 1, (short) 2), + new TestChild((short) 3, (short) 4), + new TestChild((short) 5, (short) 6), + new TestChild((short) 7, (short) 8))); + assertArrayEquals(data, roundtripData.array()); } } -- cgit v1.2.3