From 3fe790335cd5c32374edefe850f2780e2df22989 Mon Sep 17 00:00:00 2001 From: Burak KALAYCI Date: Fri, 28 Aug 2026 11:34:46 +0300 Subject: [PATCH] Fix #761: pad Smile async parser name quads like blocking parser Unused high bytes of a partial name quad were left as zeros, so a short name collided with a longer NUL-prefixed one in the symbol table. Match SmileParser's 1s-padding on every partial quad. --- release-notes/CREDITS-2.x | 5 + release-notes/VERSION-2.x | 5 + .../async/NonBlockingByteArrayParser.java | 2 +- .../smile/async/NonBlockingParserBase.java | 27 ++-- .../smile/async/AsyncSymbolTable312Test.java | 141 ++++++++++++++++++ 5 files changed, 169 insertions(+), 11 deletions(-) create mode 100644 smile/src/test/java/com/fasterxml/jackson/dataformat/smile/async/AsyncSymbolTable312Test.java diff --git a/release-notes/CREDITS-2.x b/release-notes/CREDITS-2.x index 9efa43471..8b8247fa5 100644 --- a/release-notes/CREDITS-2.x +++ b/release-notes/CREDITS-2.x @@ -461,3 +461,8 @@ Benjamin Muschko (@bmuschko) * Reported #720: (smile) `SmileGenerator` int overflow in `maxLen` for very long Strings causes `ArrayIndexOutOfBoundsException` (2.21.6) + +Burak KALAYCI (@kalayciburak) + +* Contributed fix for #761: (smile) Async parser misses #312 NUL-padding + (2.21.7) diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index ccff04a0f..03915305d 100644 --- a/release-notes/VERSION-2.x +++ b/release-notes/VERSION-2.x @@ -14,6 +14,11 @@ Active maintainers: === Releases === ------------------------------------------------------------------------ +2.21.7 (not yet released) + +#761: (smile) Async parser misses #312 NUL-padding: short property name + collides with NUL-prefixed longer one + 2.21.6 (14-Aug-2026) #708: (protobuf) proto3 label-less field declarations fail to parse diff --git a/smile/src/main/java/com/fasterxml/jackson/dataformat/smile/async/NonBlockingByteArrayParser.java b/smile/src/main/java/com/fasterxml/jackson/dataformat/smile/async/NonBlockingByteArrayParser.java index 81676e1c4..76ed113bf 100644 --- a/smile/src/main/java/com/fasterxml/jackson/dataformat/smile/async/NonBlockingByteArrayParser.java +++ b/smile/src/main/java/com/fasterxml/jackson/dataformat/smile/async/NonBlockingByteArrayParser.java @@ -690,7 +690,7 @@ private final JsonToken _finishLongFieldName(int outPtr) throws IOException } // and possibly more... ? if (in < outPtr) { // at least 1 - int q = copyBuffer[in++] & 0xFF; + int q = _padQuadForNulls(copyBuffer[in++]); if (in < outPtr) { // at least 2 q = (q << 8) | (copyBuffer[in++] & 0xFF); if (in < outPtr) { // 3 (can't be more) diff --git a/smile/src/main/java/com/fasterxml/jackson/dataformat/smile/async/NonBlockingParserBase.java b/smile/src/main/java/com/fasterxml/jackson/dataformat/smile/async/NonBlockingParserBase.java index ba2fb1cda..4d17993ac 100644 --- a/smile/src/main/java/com/fasterxml/jackson/dataformat/smile/async/NonBlockingParserBase.java +++ b/smile/src/main/java/com/fasterxml/jackson/dataformat/smile/async/NonBlockingParserBase.java @@ -462,12 +462,14 @@ protected final String _findDecodedFromSymbols(byte[] inBuf, int inPtr, int len) { // First: maybe we already have this name decoded? if (len < 5) { - int q = inBuf[inPtr] & 0xFF; - if (--len > 0) { + // [dataformats-binary#761] / #312: pad unused high bytes so a short name + // cannot collide with a longer NUL-prefixed one in ByteQuadsCanonicalizer + int q = _padQuadForNulls(inBuf[inPtr]); + if (len > 1) { q = (q << 8) + (inBuf[++inPtr] & 0xFF); - if (--len > 0) { + if (len > 2) { q = (q << 8) + (inBuf[++inPtr] & 0xFF); - if (--len > 0) { + if (len > 3) { q = (q << 8) + (inBuf[++inPtr] & 0xFF); } } @@ -483,13 +485,13 @@ protected final String _findDecodedFromSymbols(byte[] inBuf, int inPtr, int len) q1 += (inBuf[++inPtr] & 0xFF); q1 <<= 8; q1 += (inBuf[++inPtr] & 0xFF); - int q2 = (inBuf[++inPtr] & 0xFF); - len -= 5; - if (len > 0) { + int q2 = _padQuadForNulls(inBuf[++inPtr]); + int left = len - 5; + if (left > 0) { q2 = (q2 << 8) + (inBuf[++inPtr] & 0xFF); - if (--len > 0) { + if (left > 1) { q2 = (q2 << 8) + (inBuf[++inPtr] & 0xFF); - if (--len > 0) { + if (left > 2) { q2 = (q2 << 8) + (inBuf[++inPtr] & 0xFF); } } @@ -524,7 +526,7 @@ private final String _findDecodedLonger(byte[] inBuf, int inPtr, int len) throws } while ((len -= 4) > 3); // and then leftovers if (len > 0) { - int q = inBuf[inPtr] & 0xFF; + int q = _padQuadForNulls(inBuf[inPtr]); if (--len > 0) { q = (q << 8) + (inBuf[++inPtr] & 0xFF); if (--len > 0) { @@ -536,6 +538,11 @@ private final String _findDecodedLonger(byte[] inBuf, int inPtr, int len) throws return _symbols.findName(_quadBuffer, offset); } + // Helper method needed to fix [dataformats-binary#312]/#761, masking of 0x00 character + protected final static int _padQuadForNulls(int firstByte) { + return (firstByte & 0xFF) | 0xFFFFFF00; + } + protected final String _addDecodedToSymbols(int len, String name) throws IOException { // 5-May-2023, ckozak: [core#1015] respect CANONICALIZE_FIELD_NAMES factory config. diff --git a/smile/src/test/java/com/fasterxml/jackson/dataformat/smile/async/AsyncSymbolTable312Test.java b/smile/src/test/java/com/fasterxml/jackson/dataformat/smile/async/AsyncSymbolTable312Test.java new file mode 100644 index 000000000..643f53eda --- /dev/null +++ b/smile/src/test/java/com/fasterxml/jackson/dataformat/smile/async/AsyncSymbolTable312Test.java @@ -0,0 +1,141 @@ +package com.fasterxml.jackson.dataformat.smile.async; + +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + +import org.junit.jupiter.api.Test; + +import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.core.JsonToken; +import com.fasterxml.jackson.dataformat.smile.SmileFactory; +import com.fasterxml.jackson.dataformat.smile.databind.SmileMapper; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** + * Async counterpart of {@code SymbolTable312Test}: the blocking Smile parser + * pads unused high bytes of a partial quad so that a short name cannot collide + * with a longer NUL-prefixed one. The non-blocking parser missed that padding + * ([dataformats-binary#761], follow-up to #312). + */ +public class AsyncSymbolTable312Test extends AsyncTestBase +{ + private final SmileMapper MAPPER = smileMapper(); + + @Test + public void testShortNameDoesNotCollideWithNulPrefixedLonger() throws Exception + { + // Issue #761 repro: 1-byte "a" and 4-byte "\0\0\0a" both hashed as 0x00000061 + // without padding, so the async parser reports the first name twice. + final String n1 = new String(new char[] { 0, 0, 0, 'a' }); + final String n2 = "a"; + + Map m = new LinkedHashMap<>(); + m.put(n1, 1); + m.put(n2, 2); + byte[] doc = MAPPER.writeValueAsBytes(m); + + assertEquals(listOf(n1, n2), _readNamesBlocking(doc)); + assertEquals(listOf(n1, n2), _readNamesAsync(doc, Integer.MAX_VALUE)); + assertEquals(listOf(n1, n2), _readNamesAsync(doc, 1)); + } + + @Test + public void testNullHandling1Quad() throws Exception + { + _testNullHandling(1); + _testNullHandling(2); + } + + @Test + public void testNullHandling2Quads() throws Exception + { + _testNullHandling(5); + _testNullHandling(6); + } + + @Test + public void testNullHandling3Quads() throws Exception + { + _testNullHandling(9); + _testNullHandling(10); + } + + @Test + public void testNullHandlingNQuads() throws Exception + { + _testNullHandling(13); + _testNullHandling(14); + _testNullHandling(17); + _testNullHandling(18); + _testNullHandling(21); + } + + private void _testNullHandling(int minNulls) throws Exception + { + Map m = new LinkedHashMap<>(); + List expected = new ArrayList<>(); + for (int i = 0; i < 5; i++) { + String name = _nulls(minNulls + i); + expected.add(name); + m.put(name, String.valueOf((char) ('a' + i))); + } + byte[] doc = MAPPER.writeValueAsBytes(m); + + assertEquals(expected, _readNamesBlocking(doc), + "blocking parser should distinguish NUL-only names of length " + + minNulls + ".." + (minNulls + 4)); + assertEquals(expected, _readNamesAsync(doc, Integer.MAX_VALUE), + "async parser should match blocking parser for NUL-only names of length " + + minNulls + ".." + (minNulls + 4)); + assertEquals(expected, _readNamesAsync(doc, 3), + "async parser (chunked) should match blocking parser for NUL-only names of length " + + minNulls + ".." + (minNulls + 4)); + } + + private List _readNamesBlocking(byte[] doc) throws Exception + { + List names = new ArrayList<>(); + try (JsonParser p = MAPPER.createParser(doc)) { + assertToken(JsonToken.START_OBJECT, p.nextToken()); + while (p.nextToken() == JsonToken.FIELD_NAME) { + names.add(p.currentName()); + p.nextToken(); + } + assertToken(JsonToken.END_OBJECT, p.currentToken()); + } + return names; + } + + private List _readNamesAsync(byte[] doc, int bytesPerRead) throws Exception + { + List names = new ArrayList<>(); + AsyncReaderWrapper p = asyncForBytes((SmileFactory) MAPPER.getFactory(), bytesPerRead, doc, 0); + try { + JsonToken t; + while ((t = p.nextToken()) != null) { + if (t == JsonToken.FIELD_NAME) { + names.add(p.currentName()); + } + } + } finally { + p.close(); + } + return names; + } + + private String _nulls(int len) { + return new String(new byte[len], StandardCharsets.US_ASCII); + } + + private static List listOf(String... values) { + List list = new ArrayList<>(values.length); + for (String v : values) { + list.add(v); + } + return list; + } +}