Fix #67

Author: cowtowncoder

protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ByteAccumulator.java

@@ -19,8 +19,7 @@ public class ByteAccumulator
     protected final byte[] _prefixBuffer;
 
     /**
-     * Offset within {@link #_prefixBuffer} where there is room
-     * for prefix.
+     * Offset within {@link #_prefixBuffer} where there is room for prefix.
      */
     protected final int _prefixOffset;
 
@@ -39,18 +38,12 @@ public class ByteAccumulator
 
     public ByteAccumulator(ByteAccumulator p, int typedTag,
             byte[] prefixBuffer, int prefixOffset, int parentStart)
-    {
-        this(p, typedTag, prefixBuffer, prefixOffset);
-        _parentStart = parentStart;
-    }
-
-    public ByteAccumulator(ByteAccumulator p, int typedTag,
-            byte[] prefixBuffer, int prefixOffset)
     {
         _parent = p;
         _typedTag = typedTag;
         _prefixBuffer = prefixBuffer;
         _prefixOffset = prefixOffset;
+        _parentStart = parentStart;
     }
 
     public void append(byte[] buf, int offset, int len) {
@@ -66,9 +59,9 @@ public void append(byte[] buf, int offset, int len) {
     public ByteAccumulator finish(OutputStream out,
             byte[] input, int offset, int len) throws IOException
     {
+        final byte[] prefix = _prefixBuffer;
         int start = _prefixOffset;
         int ptr;
-        final byte[] prefix = _prefixBuffer;
 
         if (_typedTag == -1) {
             ptr = start;
@@ -77,11 +70,12 @@ public ByteAccumulator finish(OutputStream out,
         }
 
         int plen = _segmentBytes + len;
-
         ptr = ProtobufUtil.appendLengthLength(plen, prefix, ptr);
-
+        
         // root? Just output it all 
         if (_parent == null) {
+            // 04-Apr-2017, tatu: We know that parent will have flushed anything it might have,
+            //    so `_parentStart` is irrelevant here (but not in the other branch)
             out.write(prefix, start, ptr-start);
             for (Segment s = _firstSegment; s != null; s = s.next()) {
                 s.writeTo(out);
@@ -90,6 +84,12 @@ public ByteAccumulator finish(OutputStream out,
                 out.write(input, offset, len);
             }
         } else {
+            // 04-Apr-2017, tatu: for [dataformats-binary#67], need to flush possible
+            //    content parent had...
+            final int flushLen = _prefixOffset - _parentStart;
+            if (flushLen > 0) {
+                _parent.append(input, _parentStart, flushLen);
+            }
             _parent.append(prefix, start, ptr-start);
             if (_firstSegment != null) {
                 _parent.appendAll(_firstSegment, _lastSegment, _segmentBytes);
@@ -101,7 +101,7 @@ public ByteAccumulator finish(OutputStream out,
         return _parent;
     }
 
-    public ByteAccumulator finish(OutputStream out) throws IOException
+    public ByteAccumulator finish(OutputStream out, byte[] input) throws IOException
     {
         int start = _prefixOffset;
         int ptr;
@@ -117,11 +117,17 @@ public ByteAccumulator finish(OutputStream out) throws IOException
 
         // root? Just output it all 
         if (_parent == null) {
+            // 04-Apr-2017, tatu: We know that parent will have flushed anything it might have,
+            //    so `_parentStart` is irrelevant here (but not in the other branch)
             out.write(prefix, start, ptr-start);
             for (Segment s = _firstSegment; s != null; s = s.next()) {
                 s.writeTo(out);
             }
         } else {
+            final int flushLen = _prefixOffset - _parentStart;
+            if (flushLen > 0) {
+                _parent.append(input, _parentStart, flushLen);
+            }
             _parent.append(prefix, start, ptr-start);
             if (_firstSegment != null) {
                 _parent.appendAll(_firstSegment, _lastSegment, _segmentBytes);

protobuf/src/main/java/com/fasterxml/jackson/dataformat/protobuf/ProtobufGenerator.java

@@ -326,7 +326,7 @@ public final void flush() throws IOException
         }
         _output.flush();
     }
-    
+
     @Override
     public void close() throws IOException
     {
@@ -1731,42 +1731,43 @@ private final void _startBuffering() throws IOException
          *  similar to method above. But somehow that does not seem to be needed...
          *  Let's add it just to be safe, still.
          */
+        // 04-Apr-2017, tatu: Most likely this is because this can only happen when we are
+        //   writing Objects as elements of packed array; and this can not be root-level
+        //   value: and if so we must be buffering (to get length prefix)
+/*        
         if (_buffered == null) {
             int len = ptr - _currStart;
             if (len > 0) {
                 ptr = 0;
                 _output.write(_currBuffer, _currStart, len);
             }
         }
+*/        
         _buffered = new ByteAccumulator(_buffered, -1, _currBuffer, ptr, _currStart);
         ptr += 5;
         _currStart = ptr;
         _currPtr = ptr;
     }
 
+    /**
+     * Helper method called when the current buffering scope is closed;
+     * when packed array is completed (`writeEndArray()`) or record is
+     * completed (`writeEndObject()`).
+     */
     private final void _finishBuffering() throws IOException
     {
         final int start = _currStart;
-        final int newStart = _currPtr;
+        final int newStart = _currPtr;        
         final int currLen = newStart - start;
 
         ByteAccumulator acc = _buffered;
-        final ByteAccumulator child = _buffered;
         acc = acc.finish(_output, _currBuffer, start, currLen);
         _buffered = acc;
         if (acc == null) {
             _currStart = 0;
             _currPtr = 0;
             return;
         }
-
-        // 08-Mar-2017, tatu: for [dataformats-binary#54]
-        // need to reposition buffer, otherwise will overwrite
-        final int parentStart = child._parentStart;
-        final int flushLen = child._prefixOffset - parentStart;
-        if (flushLen > 0) {
-            _buffered.append(_currBuffer, parentStart, flushLen);
-        }
         _currStart = newStart;
 // already same, no need to change
 //      _currPtr = newStart;
@@ -1822,7 +1823,7 @@ protected void _complete() throws IOException
         } else {
             acc = acc.finish(_output, _currBuffer, start, currLen);
             while (acc != null) {
-                acc = acc.finish(_output);
+                acc = acc.finish(_output, _currBuffer);
             }
             _buffered = null;
         }

protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/failing/NestedWrite67Test.java renamed to protobuf/src/test/java/com/fasterxml/jackson/dataformat/protobuf/NestedWrite67Test.java

@@ -1,9 +1,8 @@
-package com.fasterxml.jackson.dataformat.protobuf.failing;
+package com.fasterxml.jackson.dataformat.protobuf;
 
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.fasterxml.jackson.annotation.JsonPropertyOrder;
 import com.fasterxml.jackson.dataformat.protobuf.ProtobufMapper;
-import com.fasterxml.jackson.dataformat.protobuf.ProtobufTestBase;
 import com.fasterxml.jackson.dataformat.protobuf.schema.ProtobufSchema;
 
 public class NestedWrite67Test extends ProtobufTestBase

release-notes/VERSION

@@ -16,7 +16,8 @@ Modules:
 #54 (protobuf): Some fields are left null
 #58 (avro): Regression due to changed namespace of inner enum types
  (reported by Peter R)
-#62: `java.lang.ArrayIndexOutOfBoundsException` at `CBORGenerator.java`:548
+#62: (cbor) `java.lang.ArrayIndexOutOfBoundsException` at `CBORGenerator.java`:548
+#67: (proto) Serialization of multiple nesting levels has issues
 
 2.8.7 (21-Feb-2017)