From 398068687ba2623648261632154f0ea33120faaa Mon Sep 17 00:00:00 2001 From: "Benedicto Franco Jr." Date: Mon, 2 Nov 2026 23:38:50 -0800 Subject: [PATCH] sbe: import a char in a non-UTF-8 encoding as bytes Convert.xmlToProto mapped every char to a string and ignored characterEncoding. A decoder refuses a string that is not UTF-8, so a char declared ISO-8859-1 imported cleanly and its first byte past ASCII failed the whole message. The draft now maps a char to string only for an absent, US-ASCII or UTF-8 encoding, and to bytes of the same length otherwise (protowire#494). The names and the comment line follow protowire-go#248. --- CHANGELOG.md | 1 + .../java/org/protowire/sbe/XmlSchema.java | 3 + .../main/java/org/protowire/sbe/Convert.java | 56 +++++++++++-- .../protowire/sbe/CharacterEncodingTest.java | 84 +++++++++++++++++++ sbe/src/test/proto/charenc/enc.proto | 32 +++++++ sbe/src/test/resources/charenc/enc.xml | 23 +++++ 6 files changed, 192 insertions(+), 7 deletions(-) create mode 100644 sbe/src/test/java/org/protowire/sbe/CharacterEncodingTest.java create mode 100644 sbe/src/test/proto/charenc/enc.proto create mode 100644 sbe/src/test/resources/charenc/enc.xml diff --git a/CHANGELOG.md b/CHANGELOG.md index bca926f..0d1041a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ format changes. ### Added - **Producers refuse a message nested past `MaxNestingDepth`** (#167; HARDENING.md § Recursion, protowire#502). `Pxf.marshal` counts each `{` and `[` it writes, as a decoder of the text counts them. `Pb.marshal` counts each submessage and map entry. Past the limit, `Pxf.marshal` throws `PxfException` and `Pb.marshal` throws `IOException`, both with the message `nesting depth exceeds MaxNestingDepth=100`. Before, they wrote a document that a reader at the same limit refuses, and a reference cycle in a `Pb` struct overflowed the stack. The limit is per call: `MarshalOptions.withMaxNestingDepth` in `pxf`, and the new `org.protowire.pb.MarshalOptions` with `Pb.marshal(Object, MarshalOptions)`. An Any whose body would go past the limit is still written, as `type_url` and `value`. `check-decode --format pb-render` now reads its input with the stock reader's limit raised, so the corpus's 101-level rows reach the producer. +- **`Convert.xmlToProto` reads a `char` type's `characterEncoding`** (#173; draft `-01` § Character Encoding, protowire#494). A `char` becomes a `string` only when the encoding is absent (the FIX SBE default, US-ASCII) or names US-ASCII or UTF-8, by IANA name or alias, ignoring case and punctuation. Any other encoding becomes `bytes` of the same `(sbe.length)`, with a comment that says why. Before, every `char` was a `string`, so a schema declaring `ISO-8859-1` imported cleanly, and the first byte outside UTF-8 then failed the whole message. This follows protowire-go#248. - **`check-decode` and `check-decode-android` gain `--format pxf-pb`** (protowire#509), for the spec's new write gate. It reads a PXF document and writes the PB bytes on stdout. The full tier decodes with `unmarshalFull`, as `dump-envelope` does, and the harness sorts map entries by key before writing, because protobuf-java writes a `DynamicMessage`'s map entries in insertion order. On its first run the gate found 20 pairs that differ from protowire-go, filed as #156, #157 and #158. - **`PxfMeta.implicitPresenceFields()`** (#156): the singular fields with no presence, which a writer leaves out at their zero value. The default is empty. `protoc-gen-pxf-java-meta` emits `IMPLICIT_PRESENCE_FIELDS` from trendvidia/protowire#532 on, so a companion generated by an older plugin keeps writing every zero. diff --git a/sbe-runtime/src/main/java/org/protowire/sbe/XmlSchema.java b/sbe-runtime/src/main/java/org/protowire/sbe/XmlSchema.java index a5f4688..229619e 100644 --- a/sbe-runtime/src/main/java/org/protowire/sbe/XmlSchema.java +++ b/sbe-runtime/src/main/java/org/protowire/sbe/XmlSchema.java @@ -26,6 +26,8 @@ public final class XmlSchema { public static final class XmlType { public String name; public String primitiveType; public int length; + /** The IANA charset name a {@code char} type's bytes are in; empty when absent (US-ASCII). */ + public String characterEncoding = ""; } public static final class XmlComposite { @@ -90,6 +92,7 @@ static XmlType parseType(Element e) { t.name = e.getAttribute("name"); t.primitiveType = e.getAttribute("primitiveType"); t.length = parseInt(e.getAttribute("length"), 0); + t.characterEncoding = e.getAttribute("characterEncoding"); return t; } diff --git a/sbe/src/main/java/org/protowire/sbe/Convert.java b/sbe/src/main/java/org/protowire/sbe/Convert.java index 37aa454..bb4cc59 100644 --- a/sbe/src/main/java/org/protowire/sbe/Convert.java +++ b/sbe/src/main/java/org/protowire/sbe/Convert.java @@ -71,10 +71,7 @@ static void writeProtoComposite(StringBuilder b, XmlSchema.XmlComposite c) { b.append("message ").append(c.name).append(" {\n"); int n = 1; for (XmlSchema.XmlType t : c.types) { - String[] mt = resolveTypeToProto(t.primitiveType, t.length); - String name = camelToSnake(t.name); - if (!mt[1].isEmpty()) b.append(" ").append(mt[0]).append(' ').append(name).append(" = ").append(n).append(" [").append(mt[1]).append("];\n"); - else b.append(" ").append(mt[0]).append(' ').append(name).append(" = ").append(n).append(";\n"); + writeTypedField(b, " ", resolveTypeToProto(t), camelToSnake(t.name), n); n++; } for (XmlSchema.XmlRef r : c.refs) { @@ -106,9 +103,7 @@ static void writeProtoField(StringBuilder b, XmlSchema.XmlField f, } XmlSchema.XmlType t = typeMap.get(f.type); if (t != null) { - String[] mt = resolveTypeToProto(t.primitiveType, t.length); - if (!mt[1].isEmpty()) b.append(indent).append(mt[0]).append(' ').append(name).append(" = ").append(f.id).append(" [").append(mt[1]).append("];\n"); - else b.append(indent).append(mt[0]).append(' ').append(name).append(" = ").append(f.id).append(";\n"); + writeTypedField(b, indent, resolveTypeToProto(t), name, f.id); return; } b.append(indent).append(f.type).append(' ').append(name).append(" = ").append(f.id).append(";\n"); @@ -127,6 +122,53 @@ static void writeProtoGroup(StringBuilder b, XmlSchema.XmlGroup g, .append(" = ").append(g.id).append(";\n"); } + /** Writes a field from {@link #resolveTypeToProto(XmlSchema.XmlType)}'s type, options and comment. */ + private static void writeTypedField(StringBuilder b, String indent, String[] mt, String name, Object number) { + if (!mt[2].isEmpty()) b.append(indent).append("// ").append(mt[2]).append('\n'); + b.append(indent).append(mt[0]).append(' ').append(name).append(" = ").append(number); + if (!mt[1].isEmpty()) b.append(" [").append(mt[1]).append(']'); + b.append(";\n"); + } + + /** + * The proto type, options and a comment line (empty for none) of an SBE + * type. A {@code char} becomes a {@code string} only when its + * {@code characterEncoding} is UTF-8 compatible, and {@code bytes} of the + * same length otherwise: a decoder refuses a {@code string} that is not + * UTF-8, so a Latin-1 byte would fail the whole message (draft -01 + * § Character Encoding, protowire#494, #173). As protowire-go's + * {@code xmltoproto.go} words it. + */ + static String[] resolveTypeToProto(XmlSchema.XmlType t) { + String[] mt = resolveTypeToProto(t.primitiveType, t.length); + if ("char".equals(t.primitiveType) && !isUtf8Compatible(t.characterEncoding)) { + return new String[]{"bytes", mt[1], "SBE characterEncoding \"" + t.characterEncoding + + "\" is not UTF-8 compatible, so this field is bytes."}; + } + return new String[]{mt[0], mt[1], ""}; + } + + /** + * Whether every string in a {@code char} array of this encoding is valid + * UTF-8: absent (the FIX SBE default, US-ASCII), US-ASCII or UTF-8, by + * IANA name or alias, ignoring case, spaces and {@code -_.:}. + */ + static boolean isUtf8Compatible(String encoding) { + StringBuilder name = new StringBuilder(); + for (char c : (encoding == null ? "" : encoding.strip()).toCharArray()) { + if ("-_.: ".indexOf(c) < 0) name.append(Character.toLowerCase(c)); + } + return switch (name.toString()) { + case "", + // US-ASCII and its IANA aliases. + "usascii", "ascii", "ansix341968", "ansix341986", "iso646us", + "isoir6", "iso646irv1991", "us", "ibm367", "cp367", "csascii", + // UTF-8. + "utf8", "csutf8" -> true; + default -> false; + }; + } + static String[] resolveTypeToProto(String primitive, int length) { return switch (primitive) { case "int8" -> new String[]{"int32", "(sbe.encoding) = \"int8\""}; diff --git a/sbe/src/test/java/org/protowire/sbe/CharacterEncodingTest.java b/sbe/src/test/java/org/protowire/sbe/CharacterEncodingTest.java new file mode 100644 index 0000000..9428219 --- /dev/null +++ b/sbe/src/test/java/org/protowire/sbe/CharacterEncodingTest.java @@ -0,0 +1,84 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2026 TrendVidia, LLC. +package org.protowire.sbe; + +import com.google.protobuf.ByteString; +import enc.v1.Enc.Text; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; + +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * The XML importer maps a {@code char} to {@code string} only when its + * {@code characterEncoding} is UTF-8 compatible, and to {@code bytes} + * otherwise (draft -01 § Character Encoding, protowire#494, #173). The + * schema is protowire-go's TestXMLToProtoCharacterEncoding. + */ +final class CharacterEncodingTest { + + private static String resource(String path) throws IOException { + try (InputStream in = CharacterEncodingTest.class.getResourceAsStream(path)) { + return new String(in.readAllBytes(), StandardCharsets.UTF_8); + } + } + + private static String generated() throws IOException { + return Convert.xmlToProto(resource("/charenc/enc.xml").getBytes(StandardCharsets.UTF_8)); + } + + @Test + void eachEncodingMapsToItsType() throws IOException { + String proto = generated(); + for (String line : new String[] { + "string plain = 1 [(sbe.length) = 4];", + "string ascii = 2 [(sbe.length) = 4];", + "string us_ascii = 3 [(sbe.length) = 4];", + "string utf8 = 4 [(sbe.length) = 4];", + " // SBE characterEncoding \"ISO-8859-1\" is not UTF-8 compatible, so this field is bytes.\n" + + " bytes latin1 = 5 [(sbe.length) = 4];", + " // SBE characterEncoding \"windows-1252\" is not UTF-8 compatible, so this field is bytes.\n" + + " bytes code = 1 [(sbe.length) = 2];", + "string tag = 2 [(sbe.length) = 2];", + }) { + assertTrue(proto.contains(line), line + "\n--- in ---\n" + proto); + } + } + + /** The compiled copy is this output, so the build has compiled what the importer writes. */ + @Test + void theCompiledProtoIsTheImportersOutput() throws IOException { + String compiled = resource("/charenc/enc.proto"); + assertEquals(generated(), compiled.substring(compiled.indexOf("syntax = "))); + } + + @Test + void aLatin1FieldCarriesItsBytes() { + Codec codec = Codec.of(Text.getDescriptor().getFile()); + byte[] latin1 = {'c', 'a', 'f', (byte) 0xE9}; + byte[] data = codec.marshal(Text.newBuilder().setLatin1(ByteString.copyFrom(latin1)).build()); + Text.Builder back = Text.newBuilder(); + codec.unmarshal(data, back); + assertArrayEquals(latin1, back.getLatin1().toByteArray()); + } + + @ParameterizedTest + @ValueSource(strings = {"", "US-ASCII", "us_ascii", "ASCII", "ANSI_X3.4-1968", "IBM367", "csASCII", "UTF-8", "utf8", " Utf-8 "}) + void utf8CompatibleNames(String name) { + assertTrue(Convert.isUtf8Compatible(name), name); + } + + @ParameterizedTest + @ValueSource(strings = {"ISO-8859-1", "windows-1252", "UTF-16", "latin1", "Shift_JIS"}) + void otherNames(String name) { + assertFalse(Convert.isUtf8Compatible(name), name); + } +} diff --git a/sbe/src/test/proto/charenc/enc.proto b/sbe/src/test/proto/charenc/enc.proto new file mode 100644 index 0000000..618d836 --- /dev/null +++ b/sbe/src/test/proto/charenc/enc.proto @@ -0,0 +1,32 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2026 TrendVidia, LLC. +// Convert.xmlToProto's output for src/test/resources/charenc/enc.xml, byte for +// byte after this header (CharacterEncodingTest checks it), compiled here so the +// build proves it compiles (#173). + +syntax = "proto3"; + +package enc.v1; + +import "sbe/annotations.proto"; + +option (sbe.schema_id) = 2; +option (sbe.version) = 0; + +message Pair { + // SBE characterEncoding "windows-1252" is not UTF-8 compatible, so this field is bytes. + bytes code = 1 [(sbe.length) = 2]; + string tag = 2 [(sbe.length) = 2]; +} + +message Text { + option (sbe.template_id) = 1; + string plain = 1 [(sbe.length) = 4]; + string ascii = 2 [(sbe.length) = 4]; + string us_ascii = 3 [(sbe.length) = 4]; + string utf8 = 4 [(sbe.length) = 4]; + // SBE characterEncoding "ISO-8859-1" is not UTF-8 compatible, so this field is bytes. + bytes latin1 = 5 [(sbe.length) = 4]; + Pair pair = 6; +} + diff --git a/sbe/src/test/resources/charenc/enc.xml b/sbe/src/test/resources/charenc/enc.xml new file mode 100644 index 0000000..fd102b5 --- /dev/null +++ b/sbe/src/test/resources/charenc/enc.xml @@ -0,0 +1,23 @@ + + + + + + + + + + + + + + + + + + + + + +