diff --git a/CHANGELOG.md b/CHANGELOG.md index 0fc43af..152aaf5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,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)); + } +} 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..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 @@ -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(); @@ -717,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(); } @@ -735,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(); @@ -746,6 +783,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,