Fix four issues flagged in AI code review

- MessagePackParser: make inner constructor package-private so
  MessagePackFactory can use it directly
- MessagePackFactory._createParser(byte[],int,int): replace
  Arrays.copyOfRange with ArrayBufferInput(data, offset, len) to
  avoid an unnecessary copy when parsing a byte-array slice
- MessagePackSerializedString: add capacity checks to all append/put
  methods (return -1 instead of throwing on overflow) and propagate
  IOException from writeQuotedUTF8/writeUnquotedUTF8
- TimestampExtensionModule.InstantDeserializer: replace bare
  RuntimeException with ctxt.reportInputMismatch for proper Jackson
  error reporting
- plans/preexisting-issues.md: document node-buffering architectural
  constraint and nested-POJO generator inefficiency as known FYI items

Author: komamitsu

msgpack-jackson3/src/main/java/org/msgpack/jackson/dataformat/MessagePackFactory.java

@@ -33,11 +33,13 @@
 import org.msgpack.core.MessagePack;
 import org.msgpack.core.annotations.VisibleForTesting;
 
+import org.msgpack.core.buffer.ArrayBufferInput;
+import org.msgpack.core.buffer.MessageBufferInput;
+
 import java.io.DataInput;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
-import java.util.Arrays;
 
 public class MessagePackFactory
         extends BinaryTSFactory
@@ -131,11 +133,9 @@ protected JsonParser _createParser(ObjectReadContext readCtxt, IOContext ioCtxt,
             byte[] data, int offset, int len) throws JacksonException
     {
         try {
-            if (offset != 0 || len != data.length) {
-                data = Arrays.copyOfRange(data, offset, offset + len);
-            }
+            MessageBufferInput input = new ArrayBufferInput(data, offset, len);
             MessagePackParser parser = new MessagePackParser(readCtxt, ioCtxt,
-                    readCtxt.getStreamReadFeatures(_streamReadFeatures), data, reuseResourceInParser);
+                    readCtxt.getStreamReadFeatures(_streamReadFeatures), input, data, reuseResourceInParser);
             if (extTypeCustomDesers != null) {
                 parser.setExtensionTypeCustomDeserializers(extTypeCustomDesers);
             }

msgpack-jackson3/src/main/java/org/msgpack/jackson/dataformat/MessagePackParser.java

@@ -93,7 +93,7 @@ public MessagePackParser(
         this(readCtxt, ioCtxt, streamReadFeatures, new ArrayBufferInput(bytes), bytes, reuseResourceInParser);
     }
 
-    private MessagePackParser(ObjectReadContext readCtxt,
+    MessagePackParser(ObjectReadContext readCtxt,
             IOContext ioCtxt,
             int streamReadFeatures,
             MessageBufferInput input,

msgpack-jackson3/src/main/java/org/msgpack/jackson/dataformat/MessagePackSerializedString.java

@@ -68,6 +68,9 @@ public byte[] asQuotedUTF8()
     public int appendQuotedUTF8(byte[] bytes, int i)
     {
         byte[] utf8 = asUnquotedUTF8();
+        if (utf8.length > bytes.length - i) {
+            return -1;
+        }
         System.arraycopy(utf8, 0, bytes, i, utf8.length);
         return utf8.length;
     }
@@ -76,6 +79,9 @@ public int appendQuotedUTF8(byte[] bytes, int i)
     public int appendQuoted(char[] chars, int i)
     {
         char[] q = asQuotedChars();
+        if (q.length > chars.length - i) {
+            return -1;
+        }
         System.arraycopy(q, 0, chars, i, q.length);
         return q.length;
     }
@@ -84,6 +90,9 @@ public int appendQuoted(char[] chars, int i)
     public int appendUnquotedUTF8(byte[] bytes, int i)
     {
         byte[] utf8 = asUnquotedUTF8();
+        if (utf8.length > bytes.length - i) {
+            return -1;
+        }
         System.arraycopy(utf8, 0, bytes, i, utf8.length);
         return utf8.length;
     }
@@ -92,40 +101,36 @@ public int appendUnquotedUTF8(byte[] bytes, int i)
     public int appendUnquoted(char[] chars, int i)
     {
         String v = getValue();
+        if (v.length() > chars.length - i) {
+            return -1;
+        }
         v.getChars(0, v.length(), chars, i);
         return v.length();
     }
 
     @Override
-    public int writeQuotedUTF8(OutputStream outputStream)
+    public int writeQuotedUTF8(OutputStream outputStream) throws IOException
     {
-        try {
-            byte[] utf8 = asUnquotedUTF8();
-            outputStream.write(utf8);
-            return utf8.length;
-        }
-        catch (IOException e) {
-            return -1;
-        }
+        byte[] utf8 = asUnquotedUTF8();
+        outputStream.write(utf8);
+        return utf8.length;
     }
 
     @Override
-    public int writeUnquotedUTF8(OutputStream outputStream)
+    public int writeUnquotedUTF8(OutputStream outputStream) throws IOException
     {
-        try {
-            byte[] utf8 = asUnquotedUTF8();
-            outputStream.write(utf8);
-            return utf8.length;
-        }
-        catch (IOException e) {
-            return -1;
-        }
+        byte[] utf8 = asUnquotedUTF8();
+        outputStream.write(utf8);
+        return utf8.length;
     }
 
     @Override
     public int putQuotedUTF8(ByteBuffer byteBuffer)
     {
         byte[] utf8 = asUnquotedUTF8();
+        if (utf8.length > byteBuffer.remaining()) {
+            return -1;
+        }
         byteBuffer.put(utf8);
         return utf8.length;
     }
@@ -134,6 +139,9 @@ public int putQuotedUTF8(ByteBuffer byteBuffer)
     public int putUnquotedUTF8(ByteBuffer byteBuffer)
     {
         byte[] utf8 = asUnquotedUTF8();
+        if (utf8.length > byteBuffer.remaining()) {
+            return -1;
+        }
         byteBuffer.put(utf8);
         return utf8.length;
     }

msgpack-jackson3/src/main/java/org/msgpack/jackson/dataformat/TimestampExtensionModule.java

@@ -69,10 +69,10 @@ public Instant deserialize(JsonParser p, DeserializationContext ctxt)
             try {
                 MessagePackExtensionType ext = p.readValueAs(MessagePackExtensionType.class);
                 if (ext.getType() != EXT_TYPE) {
-                    throw new RuntimeException(
-                            String.format("Unexpected extension type (0x%X) for Instant object", ext.getType()));
+                    ctxt.reportInputMismatch(Instant.class,
+                            "Unexpected extension type (0x%X) for Instant object", ext.getType());
+                    return null; // unreachable
                 }
-
                 try (MessageUnpacker unpacker = MessagePack.newDefaultUnpacker(ext.getData())) {
                     return unpacker.unpackTimestamp(new ExtensionTypeHeader(EXT_TYPE, ext.getData().length));
                 }

msgpack-jackson3/src/test/java/org/msgpack/jackson/dataformat/MessagePackGeneratorTest.java

@@ -1188,7 +1188,7 @@ public void testVersion()
     }
 
     @Test
-    public void testSerializedStringMethods()
+    public void testSerializedStringMethods() throws IOException
     {
         MessagePackSerializedString s = new MessagePackSerializedString("hello");
 

plans/preexisting-issues.md

@@ -72,7 +72,49 @@ buffer) into a `StringBuilder`, matching the pattern already used in the len<0 b
 The len<0 path (StringBuilder + 1024-char chunk) still allocates an extra copy vs.
 `CharArrayWriter`, but the overhead is minor and not worth optimizing at this layer.
 
-### 6. `writeRaw*` / `writeRawValue*` — FYI, no action needed
+### 6. Node buffering: full-tree design — FYI, no action needed
+
+**File:** `msgpack-jackson3/.../MessagePackGenerator.java` (node buffering design)
+
+`MessagePackGenerator` buffers the entire document as a tree of `ValueNode` objects and
+only serializes to MessagePack bytes when the root container closes (or on `flush()`).
+This is required because MessagePack's array and map headers encode the element count
+upfront (e.g. `fixarray 0x9X` or `array 16 0xdc`), and the count is not known until all
+children have been written.
+
+A potential optimization would be to eagerly serialize completed sub-trees (e.g. flush a
+completed nested array into bytes as soon as its `writeEndArray` is called). This reduces
+the peak heap footprint for large flat nested collections, since the child nodes can be
+GC'd once their bytes are emitted. However:
+
+- It does not eliminate the root-container buffering problem: the root map/array must
+  still buffer all its children until the root closes, because its own header depends on
+  the total count.
+- It adds an extra copy per sub-tree (the bytes for each sub-tree must be merged into the
+  parent's payload at close time).
+- In practice, the dominant cost is the root-level buffer, which this optimization does
+  not address.
+
+Decision: left as-is. Document as a known architectural constraint.
+
+### 7. Nested POJO generator inefficiency — FYI, no action needed
+
+**File:** `msgpack-jackson3/.../MessagePackGenerator.java` (writePOJO / nested generators)
+
+When a serializer calls `gen.writePOJO(someObject)`, Jackson internally creates a new
+`JsonGenerator` wrapping the same output stream and serializes the nested object into it.
+For `MessagePackGenerator` this means the nested generator also buffers a full node tree
+and emits bytes only when it closes — triggering a `flush()` of its own root context.
+The parent generator then reads those bytes back as a raw blob and re-wraps them in a
+`RawValueNode`.
+
+The round-trip (buffer → bytes → re-buffer as raw) means one extra allocation and copy
+per nested POJO. For deeply nested structures this can add up.
+
+Decision: this is an inherent consequence of the deferred-flush design (see item 6) and
+cannot be fixed without a larger architectural change. Left as-is.
+
+### 8. `writeRaw*` / `writeRawValue*` — FYI, no action needed
 
 **File:** `msgpack-jackson3/.../MessagePackGenerator.java`