From 4c139be4bbda6f82684e5acb48784cb8f410b8e6 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Thu, 16 Jul 2026 20:43:42 -0700 Subject: [PATCH 1/7] Implement #712: idiomatic protobuf `map` support (read + write) Maps are encoded on the wire exactly like a `repeated` entry sub-message (key = tag 1, value = tag 2). This exposes them idiomatically as JSON Objects in both directions, at streaming and databind levels. Schema (TypeResolver / ProtobufField): a `map` field (or, from a protoc descriptor set, a `repeated` field whose entry message carries the `map_entry` option) is resolved into a synthetic entry message plus a repeated, map-flagged field. Key types are restricted to integral / bool / string. Generator: a map field is written as a sequence of length-delimited entry sub-messages; each writeFieldName opens an entry (key rendered from the JSON field name per the key type) and primes the value field. Parser: a map surfaces as START_OBJECT + FIELD_NAME(key)/value pairs + END_OBJECT, mirroring the unpacked-repeated-message machinery (with an entry context so end-offset tracking survives buffer reloads). Absent key/value decode to proto3 defaults; a value that precedes the key is reported clearly. Descriptor path (FileDescriptorSet): propagate the `map_entry` message option so .desc-loaded maps get the same treatment as .proto ones. NOTE: this changes the shape of .desc-loaded maps, which previously surfaced as an array of {key,value} entries. Also flips the two #708 tests that asserted maps throw, and updates the now-stale notes in ProtobufSchemaPreprocessor. Schema generation from POJO Map fields (ProtoBufSchemaVisitor.expectMapFormat) remains unsupported; left for a follow-up. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../protobuf/ProtobufGenerator.java | 146 +++++- .../dataformat/protobuf/ProtobufParser.java | 438 ++++++++++++++---- .../protobuf/ProtobufReadContext.java | 72 +++ .../protobuf/ProtobufWriteContext.java | 60 +++ .../protobuf/schema/FileDescriptorSet.java | 7 + .../protobuf/schema/ProtobufField.java | 81 ++++ .../protobuf/schema/ProtobufMessage.java | 19 + .../schema/ProtobufSchemaPreprocessor.java | 15 +- .../protobuf/schema/TypeResolver.java | 120 ++++- .../dataformat/protobuf/MapField712Test.java | 371 +++++++++++++++ .../schema/DescriptorMapField712Test.java | 107 +++++ .../schema/Proto3LabellessField708Test.java | 31 +- release-notes/VERSION-2.x | 3 +- 13 files changed, 1356 insertions(+), 114 deletions(-) create mode 100644 protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/MapField712Test.java create mode 100644 protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/schema/DescriptorMapField712Test.java diff --git a/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufGenerator.java b/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufGenerator.java index 56df1ee84..dd9ae0e8a 100644 --- a/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufGenerator.java +++ b/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufGenerator.java @@ -281,6 +281,12 @@ public final void writeFieldName(String name) throws IOException { if (!_inObject) { _reportError("Can not write field name: current context not Object but "+_pbContext.typeDesc()); } + // [dataformats-binary#712] Within a `map`, each "field name" is a map key + // that opens a new entry sub-message + if (_pbContext.inMap()) { + _startMapEntry(name); + return; + } ProtobufField f = _currField; // important: use current field only if NOT repeated field; repeated // field means an array until START_OBJECT @@ -312,8 +318,14 @@ public final void writeFieldName(SerializableString sstr) throws IOException { if (!_inObject) { _reportError("Can not write field name: current context not Object but "+_pbContext.typeDesc()); } - ProtobufField f = _currField; final String name = sstr.getValue(); + // [dataformats-binary#712] Within a `map`, each "field name" is a map key + // that opens a new entry sub-message + if (_pbContext.inMap()) { + _startMapEntry(name); + return; + } + ProtobufField f = _currField; // important: use current field only if NOT repeated field; repeated // field means an array until START_OBJECT // NOTE: not ideal -- depends on if it really is sibling field of an array, @@ -417,6 +429,11 @@ public final void writeStartArray() throws IOException _reportError("Can not write START_ARRAY without field (message type "+_currMessage.getName()+")"); return; // never gets here but code analyzers can't see that } + // [dataformats-binary#712] A `map` field is also "repeated" underneath, but + // must be written as an Object, not an Array + if (_currField.isMap) { + _reportError("Can not write START_ARRAY: field '"+_currField.name+"' is a `map`; write START_OBJECT instead"); + } if (!_currField.isArray()) { _reportError("Can not write START_ARRAY: field '"+_currField.name+"' not declared as 'repeated'"); } @@ -470,6 +487,17 @@ public final void writeStartObject() throws IOException } _currMessage = _schema.getRootType(); // note: no buffering on root + } else if (_currField.isMap) { + // [dataformats-binary#712] a `map` field: the Object being opened is + // a sequence of entry sub-messages, not a single one. Push a dedicated map + // context; per-entry buffering happens in writeFieldName. No buffering of + // the map as a whole. + _pbContext = _pbContext.createChildMapContext(_currField); + streamWriteConstraints().validateNestingDepth(_pbContext.getNestingDepth()); + _currMessage = _currField.getMessageType(); // the synthetic entry message + _inObject = true; + _writeTag = true; + return; } else { // but also, field value must be Message if so if (!_currField.isObject) { @@ -504,6 +532,28 @@ public final void writeEndObject() throws IOException if (!_inObject) { _reportError("Current context not Object but "+_pbContext.typeDesc()); } + // [dataformats-binary#712] Closing a `map`: finalize the last open entry (if + // any), then pop -- but do NOT finish-buffer the map as a whole (each entry + // was already length-prefixed on its own). + if (_pbContext.inMap()) { + if (_pbContext.isEntryOpen()) { + _pbContext.setEntryOpen(false); + _finishBuffering(); + } + _pbContext = _pbContext.getParent(); + if (_pbContext.inRoot()) { + if (!_complete) { + _complete(); + } + } else { + _currMessage = _pbContext.getMessageType(); + } + _currField = _pbContext.getField(); + boolean inObj = _pbContext.inObject(); + _inObject = inObj; + _writeTag = inObj || !_pbContext.inArray() || !_currField.packed; + return; + } _pbContext = _pbContext.getParent(); if (_pbContext.inRoot()) { if (!_complete) { @@ -1741,6 +1791,100 @@ private final int _writeTag(int ptr) * Method called when buffering an entry that should be prefixed * with a type tag. */ + /* + /********************************************************** + /* Internal map (`map`) writes [dataformats-binary#712] + /********************************************************** + */ + + /** + * Opens a new {@code map} entry sub-message for the given key, finalizing the + * previous entry first if one is still open. Writes the entry's key (tag 1) and + * leaves {@link #_currField} pointing at the value field (tag 2) so the value + * write that follows targets it. + */ + private void _startMapEntry(String name) throws IOException + { + final ProtobufField mapField = _pbContext.getField(); + // Close the previous entry, if any (its length prefix is finalized here) + if (_pbContext.isEntryOpen()) { + _pbContext.setEntryOpen(false); + _finishBuffering(); + } + // Each entry is a length-delimited sub-message, tagged with the map field's tag + _startBuffering(mapField.typedTag); + _pbContext.setEntryOpen(true); + _currMessage = mapField.getMessageType(); + // Write key (tag 1) ... + _writeTag = true; + _writeMapKey(mapField.getKeyField(), name); + // ... then prime the value field (tag 2) for the value write that follows + _currField = mapField.getValueField(); + _writeTag = true; + } + + /** + * Writes a map key (always arriving as a {@code String}) using the key field's + * declared protobuf type. Key types are restricted (and validated during schema + * resolution) to integral / {@code bool} / {@code string}. + */ + private void _writeMapKey(ProtobufField keyField, String name) throws IOException + { + _currField = keyField; + switch (keyField.type) { + case STRING: + writeString(name); + break; + case BOOLEAN: + writeBoolean(_parseBooleanMapKey(name, keyField)); + break; + case VINT32_STD: + case VINT32_Z: + case FIXINT32: + writeNumber(_parseIntMapKey(name, keyField)); + break; + case VINT64_STD: + case VINT64_Z: + case FIXINT64: + writeNumber(_parseLongMapKey(name, keyField)); + break; + default: + // Should not happen: key types are validated during schema resolution + _reportError("Unsupported `map` key type "+keyField.type+" for field '"+keyField.name+"'"); + } + } + + private int _parseIntMapKey(String name, ProtobufField keyField) throws IOException { + try { + return Integer.parseInt(name); + } catch (NumberFormatException e) { + _reportError("Invalid `map` key \""+name+"\" for integral key field '"+keyField.name + +"' (type "+keyField.type+")"); + return 0; // never reached + } + } + + private long _parseLongMapKey(String name, ProtobufField keyField) throws IOException { + try { + return Long.parseLong(name); + } catch (NumberFormatException e) { + _reportError("Invalid `map` key \""+name+"\" for integral key field '"+keyField.name + +"' (type "+keyField.type+")"); + return 0L; // never reached + } + } + + private boolean _parseBooleanMapKey(String name, ProtobufField keyField) throws IOException { + if ("true".equals(name)) { + return true; + } + if ("false".equals(name)) { + return false; + } + _reportError("Invalid `map` key \""+name+"\" for boolean key field '"+keyField.name+"'"); + return false; // never reached + } + private final void _startBuffering(int typedTag) throws IOException { // need to ensure room for tag id, length (10 bytes); might as well ask for bit more diff --git a/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufParser.java b/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufParser.java index 8baaeefd6..f236a1aea 100644 --- a/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufParser.java +++ b/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufParser.java @@ -55,6 +55,21 @@ public class ProtobufParser extends ParserMinimalBase // (added for [dataformats-binary#598] to separate END_OBJECT return from close()) private final static int STATE_ROOT_END = 13; + // [dataformats-binary#712] `map` states: a map surfaces as a JSON Object whose + // members are the map entries (each entry a length-delimited key/value sub-message). + + // About to start a map (its field tag already consumed): emit START_OBJECT + private final static int STATE_MAP_START = 14; + + // About to open the first entry (map field tag already consumed) + private final static int STATE_MAP_ENTRY_FIRST = 15; + + // Between entries: read the next tag (another entry, or end of map) + private final static int STATE_MAP_ENTRY_OTHER = 16; + + // Entry key has been surfaced (FIELD_NAME); read the entry value next + private final static int STATE_MAP_VALUE = 17; + private final static int[] UTF8_UNIT_CODES = ProtobufUtil.sUtf8UnitLengths; // @since 2.14 @@ -253,6 +268,16 @@ public class ProtobufParser extends ParserMinimalBase protected int _nextTag; + /** + * For {@code map} decoding: when a map entry's {@code value} (tag 2) tag has + * been read while scanning for the (possibly absent) {@code key} (tag 1), it is + * stashed here to be consumed by the value read that follows. {@code 0} means none + * (tag 0 is not a valid field tag). + * + * @since 2.21.6 [dataformats-binary#712] + */ + protected int _mapValueTag; + /** * Length of the value that parser points to, for scalar values that use length * prefixes (Strings, binary data). @@ -697,6 +722,52 @@ public JsonToken nextToken() throws IOException case STATE_NESTED_VALUE: return _updateToken(_readNextValue(_currentField.type, STATE_NESTED_KEY)); + case STATE_MAP_START: + // [dataformats-binary#712] Map field tag already consumed; push map context + // (inherits enclosing end offset like an unpacked array) and open as Object + _parsingContext = _parsingContext.createChildMapContext(_currentField, _currentEndOffset); + _streamReadConstraints.validateNestingDepth(_parsingContext.getNestingDepth()); + _state = STATE_MAP_ENTRY_FIRST; + return _updateToken(JsonToken.START_OBJECT); + + case STATE_MAP_ENTRY_FIRST: // map field tag already consumed + return _updateToken(_openMapEntry()); + + case STATE_MAP_ENTRY_OTHER: + if (_checkEnd()) { // reached end of enclosing message: map ends + return _updateToken(JsonToken.END_OBJECT); + } + if (_inputPtr >= _inputEnd) { + if (!loadMore()) { + ProtobufReadContext parent = _parsingContext.getParent(); + // Ok to end only if this map is a root-level value + if (!parent.inRoot()) { + _reportInvalidEOF(); + } + _parsingContext = parent; + _currentField = parent.getField(); + _state = STATE_MESSAGE_END; + return _updateToken(JsonToken.END_OBJECT); + } + } + { + int tag = _decodeVInt(); + // expected case: another entry for the same map field + if (_currentField.id == (tag >> 3)) { + return _updateToken(_openMapEntry()); + } + // otherwise a different field: end this map, replay tag in parent + _nextTag = tag; + ProtobufReadContext parent = _parsingContext.getParent(); + _parsingContext = parent; + _currentField = parent.getField(); + _state = STATE_ARRAY_END; // reused: replays _nextTag as a key + return _updateToken(JsonToken.END_OBJECT); + } + + case STATE_MAP_VALUE: + return _updateToken(_readMapValue()); + case STATE_MESSAGE_END: // occurs if we end with array _state = STATE_ROOT_END; _parsingContext.setCurrentName(null); @@ -729,10 +800,25 @@ private boolean _checkEnd() throws IOException _currentMessage = parentCtxt.getMessageType(); _currentEndOffset = parentCtxt.getEndOffset(); _currentField = parentCtxt.getField(); + // [dataformats-binary#712] If we popped into a map entry that is now fully + // consumed (its message-typed value was the last field), pop the entry too so + // we resume iterating entries rather than treating it as a nested message. + if (parentCtxt.isMapEntry() && (_inputPtr >= _currentEndOffset)) { + ProtobufReadContext mapCtxt = parentCtxt.getParent(); + _parsingContext = mapCtxt; + _currentMessage = mapCtxt.getMessageType(); + _currentEndOffset = mapCtxt.getEndOffset(); + _currentField = mapCtxt.getField(); + _state = STATE_MAP_ENTRY_OTHER; + return true; + } if (_parsingContext.inRoot()) { _state = STATE_ROOT_KEY; } else if (_parsingContext.inArray()) { _state = _currentField.packed ? STATE_ARRAY_VALUE_PACKED : STATE_ARRAY_VALUE_OTHER; + } else if (_parsingContext.inMap()) { + // popped back into a map (e.g. from a message-typed value): iterate entries + _state = STATE_MAP_ENTRY_OTHER; } else { _state = STATE_NESTED_KEY; } @@ -761,19 +847,8 @@ private JsonToken _handleRootKey(int tag) throws IOException if (!f.isValidFor(wireType)) { _reportIncompatibleType(f, wireType); } - // array? - if (f.repeated) { - // 03-Jul-2026, tatu: [dataformats-binary#134] Decide packed-vs-unpacked from - // the actual wire type, not just the schema's declared `packed` flag: - // proto3 permits either encoding for repeated scalar/enum fields. - if (f.isPackedInWire(wireType)) { - _state = STATE_ARRAY_START_PACKED; - } else { - _state = STATE_ARRAY_START; - } - } else { - _state = STATE_ROOT_VALUE; - } + // map / array / single value? + _state = _stateAfterFieldName(f, wireType, STATE_ROOT_VALUE); _currentField = f; return _updateToken(JsonToken.FIELD_NAME); } @@ -808,19 +883,8 @@ private JsonToken _handleNestedKey(int tag) throws IOException _reportIncompatibleType(f, wireType); } - // array? - if (f.repeated) { - // 03-Jul-2026, tatu: [dataformats-binary#134] Decide packed-vs-unpacked from - // the actual wire type, not just the schema's declared `packed` flag: - // proto3 permits either encoding for repeated scalar/enum fields. - if (f.isPackedInWire(wireType)) { - _state = STATE_ARRAY_START_PACKED; - } else { - _state = STATE_ARRAY_START; - } - } else { - _state = STATE_NESTED_VALUE; - } + // map / array / single value? + _state = _stateAfterFieldName(f, wireType, STATE_NESTED_VALUE); _currentField = f; return _updateToken(JsonToken.FIELD_NAME); } @@ -1042,6 +1106,249 @@ private void _skipUnknownValue(int wireType) throws IOException } } + /* + /********************************************************** + /* Internal methods, `map` decoding [dataformats-binary#712] + /********************************************************** + */ + + /** + * Opens the next {@code map} entry sub-message (whose length prefix is at the current + * position), reads its key (tag 1, possibly absent), and surfaces the key as the + * current field name. Leaves {@link #_currentField} pointing at the value field and + * transitions to {@link #STATE_MAP_VALUE}. + */ + private JsonToken _openMapEntry() throws IOException + { + final ProtobufField mapField = _currentField; + final int len = _decodeLength(); + final int newEnd = _inputPtr + len; + // Guard against integer overflow (see MESSAGE handling in _readNextValue) + if (newEnd < _inputPtr) { + _reportErrorF("Map entry length overflows for field '%s': ptr=%d, len=%d", + mapField.name, _inputPtr, len); + } + if (newEnd > _currentEndOffset) { + _reportErrorF("Map entry for field '%s' extends past end of enclosing message: %d > %d (length: %d)", + mapField.name, newEnd, _currentEndOffset, len); + } + _parsingContext = _parsingContext.createChildMapEntryContext(newEnd); + _streamReadConstraints.validateNestingDepth(_parsingContext.getNestingDepth()); + _currentEndOffset = newEnd; + _currentMessage = mapField.getMessageType(); + + final ProtobufField keyField = mapField.getKeyField(); + // Read the key (tag 1). It may be absent (proto3 default), and unknown fields may + // (rarely) precede it; if the value (tag 2) is seen first, stash it. + _mapValueTag = 0; + String keyName = null; + while (keyName == null) { + if (_inputPtr >= newEnd) { // empty (or key-less) entry + keyName = _defaultMapKeyName(keyField); + break; + } + final int tag = _decodeVInt(); + final int id = (tag >> 3); + final int wireType = (tag & 0x7); + if (id == 1) { + keyName = _readMapKeyValue(keyField, wireType); + } else if (id == 2) { + keyName = _defaultMapKeyName(keyField); // key omitted; this is the value + _mapValueTag = tag; + } else { + _skipUnknownValue(wireType); // unknown field inside entry: keep scanning + } + } + _parsingContext.setCurrentName(keyName); + _currentField = mapField.getValueField(); + _state = STATE_MAP_VALUE; + return JsonToken.FIELD_NAME; + } + + /** + * Reads the value of the current {@code map} entry (tag 2), synthesizing a proto3 + * default when the value is absent. For scalar values the entry is then popped; for + * message values the nested Object is streamed (and the entry popped afterwards, in + * {@link #_checkEnd}). + */ + private JsonToken _readMapValue() throws IOException + { + final ProtobufField valueField = _currentField; + int tag; + if (_mapValueTag != 0) { + tag = _mapValueTag; + _mapValueTag = 0; + } else { + while (true) { + if (_inputPtr >= _currentEndOffset) { // value absent -> proto3 default + JsonToken t = _mapDefaultValueToken(valueField); + _popMapEntry(); + return t; + } + tag = _decodeVInt(); + final int id = (tag >> 3); + if (id == 2) { + break; + } + if (id == 1) { + _reportErrorF("Map entry for field has 'key' (id 1) after 'value': not supported"); + } + _skipUnknownValue(tag & 0x7); // unknown field inside entry + } + } + final int wireType = (tag & 0x7); + if (!valueField.isValidFor(wireType)) { + _reportIncompatibleType(valueField, wireType); + } + if (valueField.type == FieldType.MESSAGE) { + // pushes value context (under the entry); entry is popped later, in _checkEnd + return _readNextValue(valueField.type, STATE_NESTED_KEY); + } + JsonToken t = _readNextValue(valueField.type, STATE_MAP_VALUE); + _popMapEntry(); + return t; + } + + /** + * Pops the current {@code map} entry context back to the enclosing map context, ready + * to read the next entry (or end the map). + */ + private void _popMapEntry() + { + ProtobufReadContext mapCtxt = _parsingContext.getParent(); + _parsingContext = mapCtxt; + _currentMessage = mapCtxt.getMessageType(); + _currentEndOffset = mapCtxt.getEndOffset(); + _currentField = mapCtxt.getField(); + _state = STATE_MAP_ENTRY_OTHER; + } + + /** + * Reads a {@code map} key value (tag 1 already consumed) and renders it as the String + * used for the JSON field name. Key types are restricted to integral / {@code bool} / + * {@code string} (validated during schema resolution). + */ + private String _readMapKeyValue(ProtobufField keyField, int wireType) throws IOException + { + switch (keyField.type) { + case STRING: + return _readStringKey(); + case BOOLEAN: + if (_inputPtr >= _inputEnd) { + loadMoreGuaranteed(); + } + return (_inputBuffer[_inputPtr++] != 0) ? "true" : "false"; + case VINT32_Z: + return Integer.toString(ProtobufUtil.zigzagDecode(_decodeVInt())); + case VINT32_STD: + return Integer.toString(_decodeVInt()); + case FIXINT32: + return Integer.toString(_decode32Bits()); + case VINT64_Z: + return Long.toString(ProtobufUtil.zigzagDecode(_decodeVLong())); + case VINT64_STD: + return Long.toString(_decodeVLong()); + case FIXINT64: + return Long.toString(_decode64Bits()); + default: + _reportError("Unsupported `map` key type "+keyField.type+" for field '"+keyField.name+"'"); + return null; // never reached + } + } + + /** + * Proto3 default key name for an entry whose {@code key} is absent on the wire. + */ + private String _defaultMapKeyName(ProtobufField keyField) + { + switch (keyField.type) { + case STRING: + return ""; + case BOOLEAN: + return "false"; + default: // all integral key types + return "0"; + } + } + + /** + * Reads a length-prefixed UTF-8 string (used for {@code string} map keys) into a Java + * String, handling values that span input-buffer boundaries. + */ + private String _readStringKey() throws IOException + { + final int len = _decodeLength(); + if (len == 0) { + return ""; + } + if ((_inputPtr + len) <= _inputEnd) { + return _finishShortText(len); + } + if (len >= _inputBuffer.length) { + _finishLongText(len); + return _textBuffer.contentsAsString(); + } + _loadToHaveAtLeast(len); + return _finishShortText(len); + } + + /** + * Produces the proto3 default value token for a {@code map} entry whose {@code value} + * is absent on the wire, setting up numeric/text/binary state as needed. + */ + private JsonToken _mapDefaultValueToken(ProtobufField valueField) throws IOException + { + switch (valueField.type) { + case DOUBLE: + _numberDouble = 0.0d; + _numTypesValid = NR_DOUBLE; + return JsonToken.VALUE_NUMBER_FLOAT; + case FLOAT: + _numberFloat = 0.0f; + _numTypesValid = NR_FLOAT; + return JsonToken.VALUE_NUMBER_FLOAT; + case VINT32_Z: + case VINT32_STD: + case FIXINT32: + _numberInt = 0; + _numTypesValid = NR_INT; + return JsonToken.VALUE_NUMBER_INT; + case VINT64_Z: + case VINT64_STD: + case FIXINT64: + _numberLong = 0L; + _numTypesValid = NR_LONG; + return JsonToken.VALUE_NUMBER_INT; + case BOOLEAN: + return JsonToken.VALUE_FALSE; + case STRING: + _textBuffer.resetWithEmpty(); + return JsonToken.VALUE_STRING; + case BYTES: + _binaryValue = ByteArrayBuilder.NO_BYTES; + return JsonToken.VALUE_EMBEDDED_OBJECT; + case ENUM: + if (valueField.isStdEnum) { + _numberInt = 0; + _numTypesValid = NR_INT; + return JsonToken.VALUE_NUMBER_INT; + } + { + String enumStr = valueField.findEnumByIndex(0); + if (enumStr == null) { + _numberInt = 0; + _numTypesValid = NR_INT; + return JsonToken.VALUE_NUMBER_INT; + } + _textBuffer.resetWithString(enumStr); + return JsonToken.VALUE_STRING; + } + case MESSAGE: + default: + return JsonToken.VALUE_NULL; + } + } + /* /********************************************************** /* Public API, traversal, nextXxxValue/nextFieldName @@ -1078,19 +1385,8 @@ public boolean nextFieldName(SerializableString sstr) throws IOException _reportIncompatibleType(_currentField, wireType); } - // array? - if (_currentField.repeated) { - // 03-Jul-2026, tatu: [dataformats-binary#134] Decide packed-vs-unpacked from - // the actual wire type, not just the schema's declared `packed` flag: - // proto3 permits either encoding for repeated scalar/enum fields. - if (_currentField.isPackedInWire(wireType)) { - _state = STATE_ARRAY_START_PACKED; - } else { - _state = STATE_ARRAY_START; - } - } else { - _state = STATE_ROOT_VALUE; - } + // map / array / single value? + _state = _stateAfterFieldName(_currentField, wireType, STATE_ROOT_VALUE); _updateToken(JsonToken.FIELD_NAME); return name.equals(sstr.getValue()); } @@ -1117,19 +1413,8 @@ public boolean nextFieldName(SerializableString sstr) throws IOException _reportIncompatibleType(_currentField, wireType); } - // array? - if (_currentField.repeated) { - // 03-Jul-2026, tatu: [dataformats-binary#134] Decide packed-vs-unpacked from - // the actual wire type, not just the schema's declared `packed` flag: - // proto3 permits either encoding for repeated scalar/enum fields. - if (_currentField.isPackedInWire(wireType)) { - _state = STATE_ARRAY_START_PACKED; - } else { - _state = STATE_ARRAY_START; - } - } else { - _state = STATE_NESTED_VALUE; - } + // map / array / single value? + _state = _stateAfterFieldName(_currentField, wireType, STATE_NESTED_VALUE); _updateToken(JsonToken.FIELD_NAME); return name.equals(sstr.getValue()); } @@ -1169,19 +1454,8 @@ public String nextFieldName() throws IOException _reportIncompatibleType(_currentField, wireType); } - // array? - if (_currentField.repeated) { - // 03-Jul-2026, tatu: [dataformats-binary#134] Decide packed-vs-unpacked from - // the actual wire type, not just the schema's declared `packed` flag: - // proto3 permits either encoding for repeated scalar/enum fields. - if (_currentField.isPackedInWire(wireType)) { - _state = STATE_ARRAY_START_PACKED; - } else { - _state = STATE_ARRAY_START; - } - } else { - _state = STATE_ROOT_VALUE; - } + // map / array / single value? + _state = _stateAfterFieldName(_currentField, wireType, STATE_ROOT_VALUE); _updateToken(JsonToken.FIELD_NAME); return name; } @@ -1211,19 +1485,8 @@ public String nextFieldName() throws IOException _reportIncompatibleType(_currentField, wireType); } - // array? - if (_currentField.repeated) { - // 03-Jul-2026, tatu: [dataformats-binary#134] Decide packed-vs-unpacked from - // the actual wire type, not just the schema's declared `packed` flag: - // proto3 permits either encoding for repeated scalar/enum fields. - if (_currentField.isPackedInWire(wireType)) { - _state = STATE_ARRAY_START_PACKED; - } else { - _state = STATE_ARRAY_START; - } - } else { - _state = STATE_NESTED_VALUE; - } + // map / array / single value? + _state = _stateAfterFieldName(_currentField, wireType, STATE_NESTED_VALUE); _updateToken(JsonToken.FIELD_NAME); return name; } @@ -1328,6 +1591,25 @@ public String nextTextValue() throws IOException return _textBuffer.contentsAsString(); } + /** + * Determines the parser state to enter right after surfacing a {@code FIELD_NAME} + * for the given field: a {@code map} opens as an Object, a {@code repeated} field as + * an Array (packed or unpacked per wire type), everything else reads a single value. + * + * @since 2.21.6 [dataformats-binary#712] + */ + private int _stateAfterFieldName(ProtobufField f, int wireType, int scalarValueState) + { + if (f.isMap) { + return STATE_MAP_START; + } + if (f.repeated) { + // [dataformats-binary#134] packed-vs-unpacked from the actual wire type + return f.isPackedInWire(wireType) ? STATE_ARRAY_START_PACKED : STATE_ARRAY_START; + } + return scalarValueState; + } + private final ProtobufField _findField(int id) { ProtobufField f; diff --git a/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufReadContext.java b/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufReadContext.java index 83488cdd4..75ab7542c 100644 --- a/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufReadContext.java +++ b/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufReadContext.java @@ -42,6 +42,22 @@ public final class ProtobufReadContext */ protected int _endOffset; + /** + * Whether this (Object-typed) context represents a {@code map} field: + * its entries are surfaced as key/value pairs of a JSON Object. + * + * @since 2.21.6 [dataformats-binary#712] + */ + protected boolean _inMap; + + /** + * Whether this (Object-typed) context represents a single {@code map} entry + * sub-message (whose {@code key}/{@code value} are surfaced as one Object member). + * + * @since 2.21.6 [dataformats-binary#712] + */ + protected boolean _mapEntry; + /* /********************************************************** /* Simple instance reuse slots @@ -77,6 +93,8 @@ protected void reset(ProtobufMessage messageType, int type, int endOffset) _currentValue = null; _endOffset = endOffset; _field = null; + _inMap = false; + _mapEntry = false; } @Override @@ -121,6 +139,50 @@ public ProtobufReadContext createChildArrayContext(ProtobufField f, int endOffse return ctxt; } + /** + * Factory for the context of a {@code map} field: an Object-typed context + * (carrying the synthetic entry message and the map field itself) whose entries + * are iterated much like an unpacked array, so it inherits the enclosing message's + * end offset. + * + * @since 2.21.6 [dataformats-binary#712] + */ + public ProtobufReadContext createChildMapContext(ProtobufField mapField, int endOffset) + { + // Enclosing context remembers the map field, so it can be replayed once the + // map ends (mirrors createChildArrayContext) + _field = mapField; + final ProtobufMessage entryType = mapField.getMessageType(); + ProtobufReadContext ctxt = _child; + if (ctxt == null) { + _child = ctxt = new ProtobufReadContext(this, entryType, TYPE_OBJECT, endOffset); + } else { + ctxt.reset(entryType, TYPE_OBJECT, endOffset); + } + ctxt._field = mapField; + ctxt._inMap = true; + return ctxt; + } + + /** + * Factory for the context of a single {@code map} entry sub-message. Must be called + * on a map context (see {@link #createChildMapContext}); the entry inherits that + * context's (synthetic entry) message type. + * + * @since 2.21.6 [dataformats-binary#712] + */ + public ProtobufReadContext createChildMapEntryContext(int endOffset) + { + ProtobufReadContext ctxt = _child; + if (ctxt == null) { + _child = ctxt = new ProtobufReadContext(this, _messageType, TYPE_OBJECT, endOffset); + } else { + ctxt.reset(_messageType, TYPE_OBJECT, endOffset); + } + ctxt._mapEntry = true; + return ctxt; + } + public ProtobufReadContext createChildObjectContext(ProtobufMessage messageType, ProtobufField f, int endOffset) { @@ -182,6 +244,16 @@ private void _adjustEnd(int bytesConsumed) { public int getEndOffset() { return _endOffset; } + /** + * @since 2.21.6 [dataformats-binary#712] + */ + public boolean inMap() { return _inMap; } + + /** + * @since 2.21.6 [dataformats-binary#712] + */ + public boolean isMapEntry() { return _mapEntry; } + public ProtobufMessage getMessageType() { return _messageType; } public ProtobufField getField() { return _field; } diff --git a/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufWriteContext.java b/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufWriteContext.java index eb6df991d..21b62a34a 100644 --- a/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufWriteContext.java +++ b/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufWriteContext.java @@ -27,6 +27,24 @@ public class ProtobufWriteContext */ protected Object _currentValue; + /** + * Whether this (Object-typed) context represents a {@code map} field + * being written: entries are emitted as repeated length-delimited sub-messages + * rather than as a single sub-message. + * + * @since 2.21.6 [dataformats-binary#712] + */ + protected boolean _inMap; + + /** + * For a map context ({@link #_inMap}): whether a map entry sub-message is + * currently being buffered and still needs its length prefix finalized (which + * happens when the next key, or the end of the map, is written). + * + * @since 2.21.6 [dataformats-binary#712] + */ + protected boolean _entryOpen; + /* /********************************************************** /* Simple instance reuse slots; speed up things @@ -58,6 +76,8 @@ private void reset(int type, ProtobufMessage msg, ProtobufField f) { _message = msg; _field = f; _currentValue = null; + _inMap = false; + _entryOpen = false; } // // // Factory methods @@ -85,6 +105,29 @@ public ProtobufWriteContext createChildArrayContext() { return ctxt; } + /** + * Factory method for the context of a {@code map} field: an Object-typed + * context that carries the map field itself (so its entry key/value fields stay + * reachable) and is flagged as a map. + * + * @since 2.21.6 [dataformats-binary#712] + */ + public ProtobufWriteContext createChildMapContext(ProtobufField mapField) { + // Carry the synthetic entry message so getMessageType() is correct while + // entries are written (e.g. after a message-valued entry closes). + final ProtobufMessage entryType = mapField.getMessageType(); + ProtobufWriteContext ctxt = _child; + if (ctxt == null) { + _child = ctxt = new ProtobufWriteContext(TYPE_OBJECT, this, entryType); + } else { + ctxt.reset(TYPE_OBJECT, entryType, null); + } + ctxt._field = mapField; + ctxt._inMap = true; + ctxt._entryOpen = false; + return ctxt; + } + public ProtobufWriteContext createChildObjectContext(ProtobufMessage type) { ProtobufWriteContext ctxt = _child; if (ctxt == null) { @@ -133,6 +176,23 @@ public ProtobufMessage getMessageType() { public boolean notArray() { return _type != TYPE_ARRAY; } + /** + * @return Whether this context represents a {@code map} field being written. + * + * @since 2.21.6 [dataformats-binary#712] + */ + public boolean inMap() { return _inMap; } + + /** + * @since 2.21.6 [dataformats-binary#712] + */ + public boolean isEntryOpen() { return _entryOpen; } + + /** + * @since 2.21.6 [dataformats-binary#712] + */ + public void setEntryOpen(boolean state) { _entryOpen = state; } + public StringBuilder appendDesc(StringBuilder sb) { if (_parent != null) { sb = _parent.appendDesc(sb); diff --git a/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/schema/FileDescriptorSet.java b/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/schema/FileDescriptorSet.java index 4b74d6b3b..790376a77 100644 --- a/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/schema/FileDescriptorSet.java +++ b/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/schema/FileDescriptorSet.java @@ -181,6 +181,13 @@ public MessageElement buildMessageElement() { MessageElement.Builder messageElementBuilder = MessageElement.builder(); messageElementBuilder.name(name); + // [dataformats-binary#712] protoc desugars `map` into a nested entry + // message flagged `map_entry`; carry that through so the resolver can + // re-expose the enclosing `repeated` field idiomatically as a map. + if (options != null && options.map_entry) { + messageElementBuilder.addOption( + OptionElement.create("map_entry", OptionElement.Kind.BOOLEAN, Boolean.TRUE)); + } // fields if (field != null) { for (FieldDescriptorProto f : field) { diff --git a/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/schema/ProtobufField.java b/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/schema/ProtobufField.java index b0a4380e0..131b8ea64 100644 --- a/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/schema/ProtobufField.java +++ b/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/schema/ProtobufField.java @@ -58,6 +58,31 @@ public class ProtobufField public final boolean isStdEnum; + /** + * Whether this field is a {@code map} field: encoded on the wire exactly + * like a {@code repeated} length-delimited "entry" sub-message (key = tag 1, + * value = tag 2), but exposed to databind as a JSON Object. + * + * @since 2.21.6 [dataformats-binary#712] + */ + public final boolean isMap; + + /** + * For {@code map} fields: the synthetic "key" field (tag 1) of the entry + * sub-message; {@code null} for non-map fields. + * + * @since 2.21.6 [dataformats-binary#712] + */ + protected final ProtobufField _keyField; + + /** + * For {@code map} fields: the synthetic "value" field (tag 2) of the entry + * sub-message; {@code null} for non-map fields. + * + * @since 2.21.6 [dataformats-binary#712] + */ + protected final ProtobufField _valueField; + public ProtobufField(FieldElement nativeField, FieldType type) { this(nativeField, type, false); } @@ -87,12 +112,48 @@ public static ProtobufField unknownField() { return new ProtobufField(null, FieldType.MESSAGE, null, null, false); } + /** + * Constructor for {@code map} fields. Such a field is always encoded as a + * {@code repeated}, length-delimited "entry" sub-message (regardless of the + * label its declaration carries after preprocessing), so it is set up as a + * repeated {@link FieldType#MESSAGE} pointing at the synthetic entry type, + * additionally flagged with {@link #isMap}. + * + * @since 2.21.6 [dataformats-binary#712] + */ + public ProtobufField(FieldElement nativeField, ProtobufMessage entryType, + ProtobufField keyField, ProtobufField valueField) + { + type = FieldType.MESSAGE; + wireType = FieldType.MESSAGE.getWireType(); // LENGTH_PREFIXED + usesZigZag = false; + enumValues = EnumLookup.empty(); + isStdEnum = false; + messageType = entryType; + isMap = true; + _keyField = keyField; + _valueField = valueField; + + id = nativeField.tag(); + name = nativeField.name(); + required = false; + repeated = true; + packed = false; + deprecated = Boolean.TRUE.equals(_findBooleanOptionValue(nativeField, "deprecated")); + // repeated, non-packed length-delimited: one tag per entry + typedTag = (id << 3) + wireType; + isObject = true; + } + protected ProtobufField(FieldElement nativeField, FieldType type, ProtobufMessage msg, ProtobufEnum et, boolean isProto3) { this.type = type; wireType = type.getWireType(); usesZigZag = type.usesZigZag(); + isMap = false; + _keyField = null; + _valueField = null; if (et == null) { enumValues = EnumLookup.empty(); isStdEnum = false; @@ -179,6 +240,26 @@ public final ProtobufMessage getMessageType() { return messageType; } + /** + * @return For {@code map} fields, the synthetic "key" field (tag 1) of the + * entry sub-message; {@code null} otherwise. + * + * @since 2.21.6 [dataformats-binary#712] + */ + public final ProtobufField getKeyField() { + return _keyField; + } + + /** + * @return For {@code map} fields, the synthetic "value" field (tag 2) of the + * entry sub-message; {@code null} otherwise. + * + * @since 2.21.6 [dataformats-binary#712] + */ + public final ProtobufField getValueField() { + return _valueField; + } + public final ProtobufField nextOrThisIf(int idToMatch) { if ((next != null) && (next.id == idToMatch)) { return next; diff --git a/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/schema/ProtobufMessage.java b/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/schema/ProtobufMessage.java index 318cb9d14..eaf0567b4 100644 --- a/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/schema/ProtobufMessage.java +++ b/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/schema/ProtobufMessage.java @@ -35,6 +35,15 @@ public class ProtobufMessage protected int _idOffset = -1; + /** + * Whether this message is a synthetic {@code map} entry type (as emitted by + * {@code protoc} into descriptor sets, carrying the {@code map_entry} option): + * a {@code repeated} field of such a type is re-exposed idiomatically as a map. + * + * @since 2.21.6 [dataformats-binary#712] + */ + protected boolean _isMapEntry; + public ProtobufMessage(String name, ProtobufField[] fields) { _name = name; @@ -85,6 +94,16 @@ public static ProtobufMessage bogusMessage(String desc) { return bogus; } + /** + * @since 2.21.6 [dataformats-binary#712] + */ + public boolean isMapEntry() { return _isMapEntry; } + + /** + * @since 2.21.6 [dataformats-binary#712] + */ + public void markAsMapEntry() { _isMapEntry = true; } + public ProtobufField firstField() { return _firstField; } public ProtobufField firstIf(String name) { diff --git a/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/schema/ProtobufSchemaPreprocessor.java b/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/schema/ProtobufSchemaPreprocessor.java index 65a3646e5..d91f61407 100644 --- a/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/schema/ProtobufSchemaPreprocessor.java +++ b/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/schema/ProtobufSchemaPreprocessor.java @@ -35,10 +35,10 @@ *

* NOTE: {@code map} fields are also label-less (in both {@code proto2} and * {@code proto3}), so a synthetic label is injected for them as well, purely so - * they parse. Actually rejecting them -- full map support is not yet implemented - * (see #708) - * -- is a semantic concern handled downstream during type resolution - * ({@link TypeResolver}), not here. + * they parse. Turning a {@code map} field into its actual (repeated entry) form is a + * semantic concern handled downstream during type resolution ({@link TypeResolver}, + * see #712), + * not here. * * @since 2.21.6 */ @@ -69,8 +69,9 @@ private ProtobufSchemaPreprocessor(String schema) { /** * Rewrites given schema so that {@code proto3} label-less fields -- and * label-less {@code map} fields in either syntax -- parse with the - * bundled parser. (Map fields are then rejected during type resolution; see - * the class comment.) Schemas that need no rewriting are returned unchanged. + * bundled parser. (Map fields are then resolved to their entry form during type + * resolution; see the class comment.) Schemas that need no rewriting are returned + * unchanged. */ public static String preprocess(String schema) { // Fast path: label injection only matters for proto3, and map handling @@ -166,7 +167,7 @@ private String _process() { // other label-less fields occur only in proto3 (proto2 requires // explicit labels, so a label-less field there is a genuine error). // Injecting a label lets the field parse; `map` fields are then - // rejected downstream during type resolution (see #708). + // resolved to their entry form during type resolution (see #712). boolean labelless = "map".equals(word) || (isProto3 && !NON_FIELD_KEYWORDS.contains(word)); if (labelless) { diff --git a/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/schema/TypeResolver.java b/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/schema/TypeResolver.java index 14d79c0c3..9b343a3a6 100644 --- a/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/schema/TypeResolver.java +++ b/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/schema/TypeResolver.java @@ -160,6 +160,12 @@ protected ProtobufMessage _resolve(MessageElement rawType) ProtobufField[] resolvedFields = new ProtobufField[rawFields.size()]; ProtobufMessage message = new ProtobufMessage(rawType.name(), resolvedFields); + // 15-Jul-2026, tatu: [dataformats-binary#712] protoc descriptor sets desugar + // `map` into a nested entry message flagged `map_entry`; note that here + // so a `repeated` field of this type can be re-exposed as a map (below). + if (_hasMapEntryOption(rawType)) { + message.markAsMapEntry(); + } // Important: add type itself as (being) resolved, to allow for self- and cyclic refs if (_parent != null) { // 09-Jul-2021, tatu: LGTM suggestion -- can it ever be null?! _parent.addResolvedMessageType(rawType.name(), message); @@ -204,18 +210,23 @@ protected ProtobufMessage _resolve(MessageElement rawType) } } } else if (fieldType instanceof DataType.MapType) { - // 10-Jul-2026, tatu: [dataformats-binary#708] `map` fields parse (a - // synthetic label is injected upstream) but full map support is - // not yet implemented; reject here with a clear error. - throw new IllegalArgumentException(String.format( - "Unsupported `map` field '%s' in MessageType '%s': `map` type is not yet" - + " supported by jackson-dataformats-binary" - + " (see https://github.com/FasterXML/jackson-dataformats-binary/issues/708)", - f.name(), rawType.name())); + // 15-Jul-2026, tatu: [dataformats-binary#712] `map` is encoded + // exactly like a `repeated` entry sub-message; synthesize that entry + // type and expose the field as a map. + pbf = _resolveMapField(f, (DataType.MapType) fieldType, rawType); } else { throw new IllegalArgumentException(String.format( "Unrecognized DataType '%s' for field '%s'", fieldType.getClass().getName(), f.name())); } + // [dataformats-binary#712] A `repeated Entry` field whose entry message + // is `map_entry`-flagged (from a protoc descriptor set) is really a map; + // re-expose it idiomatically, matching the `.proto` `map` path. + if (pbf.repeated && !pbf.isMap && (pbf.type == FieldType.MESSAGE)) { + final ProtobufMessage entryMsg = pbf.getMessageType(); + if ((entryMsg != null) && entryMsg.isMapEntry()) { + pbf = new ProtobufField(f, entryMsg, entryMsg.field(1), entryMsg.field(2)); + } + } resolvedFields[ix++] = pbf; } ProtobufField first = (resolvedFields.length == 0) ? null : resolvedFields[0]; @@ -231,6 +242,99 @@ protected ProtobufMessage _resolve(MessageElement rawType) return message; } + /** + * Resolves a {@code map} field by synthesizing the "entry" sub-message + * ({@code key} = tag 1, {@code value} = tag 2) that protobuf uses to encode maps + * on the wire, then wrapping it in a repeated, map-flagged {@link ProtobufField}. + * + * @since 2.21.6 [dataformats-binary#712] + */ + private ProtobufField _resolveMapField(FieldElement f, DataType.MapType mapType, + MessageElement rawType) + { + final DataType keyType = mapType.keyType(); + final DataType valueType = mapType.valueType(); + _verifyMapKeyType(keyType, f, rawType); + + // Build the synthetic entry message: `message XxxEntry { key = 1; value = 2; }`. + // Both are proto3 "singular" (OPTIONAL label) fields; resolving through the + // normal machinery reuses all scalar / enum / message / nested type handling, + // and resolves the value type against this (the enclosing) scope. + final String entryName = _mapEntryName(f.name()); + final MessageElement entryElem = MessageElement.builder() + .name(entryName) + .addField(FieldElement.builder() + .name("key").tag(1) + .label(FieldElement.Label.OPTIONAL) + .type(keyType) + .build()) + .addField(FieldElement.builder() + .name("value").tag(2) + .label(FieldElement.Label.OPTIONAL) + .type(valueType) + .build()) + .build(); + final ProtobufMessage entryMsg = resolve(this, entryElem); + return new ProtobufField(f, entryMsg, entryMsg.field(1), entryMsg.field(2)); + } + + /** + * @return Whether given message declaration carries the {@code map_entry} option + * (set by {@code protoc} on the synthetic entry type of a {@code map} field). + * + * @since 2.21.6 [dataformats-binary#712] + */ + private static boolean _hasMapEntryOption(MessageElement rawType) + { + for (OptionElement opt : rawType.options()) { + if ("map_entry".equals(opt.name())) { + Object v = opt.value(); + if (v instanceof Boolean) { + return ((Boolean) v).booleanValue(); + } + return "true".equals(String.valueOf(v).trim()); + } + } + return false; + } + + /** + * Verifies that the key type of a {@code map} field is one protobuf permits: + * any integral type, {@code bool} or {@code string} (not floating-point, {@code bytes}, + * enum or message). + */ + private void _verifyMapKeyType(DataType keyType, FieldElement f, MessageElement rawType) + { + if (keyType instanceof DataType.ScalarType) { + switch ((DataType.ScalarType) keyType) { + case DOUBLE: + case FLOAT: + case BYTES: + case ANY: + break; // not allowed as key -- fall through to throw + default: + return; // integral types, bool and string are all valid keys + } + } + throw new IllegalArgumentException(String.format( + "Illegal key type (%s) for `map` field '%s' in MessageType '%s': protobuf" + + " map keys must be an integral type, `bool` or `string`", + keyType, f.name(), rawType.name())); + } + + /** + * Derives the name of the synthetic entry message for a {@code map} field, mirroring + * the {@code Entry} convention used by {@code protoc}. Only used for + * diagnostics and internal type lookup, so exact CamelCase-ing is not essential. + */ + private static String _mapEntryName(String fieldName) + { + if (fieldName == null || fieldName.isEmpty()) { + return "Entry"; + } + return Character.toUpperCase(fieldName.charAt(0)) + fieldName.substring(1) + "Entry"; + } + protected void addResolvedMessageType(String name, ProtobufMessage toResolve) { if (_resolvedMessageTypes.isEmpty()) { _resolvedMessageTypes = new HashMap(); diff --git a/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/MapField712Test.java b/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/MapField712Test.java new file mode 100644 index 000000000..2b824f0a2 --- /dev/null +++ b/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/MapField712Test.java @@ -0,0 +1,371 @@ +package com.fasterxml.jackson.dataformat.protobuf; + +import java.io.ByteArrayInputStream; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.TreeMap; + +import org.junit.jupiter.api.Test; + +import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.core.JsonToken; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.dataformat.protobuf.schema.ProtobufField; +import com.fasterxml.jackson.dataformat.protobuf.schema.ProtobufSchema; +import com.fasterxml.jackson.dataformat.protobuf.schema.ProtobufSchemaLoader; + +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; + +/** + * Tests for idiomatic {@code map} support: [dataformats-binary#712]. + */ +public class MapField712Test extends ProtobufTestBase +{ + private final ProtobufMapper MAPPER = newObjectMapper(); + + // POJOs for databind round-trips + static class Counts { + public Map counts; + public String name; + } + static class Val { + public int x; + public String y; + protected Val() { } + public Val(int x, String y) { this.x = x; this.y = y; } + } + static class MsgMap { + public Map m; + } + + /* + /********************************************************** + /* Schema resolution + /********************************************************** + */ + + @Test + public void testMapSchemaResolution() throws Exception + { + ProtobufSchema schema = ProtobufSchemaLoader.std.parse( + "syntax = \"proto3\";\n" + + "message Msg {\n" + + " map counts = 1;\n" + + " string name = 2;\n" + + "}\n", "Msg"); + ProtobufField f = schema.getRootType().field("counts"); + assertNotNull(f); + assertTrue(f.isMap); + assertTrue(f.repeated); // encoded as repeated entries + assertNotNull(f.getKeyField()); + assertNotNull(f.getValueField()); + } + + @Test + public void testInvalidMapKeyTypeRejected() throws Exception + { + for (String badKey : new String[] { "double", "float", "bytes" }) { + final String proto = "syntax = \"proto3\";\n" + + "message Msg {\n" + + " map<" + badKey + ", int32> m = 1;\n" + + "}\n"; + try { + ProtobufSchemaLoader.std.parse(proto, "Msg"); + fail("Should not accept `map` with key type `" + badKey + "`"); + } catch (IllegalArgumentException e) { + verifyException(e, "map keys must be"); + } + } + } + + /* + /********************************************************** + /* Streaming round-trips + /********************************************************** + */ + + @Test + public void testStringToIntMapTokens() throws Exception + { + ProtobufSchema schema = ProtobufSchemaLoader.std.parse( + "syntax = \"proto3\";\n" + + "message Msg {\n" + + " map counts = 1;\n" + + " string name = 2;\n" + + "}\n", "Msg"); + Map root = new LinkedHashMap<>(); + Map counts = new LinkedHashMap<>(); + counts.put("a", 1); + counts.put("b", 2); + root.put("counts", counts); + root.put("name", "x"); + byte[] doc = MAPPER.writer(schema).writeValueAsBytes(root); + + try (JsonParser p = MAPPER.getFactory().createParser(doc)) { + p.setSchema(schema); + assertToken(JsonToken.START_OBJECT, p.nextToken()); + assertToken(JsonToken.FIELD_NAME, p.nextToken()); + assertEquals("counts", p.currentName()); + assertToken(JsonToken.START_OBJECT, p.nextToken()); + assertToken(JsonToken.FIELD_NAME, p.nextToken()); + assertEquals("a", p.currentName()); + assertToken(JsonToken.VALUE_NUMBER_INT, p.nextToken()); + assertEquals(1, p.getIntValue()); + assertToken(JsonToken.FIELD_NAME, p.nextToken()); + assertEquals("b", p.currentName()); + assertToken(JsonToken.VALUE_NUMBER_INT, p.nextToken()); + assertEquals(2, p.getIntValue()); + assertToken(JsonToken.END_OBJECT, p.nextToken()); + assertToken(JsonToken.FIELD_NAME, p.nextToken()); + assertEquals("name", p.currentName()); + assertToken(JsonToken.VALUE_STRING, p.nextToken()); + assertEquals("x", p.getText()); + assertToken(JsonToken.END_OBJECT, p.nextToken()); + assertEquals(null, p.nextToken()); + } + } + + @Test + public void testMapDatabindRoundtrip() throws Exception + { + ProtobufSchema schema = ProtobufSchemaLoader.std.parse( + "syntax = \"proto3\";\n" + + "message Counts {\n" + + " map counts = 1;\n" + + " string name = 2;\n" + + "}\n", "Counts"); + Counts input = new Counts(); + input.counts = new LinkedHashMap<>(); + input.counts.put("a", 1); + input.counts.put("bb", 22); + input.name = "hi"; + + byte[] doc = MAPPER.writer(schema).writeValueAsBytes(input); + Counts result = MAPPER.readerFor(Counts.class).with(schema).readValue(doc); + assertEquals(input.counts, result.counts); + assertEquals("hi", result.name); + } + + @Test + public void testNonStringKeyMaps() throws Exception + { + // int32, int64, bool keys + _verifyKeyRoundtrip("int32", "int32", "7", 7); + _verifyKeyRoundtrip("int64", "int64", "100000000000", 100000000000L); + _verifyKeyRoundtrip("sint32", "int32", "-5", -5); + _verifyKeyRoundtrip("bool", "int32", "true", 1); + } + + private void _verifyKeyRoundtrip(String keyType, String valType, + String key, Object expectedNumKey) throws Exception + { + ProtobufSchema schema = ProtobufSchemaLoader.std.parse( + "syntax = \"proto3\";\n" + + "message Msg {\n" + + " map<" + keyType + ", " + valType + "> m = 1;\n" + + "}\n", "Msg"); + Map root = new LinkedHashMap<>(); + Map m = new LinkedHashMap<>(); + m.put(key, 42); + root.put("m", m); + byte[] doc = MAPPER.writer(schema).writeValueAsBytes(root); + + JsonNode tree = MAPPER.readerFor(JsonNode.class).with(schema).readValue(doc); + assertEquals(1, tree.get("m").size()); + assertEquals(42, tree.get("m").get(key).asInt()); + } + + @Test + public void testMessageValuedMap() throws Exception + { + ProtobufSchema schema = ProtobufSchemaLoader.std.parse( + "syntax = \"proto3\";\n" + + "message Val { int32 x = 1; string y = 2; }\n" + + "message MsgMap { map m = 1; }\n", "MsgMap"); + MsgMap input = new MsgMap(); + input.m = new LinkedHashMap<>(); + input.m.put(1L, new Val(10, "one")); + input.m.put(2L, new Val(20, "two")); + input.m.put(3L, new Val(30, "three")); + + byte[] doc = MAPPER.writer(schema).writeValueAsBytes(input); + MsgMap result = MAPPER.readerFor(MsgMap.class).with(schema).readValue(doc); + assertEquals(3, result.m.size()); + assertEquals(20, result.m.get(2L).x); + assertEquals("three", result.m.get(3L).y); + } + + @Test + public void testEmptyMapWritesNothing() throws Exception + { + ProtobufSchema schema = ProtobufSchemaLoader.std.parse( + "syntax = \"proto3\";\n" + + "message Msg {\n" + + " map counts = 1;\n" + + " string name = 2;\n" + + "}\n", "Msg"); + Map root = new LinkedHashMap<>(); + root.put("counts", new LinkedHashMap<>()); + root.put("name", "y"); + byte[] doc = MAPPER.writer(schema).writeValueAsBytes(root); + + // Empty map => field entirely absent on the wire; only "name" (tag 2, len 1, 'y') + assertArrayEquals(new byte[] { 0x12, 0x01, 0x79 }, doc); + + JsonNode tree = MAPPER.readerFor(JsonNode.class).with(schema).readValue(doc); + assertTrue(tree.path("counts").isMissingNode() || tree.path("counts").size() == 0); + assertEquals("y", tree.get("name").asText()); + } + + @Test + public void testMapNestedInRepeatedMessage() throws Exception + { + ProtobufSchema schema = ProtobufSchemaLoader.std.parse( + "syntax = \"proto3\";\n" + + "message Inner { map m = 1; }\n" + + "message Outer { repeated Inner items = 1; }\n", "Outer"); + Map root = new LinkedHashMap<>(); + java.util.List items = new java.util.ArrayList<>(); + for (int i = 0; i < 3; ++i) { + Map inner = new LinkedHashMap<>(); + Map m = new LinkedHashMap<>(); + m.put("k" + i, i); + inner.put("m", m); + items.add(inner); + } + root.put("items", items); + byte[] doc = MAPPER.writer(schema).writeValueAsBytes(root); + + JsonNode tree = MAPPER.readerFor(JsonNode.class).with(schema).readValue(doc); + assertEquals(3, tree.get("items").size()); + assertEquals(2, tree.get("items").get(2).get("m").get("k2").asInt()); + } + + @Test + public void testProto2Map() throws Exception + { + ProtobufSchema schema = ProtobufSchemaLoader.std.parse( + "syntax = \"proto2\";\n" + + "message Msg {\n" + + " map counts = 1;\n" + + "}\n", "Msg"); + Map root = new LinkedHashMap<>(); + Map counts = new LinkedHashMap<>(); + counts.put("a", 1); + root.put("counts", counts); + byte[] doc = MAPPER.writer(schema).writeValueAsBytes(root); + + JsonNode tree = MAPPER.readerFor(JsonNode.class).with(schema).readValue(doc); + assertEquals(1, tree.get("counts").get("a").asInt()); + } + + /* + /********************************************************** + /* Wire-format and interoperability checks + /********************************************************** + */ + + @Test + public void testKnownWireEncoding() throws Exception + { + ProtobufSchema schema = ProtobufSchemaLoader.std.parse( + "syntax = \"proto3\";\n" + + "message Msg { map counts = 1; }\n", "Msg"); + Map root = new LinkedHashMap<>(); + Map counts = new TreeMap<>(); + counts.put("a", 1); + counts.put("b", 2); + root.put("counts", counts); + byte[] doc = MAPPER.writer(schema).writeValueAsBytes(root); + + // Two entries; each: tag 1 (0x0a) | len 5 | key(0a 01 ) | value(10 ) + byte[] expected = { + 0x0a, 0x05, 0x0a, 0x01, 0x61, 0x10, 0x01, // {"a":1} + 0x0a, 0x05, 0x0a, 0x01, 0x62, 0x10, 0x02 // {"b":2} + }; + assertArrayEquals(expected, doc); + } + + // A value written before the key (or an absent key) must decode using proto3 + // defaults, matching what other protobuf libraries can emit. + @Test + public void testAbsentKeyAndValueDefaults() throws Exception + { + ProtobufSchema schema = ProtobufSchemaLoader.std.parse( + "syntax = \"proto3\";\n" + + "message Msg { map counts = 1; }\n", "Msg"); + + // Entry with only key present (value absent -> default 0): 0a 03 [0a 01 6b] + JsonNode t1 = MAPPER.readerFor(JsonNode.class).with(schema) + .readValue(new byte[] { 0x0a, 0x03, 0x0a, 0x01, 0x6b }); + assertEquals(0, t1.get("counts").get("k").asInt()); + + // Entry with only value present (key absent -> default ""): 0a 02 [10 05] + JsonNode t2 = MAPPER.readerFor(JsonNode.class).with(schema) + .readValue(new byte[] { 0x0a, 0x02, 0x10, 0x05 }); + assertEquals(5, t2.get("counts").get("").asInt()); + } + + @Test + public void testUnknownFieldInsideEntrySkipped() throws Exception + { + ProtobufSchema schema = ProtobufSchemaLoader.std.parse( + "syntax = \"proto3\";\n" + + "message Msg { map counts = 1; }\n", "Msg"); + // Entry {key="k", value=1} with an extra unknown field id 3 (varint 9) interleaved: + // key: 0a 01 6b | unknown(id3,varint): 18 09 | value: 10 01 -> body len 7 + byte[] doc = { 0x0a, 0x07, 0x0a, 0x01, 0x6b, 0x18, 0x09, 0x10, 0x01 }; + JsonNode tree = MAPPER.readerFor(JsonNode.class).with(schema).readValue(doc); + assertEquals(1, tree.get("counts").get("k").asInt()); + } + + // Large map read from an InputStream, forcing buffer reloads mid-map + @Test + public void testLargeMapAcrossBufferReload() throws Exception + { + ProtobufSchema schema = ProtobufSchemaLoader.std.parse( + "syntax = \"proto3\";\n" + + "message Big { map m = 1; }\n", "Big"); + final int COUNT = 2000; + Map root = new LinkedHashMap<>(); + Map big = new LinkedHashMap<>(); + for (int i = 0; i < COUNT; ++i) { + big.put(String.valueOf(i), "value-string-number-" + i); + } + root.put("m", big); + byte[] doc = MAPPER.writer(schema).writeValueAsBytes(root); + assertTrue(doc.length > 8000, "doc should exceed one input buffer"); + + JsonNode tree = MAPPER.readerFor(JsonNode.class).with(schema) + .readValue(new ByteArrayInputStream(doc)); + assertEquals(COUNT, tree.get("m").size()); + for (int i = 0; i < COUNT; ++i) { + assertEquals("value-string-number-" + i, + tree.get("m").get(String.valueOf(i)).asText()); + } + } + + @Test + public void testWriteStartArrayOnMapRejected() throws Exception + { + ProtobufSchema schema = ProtobufSchemaLoader.std.parse( + "syntax = \"proto3\";\n" + + "message Msg { map counts = 1; }\n", "Msg"); + java.io.ByteArrayOutputStream bytes = new java.io.ByteArrayOutputStream(); + try (com.fasterxml.jackson.core.JsonGenerator g = MAPPER.getFactory().createGenerator(bytes)) { + g.setSchema(schema); + g.writeStartObject(); + g.writeFieldName("counts"); + try { + g.writeStartArray(); + fail("Should not allow START_ARRAY for a map field"); + } catch (Exception e) { + verifyException(e, "is a `map`"); + } + } + } +} diff --git a/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/schema/DescriptorMapField712Test.java b/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/schema/DescriptorMapField712Test.java new file mode 100644 index 000000000..3b28f8d27 --- /dev/null +++ b/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/schema/DescriptorMapField712Test.java @@ -0,0 +1,107 @@ +package com.fasterxml.jackson.dataformat.protobuf.schema; + +import java.util.LinkedHashMap; +import java.util.Map; + +import org.junit.jupiter.api.Test; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.dataformat.protobuf.ProtobufMapper; +import com.fasterxml.jackson.dataformat.protobuf.ProtobufTestBase; +import com.fasterxml.jackson.dataformat.protobuf.schema.FileDescriptorSet.DescriptorProto; +import com.fasterxml.jackson.dataformat.protobuf.schema.FileDescriptorSet.FieldDescriptorProto; +import com.fasterxml.jackson.dataformat.protobuf.schema.FileDescriptorSet.FieldDescriptorProto.Label; +import com.fasterxml.jackson.dataformat.protobuf.schema.FileDescriptorSet.FieldDescriptorProto.Type; +import com.fasterxml.jackson.dataformat.protobuf.schema.FileDescriptorSet.FileDescriptorProto; +import com.fasterxml.jackson.dataformat.protobuf.schema.FileDescriptorSet.MessageOptions; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Verifies that a {@code map} loaded from a {@code protoc} descriptor set -- + * where the map has already been desugared into a nested {@code map_entry} message + * plus a {@code repeated} field -- is re-exposed idiomatically as a map, matching the + * {@code .proto} path. See [dataformats-binary#712]. + */ +public class DescriptorMapField712Test extends ProtobufTestBase +{ + private final ProtobufMapper MAPPER = new ProtobufMapper(); + + private static FieldDescriptorProto field(String name, int num, Label label, + Type type, String typeName) + { + FieldDescriptorProto f = new FieldDescriptorProto(); + f.name = name; + f.number = num; + f.label = label; + f.type = type; + f.type_name = typeName; + return f; + } + + // Builds the descriptor equivalent of: + // message Msg { + // map counts = 1; // => repeated CountsEntry + nested entry + // string name = 2; + // } + private FileDescriptorSet buildDescriptorWithMap() + { + DescriptorProto entry = new DescriptorProto(); + entry.name = "CountsEntry"; + entry.field = new FieldDescriptorProto[] { + field("key", 1, Label.LABEL_OPTIONAL, Type.TYPE_STRING, null), + field("value", 2, Label.LABEL_OPTIONAL, Type.TYPE_INT32, null) + }; + MessageOptions opts = new MessageOptions(); + opts.map_entry = true; + entry.options = opts; + + DescriptorProto msg = new DescriptorProto(); + msg.name = "Msg"; + msg.field = new FieldDescriptorProto[] { + field("counts", 1, Label.LABEL_REPEATED, Type.TYPE_MESSAGE, ".Msg.CountsEntry"), + field("name", 2, Label.LABEL_OPTIONAL, Type.TYPE_STRING, null) + }; + msg.nested_type = new DescriptorProto[] { entry }; + + FileDescriptorProto fdp = new FileDescriptorProto(); + fdp.name = "test.proto"; + fdp.setPackage(""); + fdp.syntax = "proto3"; + fdp.message_type = new DescriptorProto[] { msg }; + + return new FileDescriptorSet(new FileDescriptorProto[] { fdp }); + } + + @Test + public void testDescriptorMapResolvesAsMap() throws Exception + { + ProtobufSchema schema = buildDescriptorWithMap().schemaFor("Msg"); + ProtobufField f = schema.getRootType().field("counts"); + assertNotNull(f); + assertTrue(f.isMap, "'counts' from a map_entry descriptor should resolve as a map"); + assertEquals(FieldType.STRING, f.getKeyField().type); + assertEquals(FieldType.VINT32_STD, f.getValueField().type); + } + + @Test + public void testDescriptorMapRoundtrip() throws Exception + { + ProtobufSchema schema = buildDescriptorWithMap().schemaFor("Msg"); + + Map root = new LinkedHashMap<>(); + Map counts = new LinkedHashMap<>(); + counts.put("x", 7); + counts.put("y", 9); + root.put("counts", counts); + root.put("name", "hi"); + + byte[] doc = MAPPER.writer(schema).writeValueAsBytes(root); + JsonNode tree = MAPPER.readerFor(JsonNode.class).with(schema).readValue(doc); + assertEquals(7, tree.get("counts").get("x").asInt()); + assertEquals(9, tree.get("counts").get("y").asInt()); + assertEquals("hi", tree.get("name").asText()); + } +} diff --git a/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/schema/Proto3LabellessField708Test.java b/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/schema/Proto3LabellessField708Test.java index e4cd16c32..fdfe1f236 100644 --- a/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/schema/Proto3LabellessField708Test.java +++ b/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/schema/Proto3LabellessField708Test.java @@ -182,39 +182,32 @@ public void testProto3MentionInCommentNotDetected() throws Exception } } - // Map fields are not yet supported; must fail with a clear, dedicated message + // Map fields now parse and resolve as `map` fields (see [dataformats-binary#712]) @Test - public void testMapFieldClearError() throws Exception + public void testMapFieldParses() throws Exception { final String proto = "syntax = \"proto3\";\n" + "message Msg {\n" + " map counts = 1;\n" + "}\n"; - try { - ProtobufSchemaLoader.std.parse(proto); - fail("Should not pass: map fields are not yet supported"); - } catch (IllegalArgumentException e) { - verifyException(e, "map"); - verifyException(e, "not yet supported"); - } + ProtobufSchema schema = ProtobufSchemaLoader.std.parse(proto); + ProtobufField f = schema.getRootType().field("counts"); + assertNotNull(f); + assertTrue(f.isMap, "'counts' should be resolved as a map field"); } - // `map` is valid (and label-less) in proto2 too, so the same clear error - // must be raised there -- not protoparser's cryptic "unexpected label: map" + // `map` is valid (and label-less) in proto2 too, and must resolve the same way @Test - public void testProto2MapFieldClearError() throws Exception + public void testProto2MapFieldParses() throws Exception { final String proto = "syntax = \"proto2\";\n" + "message Msg {\n" + " map counts = 1;\n" + "}\n"; - try { - ProtobufSchemaLoader.std.parse(proto); - fail("Should not pass: map fields are not yet supported"); - } catch (IllegalArgumentException e) { - verifyException(e, "map"); - verifyException(e, "not yet supported"); - } + ProtobufSchema schema = ProtobufSchemaLoader.std.parse(proto); + ProtobufField f = schema.getRootType().field("counts"); + assertNotNull(f); + assertTrue(f.isMap, "'counts' should be resolved as a map field"); } // Preprocessing must be applied for stream-based loading too, not just String diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index b5b3c269d..c8b6c6eb2 100644 --- a/release-notes/VERSION-2.x +++ b/release-notes/VERSION-2.x @@ -17,7 +17,8 @@ Active maintainers: 2.21.6 (not yet released) #708: (protobuf) proto3 label-less field declarations fail to parse - (NOTE: `map` fields still not supported, but now fail with a clear error) +#712: (protobuf) `map` type not supported with protobuf: should be supported + idiomatically #714: (protobuf) Packed repeated field fails to decode when array spans an input buffer reload #715: (protobuf) `ProtobufGenerator._reportWrongWireType()` always reports `string`, From f57b4a957cd2982c1aa839f81bbcb505d8b74051 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Fri, 7 Aug 2026 18:52:44 -0700 Subject: [PATCH 2/7] Minor fix --- .../dataformat/protobuf/ProtobufParser.java | 5 ++- .../dataformat/protobuf/MapField712Test.java | 39 +++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufParser.java b/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufParser.java index f236a1aea..3b29c1a86 100644 --- a/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufParser.java +++ b/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufParser.java @@ -1143,7 +1143,10 @@ private JsonToken _openMapEntry() throws IOException _mapValueTag = 0; String keyName = null; while (keyName == null) { - if (_inputPtr >= newEnd) { // empty (or key-less) entry + // NOTE: must test against `_currentEndOffset`, not the local `newEnd`: reads + // below may trigger a buffer reload, which rebases all end offsets (see + // `ProtobufReadContext.adjustEnd()`) and so leaves the local stale. + if (_inputPtr >= _currentEndOffset) { // empty (or key-less) entry keyName = _defaultMapKeyName(keyField); break; } diff --git a/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/MapField712Test.java b/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/MapField712Test.java index 2b824f0a2..1c750552c 100644 --- a/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/MapField712Test.java +++ b/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/MapField712Test.java @@ -349,6 +349,45 @@ public void testLargeMapAcrossBufferReload() throws Exception } } + // The entry-end bound must survive a buffer reload that happens while scanning an + // entry's fields, not just while reading the value: end offsets are rebased on + // reload (see `ProtobufReadContext.adjustEnd()`), so a bound captured beforehand + // goes stale and lets the scan run past the entry into the next one. + @Test + public void testUnknownFieldInEntryAcrossBufferReload() throws Exception + { + ProtobufSchema schema = ProtobufSchemaLoader.std.parse( + "syntax = \"proto3\";\n" + + "message Msg { map counts = 1; string name = 2; }\n", "Msg"); + // Entry carrying only an unknown field (id 3, varint): key and value are both + // absent, so both decode to proto3 defaults. + final byte[] entryUnknownOnly = { 0x0a, 0x02, 0x18, 0x09 }; + final byte[] entryA = { 0x0a, 0x05, 0x0a, 0x01, 0x61, 0x10, 0x01 }; // {"a":1} + + // Sweep a padding prefix so the entry straddles the read-buffer boundary + // (8000 bytes, jackson-core's default) at every alignment. + for (int padLen = 7980; padLen <= 8020; ++padLen) { + java.io.ByteArrayOutputStream b = new java.io.ByteArrayOutputStream(); + b.write(0x12); // tag 2 (`name`), length-prefixed + b.write(0x80 | (padLen & 0x7F)); // 2-byte varint length + b.write(padLen >> 7); + for (int i = 0; i < padLen; ++i) { + b.write('x'); + } + b.write(entryUnknownOnly); + b.write(entryA); + final byte[] doc = b.toByteArray(); + + JsonNode tree = MAPPER.readerFor(JsonNode.class).with(schema) + .readValue(new ByteArrayInputStream(doc)); + JsonNode counts = tree.get("counts"); + final String desc = "padLen "+padLen; + assertEquals(2, counts.size(), desc); + assertEquals(0, counts.get("").asInt(), desc); + assertEquals(1, counts.get("a").asInt(), desc); + } + } + @Test public void testWriteStartArrayOnMapRejected() throws Exception { From d607843127dbb4911b553a0533009f0d6c30c560 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Fri, 7 Aug 2026 21:36:33 -0700 Subject: [PATCH 3/7] Fix handling --- .../dataformat/protobuf/ProtobufParser.java | 80 ++++++++++++---- .../dataformat/protobuf/MapField712Test.java | 91 +++++++++++++++++++ 2 files changed, 151 insertions(+), 20 deletions(-) diff --git a/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufParser.java b/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufParser.java index 3b29c1a86..f975d6270 100644 --- a/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufParser.java +++ b/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufParser.java @@ -70,6 +70,12 @@ public class ProtobufParser extends ParserMinimalBase // Entry key has been surfaced (FIELD_NAME); read the entry value next private final static int STATE_MAP_VALUE = 17; + // Entry value has been surfaced: drain whatever remains of the entry, then pop it + // and resume iterating entries. Draining is deferred to this state (rather than done + // as the value is read) because length-prefixed values are skipped lazily, so the + // input pointer only reaches the value's end on the following `nextToken()`. + private final static int STATE_MAP_ENTRY_END = 18; + private final static int[] UTF8_UNIT_CODES = ProtobufUtil.sUtf8UnitLengths; // @since 2.14 @@ -733,6 +739,12 @@ public JsonToken nextToken() throws IOException case STATE_MAP_ENTRY_FIRST: // map field tag already consumed return _updateToken(_openMapEntry()); + case STATE_MAP_ENTRY_END: + // Entry may still hold content the value read did not consume (unknown + // fields, or a `key` that followed the value); resolve it, then pop. + _finishMapEntry(); + // fall through to iterating the next entry + case STATE_MAP_ENTRY_OTHER: if (_checkEnd()) { // reached end of enclosing message: map ends return _updateToken(JsonToken.END_OBJECT); @@ -800,16 +812,12 @@ private boolean _checkEnd() throws IOException _currentMessage = parentCtxt.getMessageType(); _currentEndOffset = parentCtxt.getEndOffset(); _currentField = parentCtxt.getField(); - // [dataformats-binary#712] If we popped into a map entry that is now fully - // consumed (its message-typed value was the last field), pop the entry too so - // we resume iterating entries rather than treating it as a nested message. - if (parentCtxt.isMapEntry() && (_inputPtr >= _currentEndOffset)) { - ProtobufReadContext mapCtxt = parentCtxt.getParent(); - _parsingContext = mapCtxt; - _currentMessage = mapCtxt.getMessageType(); - _currentEndOffset = mapCtxt.getEndOffset(); - _currentField = mapCtxt.getField(); - _state = STATE_MAP_ENTRY_OTHER; + // [dataformats-binary#712] If we popped into a map entry, its message-typed value + // has just ended: the entry itself is finished as far as tokens go, so drain and + // pop it (in STATE_MAP_ENTRY_END) rather than treating leftover entry content as + // fields of a nested message -- doing the latter would emit a second END_OBJECT. + if (parentCtxt.isMapEntry()) { + _state = STATE_MAP_ENTRY_END; return true; } if (_parsingContext.inRoot()) { @@ -1170,9 +1178,10 @@ private JsonToken _openMapEntry() throws IOException /** * Reads the value of the current {@code map} entry (tag 2), synthesizing a proto3 - * default when the value is absent. For scalar values the entry is then popped; for - * message values the nested Object is streamed (and the entry popped afterwards, in - * {@link #_checkEnd}). + * default when the value is absent. In every case the entry context is left in place + * and torn down later, from {@link #STATE_MAP_ENTRY_END}: for message values the + * nested Object is streamed first, and even for scalars the value's bytes may not + * have been consumed yet (length-prefixed values are skipped lazily). */ private JsonToken _readMapValue() throws IOException { @@ -1184,9 +1193,8 @@ private JsonToken _readMapValue() throws IOException } else { while (true) { if (_inputPtr >= _currentEndOffset) { // value absent -> proto3 default - JsonToken t = _mapDefaultValueToken(valueField); - _popMapEntry(); - return t; + _state = STATE_MAP_ENTRY_END; + return _mapDefaultValueToken(valueField); } tag = _decodeVInt(); final int id = (tag >> 3); @@ -1194,7 +1202,10 @@ private JsonToken _readMapValue() throws IOException break; } if (id == 1) { - _reportErrorF("Map entry for field has 'key' (id 1) after 'value': not supported"); + // 'key' was already read (or defaulted) before we got here, so this + // is a second one; a forward-only parser can not surface both. + _reportErrorF("Unsupported `map` entry encoding for field '%s': duplicate 'key' (id 1) before 'value' (id 2)", + _mapFieldName()); } _skipUnknownValue(tag & 0x7); // unknown field inside entry } @@ -1204,12 +1215,41 @@ private JsonToken _readMapValue() throws IOException _reportIncompatibleType(valueField, wireType); } if (valueField.type == FieldType.MESSAGE) { - // pushes value context (under the entry); entry is popped later, in _checkEnd + // pushes value context (under the entry); entry torn down once that ends return _readNextValue(valueField.type, STATE_NESTED_KEY); } - JsonToken t = _readNextValue(valueField.type, STATE_MAP_VALUE); + return _readNextValue(valueField.type, STATE_MAP_ENTRY_END); + } + + /** + * Consumes whatever remains of the current {@code map} entry once its value has been + * surfaced, then pops the entry. Trailing unknown fields are skipped; a {@code key} + * (tag 1) arriving after the value can not be surfaced (the field name was already + * emitted, and the input is forward-only), so it is reported rather than dropped -- + * silently substituting the proto3 default key would yield wrong data. + */ + private void _finishMapEntry() throws IOException + { + // NOTE: `_currentEndOffset` is the entry's end, and stays correct across buffer + // reloads (which rebase it); the entry context is still current at this point. + while (_inputPtr < _currentEndOffset) { + final int tag = _decodeVInt(); + if ((tag >> 3) == 1) { + _reportErrorF("Unsupported `map` entry encoding for field '%s': 'key' (id 1) follows 'value' (id 2) within the entry", + _mapFieldName()); + } + _skipUnknownValue(tag & 0x7); + } _popMapEntry(); - return t; + } + + /** + * @return Name of the {@code map} field whose entry is currently being decoded (the + * entry context is current; the enclosing map context holds the field). + */ + private String _mapFieldName() { + final ProtobufField f = _parsingContext.getParent().getField(); + return (f == null) ? "UNKNOWN" : f.name; } /** diff --git a/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/MapField712Test.java b/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/MapField712Test.java index 1c750552c..0d4350533 100644 --- a/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/MapField712Test.java +++ b/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/MapField712Test.java @@ -349,6 +349,97 @@ public void testLargeMapAcrossBufferReload() throws Exception } } + // Content trailing the value inside an entry must be consumed as part of the entry, + // not leak out into the enclosing message. + @Test + public void testUnknownFieldAfterScalarValueInEntry() throws Exception + { + ProtobufSchema schema = ProtobufSchemaLoader.std.parse( + "syntax = \"proto3\";\n" + + "message Msg { map counts = 1; string name = 2; }\n", "Msg"); + // entry body: key(0a 01 6b) value(10 01) unknown(id 3, varint: 18 09) = 7 bytes, + // then a genuine `name` field of the enclosing message + byte[] doc = { 0x0a, 0x07, 0x0a, 0x01, 0x6b, 0x10, 0x01, 0x18, 0x09, + 0x12, 0x01, 0x7a }; + JsonNode tree = MAPPER.readerFor(JsonNode.class).with(schema).readValue(doc); + assertEquals(1, tree.get("counts").get("k").asInt()); + assertEquals("z", tree.get("name").asText()); + } + + // Same, but for a message-valued entry: the stray field used to be decoded as a + // field of the entry message, yielding an extra END_OBJECT. + @Test + public void testUnknownFieldAfterMessageValueInEntry() throws Exception + { + ProtobufSchema schema = ProtobufSchemaLoader.std.parse( + "syntax = \"proto3\";\n" + + "message Val { int32 x = 1; }\n" + + "message Msg { map m = 1; string name = 2; }\n", "Msg"); + // entry body: key(0a 01 6b) value(12 02 08 07) unknown(18 09) = 9 bytes + byte[] doc = { 0x0a, 0x09, 0x0a, 0x01, 0x6b, 0x12, 0x02, 0x08, 0x07, 0x18, 0x09, + 0x12, 0x01, 0x7a }; + + // Token stream must match the well-formed encoding exactly (no stray END_OBJECT) + byte[] clean = { 0x0a, 0x07, 0x0a, 0x01, 0x6b, 0x12, 0x02, 0x08, 0x07, + 0x12, 0x01, 0x7a }; + assertEquals(_tokens(schema, clean), _tokens(schema, doc)); + + JsonNode tree = MAPPER.readerFor(JsonNode.class).with(schema).readValue(doc); + assertEquals(7, tree.get("m").get("k").get("x").asInt()); + assertEquals("z", tree.get("name").asText()); + } + + private String _tokens(ProtobufSchema schema, byte[] doc) throws Exception + { + StringBuilder sb = new StringBuilder(); + try (JsonParser p = MAPPER.getFactory().createParser(doc)) { + p.setSchema(schema); + JsonToken t; + while ((t = p.nextToken()) != null) { + sb.append(t).append(' '); + } + } + return sb.toString(); + } + + // Protobuf does not constrain field order, so an entry may encode `value` (tag 2) + // before `key` (tag 1). A forward-only parser can not surface that key, so it must + // be reported -- silently yielding the default key would be wrong data. + @Test + public void testKeyAfterValueInEntryReported() throws Exception + { + ProtobufSchema schema = ProtobufSchemaLoader.std.parse( + "syntax = \"proto3\";\n" + + "message Msg { map counts = 1; }\n", "Msg"); + // entry body: value(10 05) key(0a 01 6b) = 5 bytes + byte[] doc = { 0x0a, 0x05, 0x10, 0x05, 0x0a, 0x01, 0x6b }; + try { + MAPPER.readerFor(JsonNode.class).with(schema).readValue(doc); + fail("Should not pass: 'key' follows 'value' within entry"); + } catch (Exception e) { + verifyException(e, "'key' (id 1) follows 'value'"); + verifyException(e, "counts"); + } + } + + // Two `key` fields within one entry: distinct from the above, and must say so + @Test + public void testDuplicateKeyInEntryReported() throws Exception + { + ProtobufSchema schema = ProtobufSchemaLoader.std.parse( + "syntax = \"proto3\";\n" + + "message Msg { map counts = 1; }\n", "Msg"); + // entry body: key(0a 01 6b) key(0a 01 63) = 6 bytes + byte[] doc = { 0x0a, 0x06, 0x0a, 0x01, 0x6b, 0x0a, 0x01, 0x63 }; + try { + MAPPER.readerFor(JsonNode.class).with(schema).readValue(doc); + fail("Should not pass: duplicate 'key' within entry"); + } catch (Exception e) { + verifyException(e, "duplicate 'key' (id 1)"); + verifyException(e, "counts"); + } + } + // The entry-end bound must survive a buffer reload that happens while scanning an // entry's fields, not just while reading the value: end offsets are rebased on // reload (see `ProtobufReadContext.adjustEnd()`), so a bound captured beforehand From 6546775b6bc623dbe6768c3b2a782507a2a2ea39 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Fri, 7 Aug 2026 21:44:46 -0700 Subject: [PATCH 4/7] Moar fixing --- .../protobuf/ProtobufGenerator.java | 71 +++++++++++++++---- .../protobuf/schema/TypeResolver.java | 2 +- .../dataformat/protobuf/MapField712Test.java | 69 ++++++++++++++++++ 3 files changed, 126 insertions(+), 16 deletions(-) diff --git a/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufGenerator.java b/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufGenerator.java index dd9ae0e8a..b52b0dc51 100644 --- a/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufGenerator.java +++ b/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufGenerator.java @@ -1787,10 +1787,6 @@ private final int _writeTag(int ptr) return ptr; } - /** - * Method called when buffering an entry that should be prefixed - * with a type tag. - */ /* /********************************************************** /* Internal map (`map`) writes [dataformats-binary#712] @@ -1838,15 +1834,19 @@ private void _writeMapKey(ProtobufField keyField, String name) throws IOExceptio case BOOLEAN: writeBoolean(_parseBooleanMapKey(name, keyField)); break; - case VINT32_STD: - case VINT32_Z: - case FIXINT32: - writeNumber(_parseIntMapKey(name, keyField)); + case VINT32_Z: // `sint32`: signed only + writeNumber(_parseIntMapKey(name, keyField, false)); + break; + case VINT32_STD: // `int32` or `uint32` + case FIXINT32: // `sfixed32` or `fixed32` + writeNumber(_parseIntMapKey(name, keyField, true)); + break; + case VINT64_Z: // `sint64`: signed only + writeNumber(_parseLongMapKey(name, keyField, false)); break; - case VINT64_STD: - case VINT64_Z: - case FIXINT64: - writeNumber(_parseLongMapKey(name, keyField)); + case VINT64_STD: // `int64` or `uint64` + case FIXINT64: // `sfixed64` or `fixed64` + writeNumber(_parseLongMapKey(name, keyField, true)); break; default: // Should not happen: key types are validated during schema resolution @@ -1854,20 +1854,57 @@ private void _writeMapKey(ProtobufField keyField, String name) throws IOExceptio } } - private int _parseIntMapKey(String name, ProtobufField keyField) throws IOException { + /** + * Parses a 32-bit {@code map} key. + *

+ * {@link FieldType} does not distinguish signed from unsigned declarations + * ({@code int32} and {@code uint32} are both {@code VINT32_STD}), so for those the + * accepted range is the union of the two: {@code [Integer.MIN_VALUE, 0xFFFFFFFF]}. + * Narrowing a value above {@code Integer.MAX_VALUE} keeps the same 32 bits, which is + * exactly the {@code uint32} / {@code fixed32} encoding. + * + * @param allowUnsigned Whether the key type has an unsigned variant; {@code false} + * for {@code sint32}, which is always signed. + */ + private int _parseIntMapKey(String name, ProtobufField keyField, boolean allowUnsigned) + throws IOException + { + final long l; try { - return Integer.parseInt(name); + l = Long.parseLong(name); } catch (NumberFormatException e) { _reportError("Invalid `map` key \""+name+"\" for integral key field '"+keyField.name +"' (type "+keyField.type+")"); return 0; // never reached } + final long max = allowUnsigned ? 0xFFFFFFFFL : Integer.MAX_VALUE; + if ((l < Integer.MIN_VALUE) || (l > max)) { + _reportError("Invalid `map` key \""+name+"\" for integral key field '"+keyField.name + +"' (type "+keyField.type+"): out of range ("+Integer.MIN_VALUE+" to "+max+")"); + } + return (int) l; } - private long _parseLongMapKey(String name, ProtobufField keyField) throws IOException { + /** + * Parses a 64-bit {@code map} key. As with 32-bit keys the signed and unsigned + * declarations share a {@link FieldType}, so {@code uint64} keys above + * {@link Long#MAX_VALUE} are accepted as unsigned: the resulting bit pattern is the + * same 64-bit varint either way. + * + * @param allowUnsigned Whether the key type has an unsigned variant; {@code false} + * for {@code sint64}, which is always signed. + */ + private long _parseLongMapKey(String name, ProtobufField keyField, boolean allowUnsigned) + throws IOException + { try { return Long.parseLong(name); } catch (NumberFormatException e) { + if (allowUnsigned) { + try { + return Long.parseUnsignedLong(name); + } catch (NumberFormatException e2) { } + } _reportError("Invalid `map` key \""+name+"\" for integral key field '"+keyField.name +"' (type "+keyField.type+")"); return 0L; // never reached @@ -1885,6 +1922,10 @@ private boolean _parseBooleanMapKey(String name, ProtobufField keyField) throws return false; // never reached } + /** + * Method called when buffering an entry that should be prefixed + * with a type tag. + */ private final void _startBuffering(int typedTag) throws IOException { // need to ensure room for tag id, length (10 bytes); might as well ask for bit more diff --git a/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/schema/TypeResolver.java b/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/schema/TypeResolver.java index 9b343a3a6..ab09d7972 100644 --- a/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/schema/TypeResolver.java +++ b/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/schema/TypeResolver.java @@ -303,7 +303,7 @@ private static boolean _hasMapEntryOption(MessageElement rawType) * any integral type, {@code bool} or {@code string} (not floating-point, {@code bytes}, * enum or message). */ - private void _verifyMapKeyType(DataType keyType, FieldElement f, MessageElement rawType) + private static void _verifyMapKeyType(DataType keyType, FieldElement f, MessageElement rawType) { if (keyType instanceof DataType.ScalarType) { switch ((DataType.ScalarType) keyType) { diff --git a/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/MapField712Test.java b/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/MapField712Test.java index 0d4350533..6523c5f31 100644 --- a/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/MapField712Test.java +++ b/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/MapField712Test.java @@ -349,6 +349,75 @@ public void testLargeMapAcrossBufferReload() throws Exception } } + /* + /********************************************************** + /* Map key ranges + /********************************************************** + */ + + // Unsigned key types span the full 32/64-bit unsigned range, above what a signed + // Java int/long holds. Asserted on the wire bytes: `FieldType` does not carry + // signedness, so the parser renders these keys back as signed -- a pre-existing + // schema-layer limitation, separate from what the generator can encode. + @Test + public void testUnsignedMapKeysAtMax() throws Exception + { + // uint32 max: varint 0xFFFFFFFF is 5 bytes (ff ff ff ff 0f) + assertArrayEquals( + new byte[] { 0x0a, 0x08, 0x08, (byte) 0xff, (byte) 0xff, (byte) 0xff, (byte) 0xff, 0x0f, + 0x10, 0x01 }, + _writeSingleEntry("uint32", "4294967295")); + // fixed32 max: 4 raw bytes + assertArrayEquals( + new byte[] { 0x0a, 0x07, 0x0d, (byte) 0xff, (byte) 0xff, (byte) 0xff, (byte) 0xff, + 0x10, 0x01 }, + _writeSingleEntry("fixed32", "4294967295")); + // uint64 max: varint is 10 bytes (ff x9, then 01) + assertArrayEquals( + new byte[] { 0x0a, 0x0d, 0x08, + (byte) 0xff, (byte) 0xff, (byte) 0xff, (byte) 0xff, (byte) 0xff, + (byte) 0xff, (byte) 0xff, (byte) 0xff, (byte) 0xff, 0x01, + 0x10, 0x01 }, + _writeSingleEntry("uint64", "18446744073709551615")); + } + + @Test + public void testOutOfRangeMapKeysRejected() throws Exception + { + // just past uint32 max + _verifyBadKey("uint32", "4294967296", "out of range"); + // below int32 min + _verifyBadKey("int32", "-2147483649", "out of range"); + // `sint32` is signed only: the unsigned range must NOT be accepted for it + _verifyBadKey("sint32", "4294967295", "out of range"); + // past uint64 max + _verifyBadKey("uint64", "18446744073709551616", "Invalid `map` key"); + // not a number at all + _verifyBadKey("int32", "abc", "Invalid `map` key"); + } + + private byte[] _writeSingleEntry(String keyType, String key) throws Exception + { + ProtobufSchema schema = ProtobufSchemaLoader.std.parse( + "syntax = \"proto3\";\n" + + "message Msg { map<" + keyType + ", int32> m = 1; }\n", "Msg"); + Map root = new LinkedHashMap<>(); + Map m = new LinkedHashMap<>(); + m.put(key, 1); + root.put("m", m); + return MAPPER.writer(schema).writeValueAsBytes(root); + } + + private void _verifyBadKey(String keyType, String key, String expectedMsg) throws Exception + { + try { + _writeSingleEntry(keyType, key); + fail("Should not accept key \"" + key + "\" for `map<" + keyType + ",int32>`"); + } catch (Exception e) { + verifyException(e, expectedMsg); + } + } + // Content trailing the value inside an entry must be consumed as part of the entry, // not leak out into the enclosing message. @Test From 15907400c75f13e3d761e53d11d42f8abd88ac33 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Sat, 8 Aug 2026 19:32:35 -0700 Subject: [PATCH 5/7] Fixes --- .../dataformat/protobuf/ProtobufParser.java | 44 +++++---- .../protobuf/ProtobufReadContext.java | 92 +++++++++++++------ .../dataformat/protobuf/MapField712Test.java | 66 +++++++++++++ 3 files changed, 153 insertions(+), 49 deletions(-) diff --git a/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufParser.java b/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufParser.java index f975d6270..dd4733e14 100644 --- a/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufParser.java +++ b/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufParser.java @@ -812,11 +812,12 @@ private boolean _checkEnd() throws IOException _currentMessage = parentCtxt.getMessageType(); _currentEndOffset = parentCtxt.getEndOffset(); _currentField = parentCtxt.getField(); - // [dataformats-binary#712] If we popped into a map entry, its message-typed value - // has just ended: the entry itself is finished as far as tokens go, so drain and - // pop it (in STATE_MAP_ENTRY_END) rather than treating leftover entry content as - // fields of a nested message -- doing the latter would emit a second END_OBJECT. - if (parentCtxt.isMapEntry()) { + // [dataformats-binary#712] If we popped back into a map whose entry is still open, + // that entry's message-typed value has just ended: the entry is finished as far as + // tokens go, so drain and close it (in STATE_MAP_ENTRY_END) rather than treating + // leftover entry content as fields of a nested message -- doing the latter would + // emit a second END_OBJECT. + if (parentCtxt.isEntryOpen()) { _state = STATE_MAP_ENTRY_END; return true; } @@ -1125,6 +1126,11 @@ private void _skipUnknownValue(int wireType) throws IOException * position), reads its key (tag 1, possibly absent), and surfaces the key as the * current field name. Leaves {@link #_currentField} pointing at the value field and * transitions to {@link #STATE_MAP_VALUE}. + *

+ * No context is pushed for the entry: its key/value pair is a single member of the + * map Object, so making it a context would report -- and charge against + * {@code maxNestingDepth} -- a nesting level that the equivalent JSON does not have. + * The entry's bound lives on the map context instead. */ private JsonToken _openMapEntry() throws IOException { @@ -1140,8 +1146,7 @@ private JsonToken _openMapEntry() throws IOException _reportErrorF("Map entry for field '%s' extends past end of enclosing message: %d > %d (length: %d)", mapField.name, newEnd, _currentEndOffset, len); } - _parsingContext = _parsingContext.createChildMapEntryContext(newEnd); - _streamReadConstraints.validateNestingDepth(_parsingContext.getNestingDepth()); + _parsingContext.startMapEntry(newEnd); _currentEndOffset = newEnd; _currentMessage = mapField.getMessageType(); @@ -1231,7 +1236,7 @@ private JsonToken _readMapValue() throws IOException private void _finishMapEntry() throws IOException { // NOTE: `_currentEndOffset` is the entry's end, and stays correct across buffer - // reloads (which rebase it); the entry context is still current at this point. + // reloads (which rebase it); the entry is still open at this point. while (_inputPtr < _currentEndOffset) { final int tag = _decodeVInt(); if ((tag >> 3) == 1) { @@ -1240,29 +1245,28 @@ private void _finishMapEntry() throws IOException } _skipUnknownValue(tag & 0x7); } - _popMapEntry(); + _closeMapEntry(); } /** - * @return Name of the {@code map} field whose entry is currently being decoded (the - * entry context is current; the enclosing map context holds the field). + * @return Name of the {@code map} field whose entry is currently being decoded. */ private String _mapFieldName() { - final ProtobufField f = _parsingContext.getParent().getField(); + final ProtobufField f = _parsingContext.getMapField(); return (f == null) ? "UNKNOWN" : f.name; } /** - * Pops the current {@code map} entry context back to the enclosing map context, ready - * to read the next entry (or end the map). + * Ends the current {@code map} entry, restoring the map context's own bounds so the + * next entry (or the end of the map) can be read. */ - private void _popMapEntry() + private void _closeMapEntry() { - ProtobufReadContext mapCtxt = _parsingContext.getParent(); - _parsingContext = mapCtxt; - _currentMessage = mapCtxt.getMessageType(); - _currentEndOffset = mapCtxt.getEndOffset(); - _currentField = mapCtxt.getField(); + _parsingContext.closeMapEntry(); + _currentMessage = _parsingContext.getMessageType(); + _currentEndOffset = _parsingContext.getEndOffset(); + // `_field` may have been overwritten while a message-valued entry was streamed + _currentField = _parsingContext.getMapField(); _state = STATE_MAP_ENTRY_OTHER; } diff --git a/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufReadContext.java b/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufReadContext.java index 75ab7542c..bdcfbf820 100644 --- a/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufReadContext.java +++ b/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufReadContext.java @@ -51,12 +51,31 @@ public final class ProtobufReadContext protected boolean _inMap; /** - * Whether this (Object-typed) context represents a single {@code map} entry - * sub-message (whose {@code key}/{@code value} are surfaced as one Object member). + * For a map context ({@link #_inMap}): the {@code map} field itself. Kept apart from + * {@link #_field}, which is overwritten while a message-valued entry is streamed. * * @since 2.21.6 [dataformats-binary#712] */ - protected boolean _mapEntry; + protected ProtobufField _mapField; + + /** + * For a map context ({@link #_inMap}): whether an entry sub-message is currently + * being decoded. A map entry is not a JSON nesting level of its own -- its + * key/value pair is one member of the map Object -- so it gets no context, and its + * bound is tracked here instead. + * + * @since 2.21.6 [dataformats-binary#712] + */ + protected boolean _entryOpen; + + /** + * Offset within input buffer where the {@code map} entry currently being decoded + * ends; only meaningful while {@link #_entryOpen}. Rebased on buffer reload along + * with {@link #_endOffset}. + * + * @since 2.21.6 [dataformats-binary#712] + */ + protected int _entryEndOffset; /* /********************************************************** @@ -94,7 +113,8 @@ protected void reset(ProtobufMessage messageType, int type, int endOffset) _endOffset = endOffset; _field = null; _inMap = false; - _mapEntry = false; + _mapField = null; + _entryOpen = false; } @Override @@ -160,29 +180,11 @@ public ProtobufReadContext createChildMapContext(ProtobufField mapField, int end ctxt.reset(entryType, TYPE_OBJECT, endOffset); } ctxt._field = mapField; + ctxt._mapField = mapField; ctxt._inMap = true; return ctxt; } - /** - * Factory for the context of a single {@code map} entry sub-message. Must be called - * on a map context (see {@link #createChildMapContext}); the entry inherits that - * context's (synthetic entry) message type. - * - * @since 2.21.6 [dataformats-binary#712] - */ - public ProtobufReadContext createChildMapEntryContext(int endOffset) - { - ProtobufReadContext ctxt = _child; - if (ctxt == null) { - _child = ctxt = new ProtobufReadContext(this, _messageType, TYPE_OBJECT, endOffset); - } else { - ctxt.reset(_messageType, TYPE_OBJECT, endOffset); - } - ctxt._mapEntry = true; - return ctxt; - } - public ProtobufReadContext createChildObjectContext(ProtobufMessage messageType, ProtobufField f, int endOffset) { @@ -224,25 +226,33 @@ public int adjustEnd(int bytesConsumed) { if (_type == TYPE_ROOT) { return _endOffset; } - int newOffset = _endOffset - bytesConsumed; - - _endOffset = newOffset; + _endOffset -= bytesConsumed; + if (_entryOpen) { + _entryEndOffset -= bytesConsumed; + } for (ProtobufReadContext ctxt = _parent; ctxt != null; ctxt = ctxt.getParent()) { ctxt._adjustEnd(bytesConsumed); } // could do sanity check here; but caller should catch it - return newOffset; + return getEndOffset(); } private void _adjustEnd(int bytesConsumed) { if (_type != TYPE_ROOT) { _endOffset -= bytesConsumed; + if (_entryOpen) { + _entryEndOffset -= bytesConsumed; + } } } - public int getEndOffset() { return _endOffset; } + /** + * @return Offset at which the content currently being decoded ends: for a map context + * with an entry open that is the entry's end, otherwise this context's own end. + */ + public int getEndOffset() { return _entryOpen ? _entryEndOffset : _endOffset; } /** * @since 2.21.6 [dataformats-binary#712] @@ -250,9 +260,33 @@ private void _adjustEnd(int bytesConsumed) { public boolean inMap() { return _inMap; } /** + * Marks a {@code map} entry as being decoded, bounded by given offset. No child + * context is created: an entry is one member of the map Object, not a nesting level. + * + * @since 2.21.6 [dataformats-binary#712] + */ + public void startMapEntry(int endOffset) { + _entryOpen = true; + _entryEndOffset = endOffset; + } + + /** + * @since 2.21.6 [dataformats-binary#712] + */ + public void closeMapEntry() { _entryOpen = false; } + + /** + * @since 2.21.6 [dataformats-binary#712] + */ + public boolean isEntryOpen() { return _entryOpen; } + + /** + * @return The {@code map} field this (map) context was created for; unlike + * {@link #getField()} this survives a message-valued entry being streamed. + * * @since 2.21.6 [dataformats-binary#712] */ - public boolean isMapEntry() { return _mapEntry; } + public ProtobufField getMapField() { return _mapField; } public ProtobufMessage getMessageType() { return _messageType; } diff --git a/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/MapField712Test.java b/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/MapField712Test.java index 6523c5f31..f40ace276 100644 --- a/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/MapField712Test.java +++ b/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/MapField712Test.java @@ -8,6 +8,7 @@ import org.junit.jupiter.api.Test; import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.core.JsonStreamContext; import com.fasterxml.jackson.core.JsonToken; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.dataformat.protobuf.schema.ProtobufField; @@ -349,6 +350,71 @@ public void testLargeMapAcrossBufferReload() throws Exception } } + /* + /********************************************************** + /* Nesting / context shape + /********************************************************** + */ + + // A map entry is one member of the map Object, not a nesting level of its own, so a + // map must cost exactly what an equivalent nested message costs -- otherwise it eats + // into `maxNestingDepth` for a level the equivalent JSON does not have. + @Test + public void testMapAddsSingleNestingLevel() throws Exception + { + // both encode {"m":{"k":1}} + ProtobufSchema mapSchema = ProtobufSchemaLoader.std.parse( + "syntax = \"proto3\";\n" + + "message Msg { map m = 1; }\n", "Msg"); + byte[] mapDoc = { 0x0a, 0x05, 0x0a, 0x01, 0x6b, 0x10, 0x01 }; + + ProtobufSchema msgSchema = ProtobufSchemaLoader.std.parse( + "syntax = \"proto3\";\n" + + "message Inner { int32 k = 1; }\n" + + "message Msg { Inner m = 1; }\n", "Msg"); + byte[] msgDoc = { 0x0a, 0x02, 0x08, 0x01 }; + + assertEquals(_maxNestingDepth(msgSchema, msgDoc), _maxNestingDepth(mapSchema, mapDoc), + "`map` should nest no deeper than an equivalent nested message"); + } + + // ... and the context chain must show the map Object directly, with the entry key as + // its current name: no intermediate "entry" context. + @Test + public void testMapContextShape() throws Exception + { + ProtobufSchema schema = ProtobufSchemaLoader.std.parse( + "syntax = \"proto3\";\n" + + "message Msg { map m = 1; }\n", "Msg"); + byte[] doc = { 0x0a, 0x05, 0x0a, 0x01, 0x6b, 0x10, 0x01 }; + try (JsonParser p = MAPPER.getFactory().createParser(doc)) { + p.setSchema(schema); + assertToken(JsonToken.START_OBJECT, p.nextToken()); + assertToken(JsonToken.FIELD_NAME, p.nextToken()); // "m" + assertToken(JsonToken.START_OBJECT, p.nextToken()); + assertToken(JsonToken.FIELD_NAME, p.nextToken()); // "k" + assertToken(JsonToken.VALUE_NUMBER_INT, p.nextToken()); + + JsonStreamContext ctxt = p.getParsingContext(); + assertTrue(ctxt.inObject(), "map level should be an Object context"); + assertEquals("k", ctxt.getCurrentName()); + // parent is the enclosing message, NOT an entry wrapper + assertEquals("m", ctxt.getParent().getCurrentName()); + } + } + + private int _maxNestingDepth(ProtobufSchema schema, byte[] doc) throws Exception + { + int max = 0; + try (JsonParser p = MAPPER.getFactory().createParser(doc)) { + p.setSchema(schema); + while (p.nextToken() != null) { + max = Math.max(max, p.getParsingContext().getNestingDepth()); + } + } + return max; + } + /* /********************************************************** /* Map key ranges From a7c71ef3c95341f99c7059018cbc2f44a8883151 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Sat, 8 Aug 2026 19:41:36 -0700 Subject: [PATCH 6/7] last fixes --- .../protobuf/schema/TypeResolver.java | 19 ++++++-- .../dataformat/protobuf/MapField712Test.java | 48 +++++++++++++++++++ 2 files changed, 62 insertions(+), 5 deletions(-) diff --git a/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/schema/TypeResolver.java b/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/schema/TypeResolver.java index ab09d7972..fc96dd754 100644 --- a/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/schema/TypeResolver.java +++ b/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/schema/TypeResolver.java @@ -323,16 +323,25 @@ private static void _verifyMapKeyType(DataType keyType, FieldElement f, MessageE } /** - * Derives the name of the synthetic entry message for a {@code map} field, mirroring - * the {@code Entry} convention used by {@code protoc}. Only used for - * diagnostics and internal type lookup, so exact CamelCase-ing is not essential. + * Derives the name of the synthetic entry message for a {@code map} field. + *

+ * The name deliberately contains characters that can not occur in a protobuf + * identifier ({@code [A-Za-z_][A-Za-z0-9_]*}), so that it can never collide with a + * type the schema itself declares. Resolving the entry publishes it into the + * enclosing scope (see {@link #_resolve}), so a {@code protoc}-style + * {@code Entry} name would shadow a same-named declared type for any + * field resolved after the map field -- silently, and depending on declaration + * order. The name is only ever used for diagnostics and internal lookup, never for + * resolving references written in the schema. + * + * @since 2.21.6 [dataformats-binary#712] */ private static String _mapEntryName(String fieldName) { if (fieldName == null || fieldName.isEmpty()) { - return "Entry"; + return "map"; } - return Character.toUpperCase(fieldName.charAt(0)) + fieldName.substring(1) + "Entry"; + return "map<" + fieldName + ">"; } protected void addResolvedMessageType(String name, ProtobufMessage toResolve) { diff --git a/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/MapField712Test.java b/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/MapField712Test.java index f40ace276..a9e866d06 100644 --- a/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/MapField712Test.java +++ b/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/MapField712Test.java @@ -12,12 +12,14 @@ import com.fasterxml.jackson.core.JsonToken; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.dataformat.protobuf.schema.ProtobufField; +import com.fasterxml.jackson.dataformat.protobuf.schema.ProtobufMessage; import com.fasterxml.jackson.dataformat.protobuf.schema.ProtobufSchema; import com.fasterxml.jackson.dataformat.protobuf.schema.ProtobufSchemaLoader; import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -350,6 +352,52 @@ public void testLargeMapAcrossBufferReload() throws Exception } } + // Resolving a map publishes its synthetic entry message into the enclosing scope, so + // that name must not be one the schema could also declare -- otherwise a field naming + // that type silently resolves to the entry instead, depending on declaration order. + @Test + public void testSyntheticEntryNameDoesNotShadowDeclaredType() throws Exception + { + // `CountsEntry` is what protoc would name the entry for `map ... counts` + final String decl = "message CountsEntry { string zzz = 1; }\n"; + + // map declared BEFORE the reference (the order that used to break) + _verifyOtherIsDeclaredType( + "syntax = \"proto3\";\n" + decl + + "message Msg {\n" + + " map counts = 1;\n" + + " CountsEntry other = 2;\n" + + "}\n"); + // ... and after, which always worked: both must agree + _verifyOtherIsDeclaredType( + "syntax = \"proto3\";\n" + decl + + "message Msg {\n" + + " CountsEntry other = 1;\n" + + " map counts = 2;\n" + + "}\n"); + // control: a map whose name does not collide at all + _verifyOtherIsDeclaredType( + "syntax = \"proto3\";\n" + decl + + "message Msg {\n" + + " map tallies = 1;\n" + + " CountsEntry other = 2;\n" + + "}\n"); + } + + private void _verifyOtherIsDeclaredType(String proto) throws Exception + { + ProtobufSchema schema = ProtobufSchemaLoader.std.parse(proto, "Msg"); + ProtobufField other = schema.getRootType().field("other"); + assertNotNull(other); + ProtobufMessage type = other.getMessageType(); + assertNotNull(type); + assertNotNull(type.field("zzz"), + "'other' should resolve to declared `CountsEntry`, got fields: "+type.fieldsAsString()); + // and specifically NOT to the synthetic map entry + assertNull(type.field("key"), + "'other' resolved to the synthetic map entry: "+type.fieldsAsString()); + } + /* /********************************************************** /* Nesting / context shape From 969376bec66e34b08f431760015fca70d4ba3730 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Sun, 9 Aug 2026 16:53:27 -0700 Subject: [PATCH 7/7] Final fixes --- .../dataformat/protobuf/ProtobufParser.java | 15 ++++- .../protobuf/schema/TypeResolver.java | 66 ++++++++++++++++++- .../dataformat/protobuf/MapField712Test.java | 25 +++++++ .../schema/DescriptorMapField712Test.java | 56 ++++++++++++++++ 4 files changed, 159 insertions(+), 3 deletions(-) diff --git a/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufParser.java b/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufParser.java index dd4733e14..ed7aa28b1 100644 --- a/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufParser.java +++ b/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufParser.java @@ -1284,7 +1284,20 @@ private String _readMapKeyValue(ProtobufField keyField, int wireType) throws IOE if (_inputPtr >= _inputEnd) { loadMoreGuaranteed(); } - return (_inputBuffer[_inputPtr++] != 0) ? "true" : "false"; + { + // Same strictness as for `bool` values (see `_readNextValue()`): anything + // other than 0x0/0x1 is corrupt content, not a truthy byte + final int i = _inputBuffer[_inputPtr++]; + if (i == 1) { + return "true"; + } + if (i == 0) { + return "false"; + } + _reportError(String.format("Invalid byte value for bool `map` key field %s: 0x%2x; should be either 0x0 or 0x1", + keyField.name, i)); + } + return null; // never reached case VINT32_Z: return Integer.toString(ProtobufUtil.zigzagDecode(_decodeVInt())); case VINT32_STD: diff --git a/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/schema/TypeResolver.java b/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/schema/TypeResolver.java index fc96dd754..fd5046815 100644 --- a/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/schema/TypeResolver.java +++ b/protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/schema/TypeResolver.java @@ -224,7 +224,7 @@ protected ProtobufMessage _resolve(MessageElement rawType) if (pbf.repeated && !pbf.isMap && (pbf.type == FieldType.MESSAGE)) { final ProtobufMessage entryMsg = pbf.getMessageType(); if ((entryMsg != null) && entryMsg.isMapEntry()) { - pbf = new ProtobufField(f, entryMsg, entryMsg.field(1), entryMsg.field(2)); + pbf = _constructMapField(f, entryMsg, rawType); } } resolvedFields[ix++] = pbf; @@ -275,7 +275,69 @@ private ProtobufField _resolveMapField(FieldElement f, DataType.MapType mapType, .build()) .build(); final ProtobufMessage entryMsg = resolve(this, entryElem); - return new ProtobufField(f, entryMsg, entryMsg.field(1), entryMsg.field(2)); + return _constructMapField(f, entryMsg, rawType); + } + + /** + * Builds the map-flagged {@link ProtobufField} for given entry message, verifying + * first that the entry actually carries the {@code key} (tag 1) / {@code value} + * (tag 2) pair that map decoding requires, and that the key is of a type protobuf + * permits. + *

+ * Both are guaranteed by construction on the {@code .proto} path, but not on the + * descriptor-set path, where the entry message comes from the input: a + * {@code map_entry}-flagged message with a missing or ill-typed key/value is not + * something {@code protoc} emits, but a hand-built or corrupt descriptor can carry + * one, and it must fail as a schema error rather than as a {@code NullPointerException} + * from the codec later on. + * + * @since 2.21.6 [dataformats-binary#712] + */ + private static ProtobufField _constructMapField(FieldElement f, ProtobufMessage entryMsg, + MessageElement rawType) + { + final ProtobufField keyField = entryMsg.field(1); + final ProtobufField valueField = entryMsg.field(2); + if ((keyField == null) || (valueField == null)) { + throw new IllegalArgumentException(String.format( + "Invalid `map` entry type '%s' for field '%s' in MessageType '%s': entry" + + " message must declare both 'key' (tag 1) and 'value' (tag 2), but %s missing", + entryMsg.getName(), f.name(), rawType.name(), + (keyField == null) + ? ((valueField == null) ? "both are" : "'key' is") : "'value' is")); + } + if (!_isValidMapKeyType(keyField.type)) { + throw new IllegalArgumentException(String.format( + "Illegal key type (%s) for `map` field '%s' in MessageType '%s': protobuf" + + " map keys must be an integral type, `bool` or `string`", + keyField.type, f.name(), rawType.name())); + } + return new ProtobufField(f, entryMsg, keyField, valueField); + } + + /** + * @return Whether given (resolved) type is one protobuf permits as a {@code map} key. + * Mirrors {@link #_verifyMapKeyType}, which checks the same restriction against the + * as-declared type on the {@code .proto} path; this one works on the resolved type, + * so it covers the descriptor-set path too. + * + * @since 2.21.6 [dataformats-binary#712] + */ + private static boolean _isValidMapKeyType(FieldType t) + { + switch (t) { + case STRING: + case BOOLEAN: + case VINT32_STD: + case VINT32_Z: + case FIXINT32: + case VINT64_STD: + case VINT64_Z: + case FIXINT64: + return true; + default: // DOUBLE, FLOAT, BYTES, ENUM, MESSAGE + return false; + } } /** diff --git a/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/MapField712Test.java b/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/MapField712Test.java index a9e866d06..f7d808384 100644 --- a/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/MapField712Test.java +++ b/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/MapField712Test.java @@ -585,6 +585,31 @@ private String _tokens(ProtobufSchema schema, byte[] doc) throws Exception return sb.toString(); } + // A `bool` key must be as strictly validated as a `bool` value: only 0x0/0x1, rather + // than "any non-zero byte is true". + @Test + public void testInvalidBoolMapKeyByteReported() throws Exception + { + ProtobufSchema schema = ProtobufSchemaLoader.std.parse( + "syntax = \"proto3\";\n" + + "message Msg { map m = 1; }\n", "Msg"); + // entry body: key(08 02 -- 0x02 is not a valid bool) value(10 01) + byte[] doc = { 0x0a, 0x04, 0x08, 0x02, 0x10, 0x01 }; + try { + MAPPER.readerFor(JsonNode.class).with(schema).readValue(doc); + fail("Should not accept 0x02 as a `bool` map key"); + } catch (Exception e) { + verifyException(e, "Invalid byte value for bool"); + } + // ... while the two legal encodings still work + JsonNode t = MAPPER.readerFor(JsonNode.class).with(schema) + .readValue(new byte[] { 0x0a, 0x04, 0x08, 0x01, 0x10, 0x01 }); + assertEquals(1, t.get("m").get("true").asInt()); + JsonNode f = MAPPER.readerFor(JsonNode.class).with(schema) + .readValue(new byte[] { 0x0a, 0x04, 0x08, 0x00, 0x10, 0x01 }); + assertEquals(1, f.get("m").get("false").asInt()); + } + // Protobuf does not constrain field order, so an entry may encode `value` (tag 2) // before `key` (tag 1). A forward-only parser can not surface that key, so it must // be reported -- silently yielding the default key would be wrong data. diff --git a/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/schema/DescriptorMapField712Test.java b/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/schema/DescriptorMapField712Test.java index 3b28f8d27..1a3c33043 100644 --- a/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/schema/DescriptorMapField712Test.java +++ b/protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/schema/DescriptorMapField712Test.java @@ -18,6 +18,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; /** * Verifies that a {@code map} loaded from a {@code protoc} descriptor set -- @@ -86,6 +87,61 @@ public void testDescriptorMapResolvesAsMap() throws Exception assertEquals(FieldType.VINT32_STD, f.getValueField().type); } + // A `map_entry`-flagged message with a missing or ill-typed key/value is not something + // protoc emits, but a hand-built or corrupt descriptor can carry one. It must fail as + // a schema error, not as a NullPointerException from the codec later on. + @Test + public void testMalformedMapEntryDescriptorRejected() throws Exception + { + // no fields at all + _verifyBadEntry(new FieldDescriptorProto[] { }, + "must declare both 'key'"); + // `value` only + _verifyBadEntry(new FieldDescriptorProto[] { + field("value", 2, Label.LABEL_OPTIONAL, Type.TYPE_INT32, null) }, + "'key' is missing"); + // `key` only + _verifyBadEntry(new FieldDescriptorProto[] { + field("key", 1, Label.LABEL_OPTIONAL, Type.TYPE_STRING, null) }, + "'value' is missing"); + // key of a type protobuf does not permit for maps + _verifyBadEntry(new FieldDescriptorProto[] { + field("key", 1, Label.LABEL_OPTIONAL, Type.TYPE_DOUBLE, null), + field("value", 2, Label.LABEL_OPTIONAL, Type.TYPE_INT32, null) }, + "map keys must be"); + } + + private void _verifyBadEntry(FieldDescriptorProto[] entryFields, String expectedMsg) + throws Exception + { + DescriptorProto entry = new DescriptorProto(); + entry.name = "CountsEntry"; + entry.field = entryFields; + MessageOptions opts = new MessageOptions(); + opts.map_entry = true; + entry.options = opts; + + DescriptorProto msg = new DescriptorProto(); + msg.name = "Msg"; + msg.field = new FieldDescriptorProto[] { + field("counts", 1, Label.LABEL_REPEATED, Type.TYPE_MESSAGE, ".Msg.CountsEntry") + }; + msg.nested_type = new DescriptorProto[] { entry }; + + FileDescriptorProto fdp = new FileDescriptorProto(); + fdp.name = "test.proto"; + fdp.setPackage(""); + fdp.syntax = "proto3"; + fdp.message_type = new DescriptorProto[] { msg }; + + try { + new FileDescriptorSet(new FileDescriptorProto[] { fdp }).schemaFor("Msg"); + fail("Should not accept malformed `map_entry` message"); + } catch (IllegalArgumentException e) { + verifyException(e, expectedMsg); + } + } + @Test public void testDescriptorMapRoundtrip() throws Exception {