From 9528af6e8eac6b7e2909e8d0a31fd4d6417dfef0 Mon Sep 17 00:00:00 2001 From: "Benedicto Franco Jr." Date: Tue, 3 Nov 2026 00:31:13 -0800 Subject: [PATCH 1/4] pxf-android: skip a field whose wire type does not match its kind (#160) LiteWireReader read every known field with its kind's reader, whatever wire type the tag carried. A mistyped field was then misread and the rest of the message with it: 3a050d01000000, an Int64Value whose field 1 arrives as fixed32, failed with 'invalid tag (zero)' where protowire-go and the full tier print i64_w = 0. The field is now skipped as unknown, in a message, a map entry and every well-known payload. A repeated packable field still takes either the packed or the unpacked record. --- .../protowire/pxf/android/LiteWireReader.java | 79 +++++++++++++++---- 1 file changed, 63 insertions(+), 16 deletions(-) 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 c986b89..7852a51 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 @@ -185,6 +185,18 @@ private static List readEntries( } int kind = fieldKinds.getOrDefault(fieldNum, 0); + // A field whose wire type does not match its declared kind is + // skipped as an unknown field, as protobuf-go and the full tier + // do (#160). A repeated packable field legitimately arrives + // either packed (length-delimited) or one element per record. + int expected = mapFields.contains(fieldNum) ? WireFormat.WIRETYPE_LENGTH_DELIMITED : wireTypeOf(kind); + boolean packed = repeated.contains(fieldNum) && isPackable(kind) + && wireType == WireFormat.WIRETYPE_LENGTH_DELIMITED; + if (expected >= 0 && wireType != expected && !packed) { + in.skipField(tag); + continue; + } + if (mapFields.contains(fieldNum)) { Ast.MapEntry entry = readMapEntry(in, fieldNum, meta, registry, depth, limits); List acc = mapAcc.computeIfAbsent(fieldNum, k -> new ArrayList<>()); @@ -194,7 +206,7 @@ private static List readEntries( } if (repeated.contains(fieldNum)) { - if (wireType == WireFormat.WIRETYPE_LENGTH_DELIMITED && isPackable(kind)) { + if (packed) { // Packed encoding — one length-delimited record holds N values. int len = in.readRawVarint32(); int oldLimit = in.pushLimit(len); @@ -431,8 +443,7 @@ private static Ast.TimestampVal readTimestamp(CodedInputStream in) throws IOExce int nanos = 0; while (in.getBytesUntilLimit() > 0) { int tag = in.readTag(); - int num = WireFormat.getTagFieldNumber(tag); - switch (num) { + switch (wireField(tag, TIME_WIRE)) { case 1 -> seconds = in.readInt64(); case 2 -> nanos = in.readInt32(); default -> in.skipField(tag); @@ -451,8 +462,7 @@ private static Ast.Value readDuration(CodedInputStream in) throws IOException { int nanos = 0; while (in.getBytesUntilLimit() > 0) { int tag = in.readTag(); - int num = WireFormat.getTagFieldNumber(tag); - switch (num) { + switch (wireField(tag, TIME_WIRE)) { case 1 -> seconds = in.readInt64(); case 2 -> nanos = in.readInt32(); default -> in.skipField(tag); @@ -475,7 +485,7 @@ private static String readWrapperString(CodedInputStream in) throws IOException String value = ""; while (in.getBytesUntilLimit() > 0) { int tag = in.readTag(); - if (WireFormat.getTagFieldNumber(tag) == 1) { + if (wireField(tag, LEN) == 1) { value = in.readStringRequireUtf8(); } else { in.skipField(tag); @@ -489,7 +499,7 @@ private static byte[] readWrapperBytes(CodedInputStream in) throws IOException { byte[] value = new byte[0]; while (in.getBytesUntilLimit() > 0) { int tag = in.readTag(); - if (WireFormat.getTagFieldNumber(tag) == 1) { + if (wireField(tag, LEN) == 1) { value = in.readByteArray(); } else { in.skipField(tag); @@ -503,7 +513,7 @@ private static boolean readWrapperBool(CodedInputStream in) throws IOException { boolean value = false; while (in.getBytesUntilLimit() > 0) { int tag = in.readTag(); - if (WireFormat.getTagFieldNumber(tag) == 1) { + if (wireField(tag, VARINT) == 1) { value = in.readBool(); } else { in.skipField(tag); @@ -522,7 +532,7 @@ private static long readWrapperVarint(CodedInputStream in, int kind) throws IOEx long value = 0L; while (in.getBytesUntilLimit() > 0) { int tag = in.readTag(); - if (WireFormat.getTagFieldNumber(tag) == 1) { + if (wireField(tag, VARINT) == 1) { value = switch (kind) { case K_INT32 -> in.readInt32(); case K_INT64 -> in.readInt64(); @@ -541,7 +551,7 @@ private static float readWrapperFloat(CodedInputStream in) throws IOException { float value = 0f; while (in.getBytesUntilLimit() > 0) { int tag = in.readTag(); - if (WireFormat.getTagFieldNumber(tag) == 1) value = in.readFloat(); + if (wireField(tag, I32) == 1) value = in.readFloat(); else in.skipField(tag); } return value; @@ -551,7 +561,7 @@ private static double readWrapperDouble(CodedInputStream in) throws IOException double value = 0d; while (in.getBytesUntilLimit() > 0) { int tag = in.readTag(); - if (WireFormat.getTagFieldNumber(tag) == 1) value = in.readDouble(); + if (wireField(tag, I64) == 1) value = in.readDouble(); else in.skipField(tag); } return value; @@ -568,8 +578,8 @@ private static Ast.IntVal readBigInt(CodedInputStream in) throws IOException { boolean negative = false; while (in.getBytesUntilLimit() > 0) { int tag = in.readTag(); - switch (WireFormat.getTagFieldNumber(tag)) { - case 1 -> abs = unsignedBytesToBigInteger(in.readByteArray()); + switch (wireField(tag, BIG_INT_WIRE)) { + case 1 -> abs = unsignedBytesToBigInteger(in.readByteArray()); case 2 -> negative = in.readBool(); default -> in.skipField(tag); } @@ -590,7 +600,7 @@ private static Ast.FloatVal readDecimal(CodedInputStream in, DecodeLimits limits boolean negative = false; while (in.getBytesUntilLimit() > 0) { int tag = in.readTag(); - switch (WireFormat.getTagFieldNumber(tag)) { + switch (wireField(tag, DECIMAL_WIRE)) { case 1 -> unscaled = unsignedBytesToBigInteger(in.readByteArray()); case 2 -> scale = in.readInt32(); case 3 -> negative = in.readBool(); @@ -626,7 +636,7 @@ private static Ast.FloatVal readBigFloat(CodedInputStream in, DecodeLimits limit boolean negative = false; while (in.getBytesUntilLimit() > 0) { int tag = in.readTag(); - switch (WireFormat.getTagFieldNumber(tag)) { + switch (wireField(tag, BIG_FLOAT_WIRE)) { case 1 -> mantissa = unsignedBytesToBigInteger(in.readByteArray()); case 2 -> exponent = in.readInt32(); case 3 -> prec = in.readUInt32(); @@ -659,7 +669,7 @@ private static Ast.Value readSecret(CodedInputStream in) throws IOException { String fingerprint = ""; while (in.getBytesUntilLimit() > 0) { int tag = in.readTag(); - switch (WireFormat.getTagFieldNumber(tag)) { + switch (wireField(tag, SECRET_WIRE)) { case 1 -> value = in.readStringRequireUtf8(); case 2 -> hint = in.readStringRequireUtf8(); case 3 -> fingerprint = in.readStringRequireUtf8(); @@ -746,6 +756,43 @@ private static String mapKeyString(Ast.Value v) { throw new IllegalArgumentException("unsupported map key value type: " + v.getClass().getSimpleName()); } + /** + * The wire type a field of this kind is written with, or -1 for a kind + * this reader does not handle (left for {@link #readSingle} to reject). + */ + private static int wireTypeOf(int kind) { + return switch (kind) { + case K_BOOL, K_INT32, K_INT64, K_UINT32, K_UINT64, + K_SINT32, K_SINT64, K_ENUM -> WireFormat.WIRETYPE_VARINT; + case K_FIXED64, K_SFIXED64, K_DOUBLE -> WireFormat.WIRETYPE_FIXED64; + case K_FIXED32, K_SFIXED32, K_FLOAT -> WireFormat.WIRETYPE_FIXED32; + case K_STRING, K_BYTES, K_MESSAGE -> WireFormat.WIRETYPE_LENGTH_DELIMITED; + default -> -1; + }; + } + + // The wire type of each field of a well-known payload, field 1 first. + private static final int VARINT = WireFormat.WIRETYPE_VARINT; + private static final int I64 = WireFormat.WIRETYPE_FIXED64; + private static final int I32 = WireFormat.WIRETYPE_FIXED32; + private static final int LEN = WireFormat.WIRETYPE_LENGTH_DELIMITED; + private static final int[] TIME_WIRE = {VARINT, VARINT}; // seconds, nanos + private static final int[] BIG_INT_WIRE = {LEN, VARINT}; // abs, negative + private static final int[] DECIMAL_WIRE = {LEN, VARINT, VARINT}; // unscaled, scale, negative + private static final int[] BIG_FLOAT_WIRE = {LEN, VARINT, VARINT, VARINT}; // mantissa, exponent, prec, negative + private static final int[] SECRET_WIRE = {LEN, LEN, LEN}; // value, hint, fingerprint + + /** + * The field number of a well-known payload's tag when its wire type is + * the one {@code wireTypes} declares for that field, and 0 otherwise, so a + * mistyped or unknown field falls to the caller's skip (#160). + */ + private static int wireField(int tag, int... wireTypes) { + int num = WireFormat.getTagFieldNumber(tag); + return num >= 1 && num <= wireTypes.length + && WireFormat.getTagWireType(tag) == wireTypes[num - 1] ? num : 0; + } + private static boolean isPackable(int kind) { return switch (kind) { case K_BOOL, From 9e45525449d5c5964755bfea18f7e332113ec4cc Mon Sep 17 00:00:00 2001 From: "Benedicto Franco Jr." Date: Tue, 3 Nov 2026 00:55:29 -0800 Subject: [PATCH 2/4] pxf-android: give a map entry's missing key or value its zero value A map entry that lacks its key or its value, because the encoder left it out or because the reader now skips it as mistyped (#160), came back as the string "": by_int = { "": "x" } where protobuf-go prints { 0: "x" }, and stamps = { a: "" } where it prints the epoch. The missing field is now read from a zero encoding of its kind, so an integer key is 0, a bool key false, a message value empty and a well-known value its zero literal. --- .../protowire/pxf/android/LiteWireReader.java | 43 +++++++++++++++---- 1 file changed, 35 insertions(+), 8 deletions(-) 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 7852a51..9fc892e 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 @@ -727,17 +727,16 @@ private static Ast.MapEntry readMapEntry( List entryFields = readEntries(in, entryMeta, registry, depth + 1, limits); in.popLimit(oldLimit); - String keyStr = ""; - // A string key is spelled quoted unless identifier-safe (the - // marshaller rule, Format.needsQuoting); a bool or integer key is - // spelled bare. An entry lacking its key is the string "" (quoted). - boolean keyQuoted = true; - Ast.Value valueVal = new Ast.StringVal(Position.UNKNOWN, ""); + // An entry lacking its key or its value (left out, or skipped as + // mistyped) has that field's zero value, as in protobuf-go: 0, false, + // "", or an empty message, which a well-known type spells as its own + // zero literal. + Ast.Value keyVal = null; + Ast.Value valueVal = null; for (Ast.Entry e : entryFields) { if (e instanceof Ast.Assignment a) { if ("key".equals(a.key())) { - keyStr = mapKeyString(a.value()); - keyQuoted = a.value() instanceof Ast.StringVal && Format.needsQuoting(keyStr); + keyVal = a.value(); } else if ("value".equals(a.key())) { valueVal = a.value(); } @@ -745,9 +744,37 @@ private static Ast.MapEntry readMapEntry( valueVal = new Ast.BlockVal(Position.UNKNOWN, b.entries()); } } + if (keyVal == null) { + keyVal = zeroValue(entryMeta, 1, registry, depth, limits); + } + if (valueVal == null) { + valueVal = zeroValue(entryMeta, 2, registry, depth, limits); + } + // A string key is spelled quoted unless identifier-safe (the + // marshaller rule, Format.needsQuoting); a bool or integer key is + // spelled bare. + String keyStr = mapKeyString(keyVal); + boolean keyQuoted = keyVal instanceof Ast.StringVal && Format.needsQuoting(keyStr); return new Ast.MapEntry(Position.UNKNOWN, keyStr, valueVal, List.of(), "", keyQuoted); } + /** + * The zero value of a map entry's field, read the way the wire would + * give it: a zero varint, zero fixed bytes, or an empty record. Reading + * it, rather than spelling it here, keeps an enum's zero name and a + * well-known type's zero literal in step with the reader. + */ + private static Ast.Value zeroValue( + PxfMeta entryMeta, int fieldNum, PxfRegistry registry, int depth, DecodeLimits limits) throws IOException { + int kind = entryMeta.fieldKinds().getOrDefault(fieldNum, K_STRING); + byte[] zero = switch (wireTypeOf(kind)) { + case WireFormat.WIRETYPE_FIXED32 -> new byte[4]; + case WireFormat.WIRETYPE_FIXED64 -> new byte[8]; + default -> new byte[1]; // a zero varint, or a zero length + }; + return readSingle(CodedInputStream.newInstance(zero), kind, fieldNum, entryMeta, registry, depth + 1, limits); + } + /** Coerces an Ast.Value of a primitive map-key kind back to its parser-style string form. */ private static String mapKeyString(Ast.Value v) { if (v instanceof Ast.StringVal s) return s.value(); From 7163a68fca6fd3541ac59d2dfc333a3742a5fbec Mon Sep 17 00:00:00 2001 From: "Benedicto Franco Jr." Date: Tue, 3 Nov 2026 00:57:59 -0800 Subject: [PATCH 3/4] pxf-android-codegen-it: compare the lite reader with protowire-go on mistyped fields (#160) 62 wire messages against text.proto, each with protowire-go's pb-text output for it: every wire type arriving on a field of another kind, in a message, a repeated field, a map entry and each well-known payload, plus packed and unpacked repeated records and map entries missing a key or a value. On main's reader 58 of the 63 tests fail. --- CHANGELOG.md | 4 + .../protowire/lite/it/WireTypeLiteTest.java | 125 ++++++++++++++++++ 2 files changed, 129 insertions(+) create mode 100644 pxf-android-codegen-it/src/test/java/org/protowire/lite/it/WireTypeLiteTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index bca926f..740615e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,10 @@ format changes. - **An SBE emitter cuts an overlong string on a character boundary** (#166; draft -01 § SBE Annotations, protowire#495). Both tiers cut an `(sbe.length)` value at the byte limit, which can split a multi-byte character, and every strict decoder, this port's now included, then rejects the whole message. `ABCDEFGé` in a string field of length 8 was written `41 42 43 44 45 46 47 C3`. A string is now cut at the last whole character at or before the limit and NUL-padded: `41 42 43 44 45 46 47 00`. A `bytes` field is still cut at the limit. Only bytes that strict decoders refuse change. +- **The lite wire reader skips a field whose wire type does not match its kind** (#160). `LiteWireReader` read each known field with its kind's reader, whatever wire type the tag carried, so a mistyped field was misread and the rest of the message with it. `3a050d01000000`, an `Int64Value` whose field 1 arrives as fixed32, failed with `malformed wire: Protocol message contained an invalid tag (zero)`, where protowire-go and the full tier print `i64_w = 0`. Such a field is now skipped as an unknown field, as they do, in a message, a repeated field, a map entry and every well-known payload read as a literal (Timestamp, Duration, the wrappers, `pxf.BigInt`, `Decimal`, `BigFloat`, `Secret`). A repeated scalar field still takes both its packed and its unpacked record. + +- **A lite map entry missing its key or value gets that field's zero value** (#160). An entry without its key or value, left out by the encoder or skipped as mistyped, came back as the string `""`: `by_int = { "": "x" }` where protowire-go prints `{ 0: "x" }`, and `stamps = { a: "" }` where it prints `{ a: 1970-01-01T00:00:00Z }`. An integer key is now `0`, a bool key `false`, a message value `{ }`, and a well-known value its own zero literal. + ## [1.4.0] — 2026-11-02 A minor release, because it narrows what decodes. The lite tier catches up with diff --git a/pxf-android-codegen-it/src/test/java/org/protowire/lite/it/WireTypeLiteTest.java b/pxf-android-codegen-it/src/test/java/org/protowire/lite/it/WireTypeLiteTest.java new file mode 100644 index 0000000..e52e75f --- /dev/null +++ b/pxf-android-codegen-it/src/test/java/org/protowire/lite/it/WireTypeLiteTest.java @@ -0,0 +1,125 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2026 TrendVidia, LLC. +package org.protowire.lite.it; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; +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; + +/** + * LiteWireReader skips a field whose wire type does not match its declared + * kind, as an unknown field, the way protowire-go and the full tier do + * (#160). Each row is a wire message with such a field, usually followed by + * a well-typed one to show the rest of the message is still read in step, + * and the text protowire-go's {@code check_decode --format pb-text} printed + * for it against the spec's testdata/text/text.proto (protowire-go 7465f91). + * + *

The rows cover each wire type arriving on a field of another kind + * (varint, fixed32, fixed64, length-delimited, and a group), in a message, + * in a repeated field, in a map entry, and inside every well-known payload + * the reader spells as a literal. A repeated scalar field takes both its + * packed and its unpacked record, which is not a mismatch. A map entry + * whose key or value is missing, or skipped, has that field's zero value. + */ +final class WireTypeLiteTest { + + private static final Map METAS = Map.of( + "Scalars", text.v1.ScalarsPxfMeta.INSTANCE, + "Shapes", text.v1.ShapesPxfMeta.INSTANCE, + "WellKnown", text.v1.WellKnownPxfMeta.INSTANCE); + + private static String decode(String schema, String hex) { + return Format.formatDocument(LiteWireReader.toAst( + HexFormat.of().parseHex(hex), METAS.get(schema), DecodeLimits.defaults())); + } + + @Test + void int64ValueWithAFixed32FieldIsZero() { + // The issue's case: Int64Value (i64_w, field 7) whose field 1 is + // fixed32. It was read as a varint and failed with "invalid tag (zero)". + assertEquals("i64_w = 0\n", decode("WellKnown", "3a050d01000000")); + } + + // {schema, case, wire hex, protowire-go's text} + static Object[][] goCases() { + return new Object[][] { + {"Scalars", "bool-as-fixed64", "4101000000000000002807", "f_int32 = 7\n"}, + {"Scalars", "bytes-as-fixed32", "65010000002807", "f_int32 = 7\n"}, + {"Scalars", "double-as-fixed32", "0d010000002807", "f_int32 = 7\n"}, + {"Scalars", "enum-as-len", "7201012807", "f_int32 = 7\n"}, + {"Scalars", "fixed32-as-varint", "38052807", "f_int32 = 7\n"}, + {"Scalars", "fixed64-as-varint", "30052807", "f_int32 = 7\n"}, + {"Scalars", "float-as-fixed64", "1101000000000000002807", "f_int32 = 7\n"}, + {"Scalars", "set-then-mistyped", "28072d01000000", "f_int32 = 7\n"}, + {"Scalars", "sfixed32-as-len", "7a01052807", "f_int32 = 7\n"}, + {"Scalars", "string-as-varint", "48052807", "f_int32 = 7\n"}, + {"Scalars", "varint-as-fixed32", "1d010000002807", "f_int32 = 7\n"}, + {"Scalars", "varint-as-fixed64", "1901000000000000002807", "f_int32 = 7\n"}, + {"Scalars", "varint-as-group", "2b2c2807", "f_int32 = 7\n"}, + {"Scalars", "varint-as-len", "1a02aabb2807", "f_int32 = 7\n"}, + {"Shapes", "boolmap-key-absent", "5a03120178", "by_bool = {\n false: \"x\"\n}\n"}, + {"Shapes", "boolmap-key-as-fixed32", "5a080d01000000120178", "by_bool = {\n false: \"x\"\n}\n"}, + {"Shapes", "colors-as-fixed64", "38013901000000000000003802", "colors = [\n COLOR_RED,\n COLOR_GREEN\n]\n"}, + {"Shapes", "colors-packed-and-unpacked", "3a01013802", "colors = [\n COLOR_RED,\n COLOR_GREEN\n]\n"}, + {"Shapes", "intmap-key-absent", "5203120178", "by_int = {\n 0: \"x\"\n}\n"}, + {"Shapes", "intmap-key-as-len", "52060a0161120178", "by_int = {\n 0: \"x\"\n}\n"}, + {"Shapes", "ints-as-fixed32", "200125010000002002", "ints = [\n 1,\n 2\n]\n"}, + {"Shapes", "ints-packed", "2202010220032004", "ints = [\n 1,\n 2,\n 3,\n 4\n]\n"}, + {"Shapes", "itemmap-value-absent", "62030a0161", "by_item = {\n a: {\n }\n}\n"}, + {"Shapes", "items-as-varint", "300532030a0178", "items = [\n {\n name = \"x\"\n }\n]\n"}, + {"Shapes", "map-as-varint", "48054a060a0161120178", "by_string = {\n a: \"x\"\n}\n"}, + {"Shapes", "map-key-as-varint", "4a050805120178", "by_string = {\n \"\": \"x\"\n}\n"}, + {"Shapes", "map-value-as-fixed32", "4a080a01611501000000", "by_string = {\n a: \"\"\n}\n"}, + {"Shapes", "message-as-varint", "08051a0109", ""}, + {"Shapes", "oneof-as-varint", "68051a0109", ""}, + {"Shapes", "opt-int-as-fixed32", "1d010000001a0109", ""}, + {"Shapes", "strings-as-varint", "28052a0178", "strings = [\n \"x\"\n]\n"}, + {"Shapes", "strmap-value-absent", "4a030a0161", "by_string = {\n a: \"\"\n}\n"}, + {"WellKnown", "bigfloat-exp-as-fixed64", "720c0a0105110100000000000000", "big_float = 0\n"}, + {"WellKnown", "bigfloat-prec-as-len", "72060a01051a0105", "big_float = 0\n"}, + {"WellKnown", "bigint-abs-as-varint", "62020805", "big_int = 0\n"}, + {"WellKnown", "bigint-neg-as-len", "62060a0105120101", "big_int = 5\n"}, + {"WellKnown", "bool-as-len", "2a030a0101", "bool_w = false\n"}, + {"WellKnown", "bytes-as-fixed64", "2209090100000000000000", "bytes_w = b\"\"\n"}, + {"WellKnown", "dbl-as-fixed32", "5a050d01000000", "dbl_w = 0\n"}, + {"WellKnown", "dec-neg-as-fixed32", "6a080a01051d01000000", "dec = 5\n"}, + {"WellKnown", "dec-scale-as-len", "6a060a0105120102", "dec = 5\n"}, + {"WellKnown", "dec-unscaled-as-varint", "6a020805", "dec = 0\n"}, + {"WellKnown", "decs-scale-as-len", "92010b0a016112060a0105120102", "decs = {\n a: 5\n}\n"}, + {"WellKnown", "decs-value-absent", "9201030a0161", "decs = {\n a: 0\n}\n"}, + {"WellKnown", "dur-seconds-as-len", "12030a0141", "dur = 0s\n"}, + {"WellKnown", "durs-as-varint", "8001058201020805", "durs = [\n 5s\n]\n"}, + {"WellKnown", "flt-as-fixed64", "5209090100000000000000", "flt_w = 0\n"}, + {"WellKnown", "flt-as-varint", "52020805", "flt_w = 0\n"}, + {"WellKnown", "i32-as-fixed64", "3209090100000000000000", "i32_w = 0\n"}, + {"WellKnown", "i64-as-fixed32", "3a050d01000000", "i64_w = 0\n"}, + {"WellKnown", "secret-hint-as-fixed32", "7a080a01611501000000", "secret = \"a\"\n"}, + {"WellKnown", "secret-value-as-varint", "7a020805", "secret = \"\"\n"}, + {"WellKnown", "stamps-ts-inner-mistyped", "8a010a0a016112050d01000000", "stamps = {\n a: 1970-01-01T00:00:00Z\n}\n"}, + {"WellKnown", "stamps-value-absent", "8a01030a0161", "stamps = {\n a: 1970-01-01T00:00:00Z\n}\n"}, + {"WellKnown", "stamps-value-as-fixed32", "8a01090a0161120408011001", "stamps = {\n a: 1970-01-01T00:00:01.000000001Z\n}\n"}, + {"WellKnown", "stamps-value-as-varint", "8a01050a01611005", "stamps = {\n a: 1970-01-01T00:00:00Z\n}\n"}, + {"WellKnown", "str-as-varint", "1a020805", "str_w = \"\"\n"}, + {"WellKnown", "ts-nanos-as-len", "0a050801120101", "ts = 1970-01-01T00:00:01Z\n"}, + {"WellKnown", "ts-seconds-as-fixed32", "0a050d01000000", "ts = 1970-01-01T00:00:00Z\n"}, + {"WellKnown", "u32-as-len", "42030a0105", "u32_w = 0\n"}, + {"WellKnown", "u64-as-fixed32", "4a050d01000000", "u64_w = 0\n"}, + {"WellKnown", "wkt-field-as-varint", "08051a020a00", "str_w = \"\"\n"}, + }; + } + + @ParameterizedTest(name = "{0}.{1}") + @MethodSource("goCases") + void readsAsProtowireGoDoes(String schema, String name, String hex, String goText) { + assertEquals(goText, decode(schema, hex)); + } +} From a2c74241281128080234f7e5929ad59ae90592f7 Mon Sep 17 00:00:00 2001 From: "Benedicto Franco Jr." Date: Tue, 3 Nov 2026 01:03:58 -0800 Subject: [PATCH 4/4] pxf-android: print nothing for a plain field whose wire value is zero LiteWireReader turns each record it reads into text, so a plain proto3 field written as zero on the wire printed, where protowire-go and the full tier, which read the bytes into a message first, print nothing (#180). A singular field in implicitPresenceFields() whose last value is its zero value is now dropped; -0.0, optional fields and map entries keep theirs. --- CHANGELOG.md | 2 + .../lite/it/ImplicitZeroLiteTest.java | 55 +++++++++++++++++++ .../protowire/pxf/android/LiteWireReader.java | 45 +++++++++++++-- 3 files changed, 96 insertions(+), 6 deletions(-) create mode 100644 pxf-android-codegen-it/src/test/java/org/protowire/lite/it/ImplicitZeroLiteTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 740615e..ec5d410 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,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)); }