diff --git a/CHANGELOG.md b/CHANGELOG.md index 152aaf5..8608a0f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,8 @@ format changes. ### Changed +- **The lite tier prints nothing for a plain field whose wire value is zero** (#180). `LiteWireReader` used to print `f_int32 = 0` for `2800`, where protowire-go and the full tier print nothing: a plain proto3 field has no set state, so a zero on the wire means the same as no field. A field in `PxfMeta.implicitPresenceFields()` whose last value is its zero value is now left out. A `-0.0`, an `optional` field and a map entry's key and value keep their zeros. A companion from an older plugin has an empty table and prints as before. + - **Both tiers write PXF text to the same PB bytes as protowire-go** (#156, #157, #158; the spec's write gate, protowire#511). Each change below used to make a valid document write different bytes from Go, or accept one Go refuses: - A `float` or `double` literal that rounds to infinity is refused, as `invalid double: 1e400` or `invalid float: 3.4028236e38`. Draft `-01` § Numeric Literals admits infinity only as `inf`, `+inf` or `-inf`. Both tiers used to write it as infinity. - A `pxf.Decimal` negative zero keeps its sign: `-0` sets `negative`, and `-0.00` sets it with scale 2. Both tiers used to drop it on the way through `BigDecimal`, which has no negative zero. The lite tier also prints `{negative: true}` as `-0` now, as the full tier does. diff --git a/pxf-android-codegen-it/src/test/java/org/protowire/lite/it/ImplicitZeroLiteTest.java b/pxf-android-codegen-it/src/test/java/org/protowire/lite/it/ImplicitZeroLiteTest.java new file mode 100644 index 0000000..c8bbc32 --- /dev/null +++ b/pxf-android-codegen-it/src/test/java/org/protowire/lite/it/ImplicitZeroLiteTest.java @@ -0,0 +1,55 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2026 TrendVidia, LLC. +package org.protowire.lite.it; + +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import org.protowire.pxf.DecodeLimits; +import org.protowire.pxf.Format; +import org.protowire.pxf.PxfMeta; +import org.protowire.pxf.android.LiteWireReader; + +import java.util.HexFormat; +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** + * A field with no presence whose value on the wire is its zero value prints + * nothing, as protowire-go and the full tier print it (#180). Each expected + * text is the output of protowire-go's check_decode --format pb-text for the + * same bytes, with \n for line breaks. + */ +final class ImplicitZeroLiteTest { + + private static final Map METAS = Map.of( + "Scalars", text.v1.ScalarsPxfMeta.INSTANCE, + "Shapes", text.v1.ShapesPxfMeta.INSTANCE); + + @ParameterizedTest(name = "{0} {1}") + @CsvSource(delimiter = '|', value = { + // A plain field holding its zero value: integer, string, bool, enum, double. + "Scalars | 2800 | ''", + "Scalars | 4a00 | ''", + "Scalars | 4000 | ''", + "Scalars | 7000 | ''", + "Scalars | 090000000000000000 | ''", + // -0.0 has a bit set, so it is kept. + "Scalars | 090000000000000080 | f_double = -0\\n", + // The last value wins: 7 then 0 is 0, which prints nothing; 0 then 7 is 7. + "Scalars | 28072800 | ''", + "Scalars | 28002807 | f_int32 = 7\\n", + // Inside a nested message and a list element. + "Shapes | 0a020a00 | one {\\n}\\n", + "Shapes | 32020a00 | items = [\\n {\\n }\\n]\\n", + // Controls: an optional field keeps its zero, and so does a map entry. + "Shapes | 1800 | opt_int = 0\\n", + "Shapes | 1200 | opt_string = \"\"\\n", + "Shapes | 52020800 | by_int = {\\n 0: \"\"\\n}\\n", + }) + void printsWhatGoPrints(String schema, String hex, String text) { + String doc = Format.formatDocument(LiteWireReader.toAst( + HexFormat.of().parseHex(hex), METAS.get(schema), DecodeLimits.defaults())); + assertEquals(text.replace("\\n", "\n"), doc); + } +} diff --git a/pxf-android/src/main/java/org/protowire/pxf/android/LiteWireReader.java b/pxf-android/src/main/java/org/protowire/pxf/android/LiteWireReader.java index 9fc892e..21b881b 100644 --- a/pxf-android/src/main/java/org/protowire/pxf/android/LiteWireReader.java +++ b/pxf-android/src/main/java/org/protowire/pxf/android/LiteWireReader.java @@ -131,7 +131,7 @@ public static Ast.Document toAst(byte[] wire, PxfMeta meta, PxfRegistry registry throw new PxfException(Position.UNKNOWN, Limits.messageSizeError(wire.length, limits.maxMessageSize())); } try { - List entries = readEntries(CodedInputStream.newInstance(wire), meta, registry, 0, limits); + List entries = readEntries(CodedInputStream.newInstance(wire), meta, registry, 0, limits, false); return Ast.Document.of("", entries); } catch (InvalidProtocolBufferException e) { // Malformed input (truncated record, bad varint, invalid UTF-8 in a @@ -148,10 +148,13 @@ public static Ast.Document toAst(byte[] wire, PxfMeta meta, PxfRegistry registry /** * Walks one message's wire bytes, accumulating per-field-number values * (singular / repeated / map), then emits the entries in field-number - * order. + * order. A singular field with no presence whose last value is its zero + * value is left out, as a message read from the same bytes holds nothing + * there (#180); a map entry ({@code mapEntry}) keeps its key and value. */ private static List readEntries( - CodedInputStream in, PxfMeta meta, PxfRegistry registry, int depth, DecodeLimits limits) throws IOException { + CodedInputStream in, PxfMeta meta, PxfRegistry registry, int depth, DecodeLimits limits, + boolean mapEntry) throws IOException { // HARDENING.md § Recursion: depth is the message's own level, the // top-level call being 0 and every nested message or map entry one // descent (as in :pb's Pb.unmarshal, protowire#301); it is passed @@ -226,7 +229,13 @@ private static List readEntries( continue; } - singles.put(fieldNum, readSingle(in, kind, fieldNum, meta, registry, depth, limits)); + Ast.Value single = readSingle(in, kind, fieldNum, meta, registry, depth, limits); + // The last value wins, so a zero after a non-zero clears the field. + if (!mapEntry && meta.implicitPresenceFields().contains(fieldNum) && isZero(single, fieldNum, meta, registry)) { + singles.remove(fieldNum); + } else { + singles.put(fieldNum, single); + } } // Emit in field-number order. Stable + matches protobuf's tag ordering. @@ -404,7 +413,7 @@ private static Ast.Value readNestedMessage( } int len = in.readRawVarint32(); int oldLimit = in.pushLimit(len); - List entries = readEntries(in, nested, registry, depth + 1, limits); + List entries = readEntries(in, nested, registry, depth + 1, limits, false); in.popLimit(oldLimit); return new Ast.BlockVal(Position.UNKNOWN, entries); } @@ -724,7 +733,7 @@ private static Ast.MapEntry readMapEntry( // Decode the entry's two fields by walking its sub-bytes via a recursive // readEntries call. The result is a list with up to two assignments // (key and/or value); proto3 default-omission may drop one or both. - List entryFields = readEntries(in, entryMeta, registry, depth + 1, limits); + List entryFields = readEntries(in, entryMeta, registry, depth + 1, limits, true); in.popLimit(oldLimit); // An entry lacking its key or its value (left out, or skipped as @@ -843,6 +852,30 @@ private static Ast.IntVal uintVal(long value) { return new Ast.IntVal(Position.UNKNOWN, Long.toUnsignedString(value)); } + /** + * Whether a value read for a field with no presence is that field's zero + * value: 0, false, "", empty bytes, enum 0 or +0.0. A -0.0 has a bit set, + * so protobuf keeps it, and so does this. Only scalars and enums reach + * here; a message field always has presence. + */ + private static boolean isZero(Ast.Value v, int fieldNum, PxfMeta meta, PxfRegistry registry) { + return switch (v) { + case Ast.IntVal i -> i.raw().equals("0"); + case Ast.BoolVal b -> !b.value(); + case Ast.StringVal str -> str.value().isEmpty(); + case Ast.BytesVal b -> b.value().length == 0; + case Ast.FloatVal f -> f.raw().equals("0"); + case Ast.IdentVal id -> { + PxfEnum em = meta.enumMetas().get(fieldNum); + String fqn = meta.enumTypes().get(fieldNum); + if (em == null && fqn != null) em = registry.lookupEnum(fqn); + Integer n = em == null ? null : em.values().get(id.name()); + yield n != null && n == 0; + } + default -> false; + }; + } + private static Ast.FloatVal floatVal(double value) { return new Ast.FloatVal(Position.UNKNOWN, Format.formatDouble(value)); }