Fixed: Empty inner messages are always present on the wire + test cas…

…e + removed now unused Writer#ldelim parameter, see #585

CHANGELOG.md

@@ -1,3 +1,5 @@
+# [6.3.1](https://github.com/dcodeIO/protobuf.js/releases/tag/6.3.1)
+
 # [6.3.0](https://github.com/dcodeIO/protobuf.js/releases/tag/6.3.0)
 
 ## Breaking

package.json

@@ -1,6 +1,6 @@
 {
   "name": "protobufjs",
-  "version": "6.3.0",
+  "version": "6.3.1",
   "description": "Protocol Buffers for JavaScript (& TypeScript).",
   "author": "Daniel Wirtz <dcode+protobufjs@dcode.io>",
   "license": "BSD-3-Clause",

src/encoder.js

@@ -7,12 +7,10 @@ var Enum     = require("./enum"),
 
 var safeProp = util.safeProp;
 
-function genEncodeType(gen, field, fieldIndex, ref, alwaysRequired) {
-    if (field.resolvedType.group)
-        return gen("types[%d].encode(%s,w.uint32(%d)).uint32(%d)", fieldIndex, ref, (field.id << 3 | 3) >>> 0, (field.id << 3 | 4) >>> 0);
-    return alwaysRequired || field.required
-      ? gen("types[%d].encode(%s,w.uint32(%d).fork()).ldelim()", fieldIndex, ref, (field.id << 3 | 2) >>> 0)
-      : gen("types[%d].encode(%s,w.fork()).len&&w.ldelim(%d)||w.reset()", fieldIndex, ref, field.id);
+function genEncodeType(gen, field, fieldIndex, ref) {
+    return field.resolvedType.group
+        ? gen("types[%d].encode(%s,w.uint32(%d)).uint32(%d)", fieldIndex, ref, (field.id << 3 | 3) >>> 0, (field.id << 3 | 4) >>> 0)
+        : gen("types[%d].encode(%s,w.uint32(%d).fork()).ldelim()", fieldIndex, ref, (field.id << 3 | 2) >>> 0);
 }
 
 /**
@@ -69,7 +67,7 @@ function encoder(mtype) {
     ("if(%s)", ref)
         ("for(var i=0;i<%s.length;++i)", ref);
                 if (wireType === undefined)
-            genEncodeType(gen, field, i, ref + "[i]", true);
+            genEncodeType(gen, field, i, ref + "[i]");
                 else gen
             ("w.uint32(%d).%s(%s[i])", (field.id << 3 | wireType) >>> 0, type, ref);
 
@@ -88,7 +86,7 @@ function encoder(mtype) {
             }
 
             if (wireType === undefined)
-        genEncodeType(gen, field, i, ref, true);
+        genEncodeType(gen, field, i, ref);
             else gen
         ("w.uint32(%d).%s(%s)", (field.id << 3 | wireType) >>> 0, type, ref);
 

src/oneof.js

@@ -102,11 +102,12 @@ OneOfPrototype.toJSON = function toJSON() {
  * @ignore
  */
 function addFieldsToParent(oneof) {
-    if (oneof.parent)
+    if (oneof.parent) {
         oneof._fieldsArray.forEach(function(field) {
             if (!field.parent)
                 oneof.parent.add(field);
         });
+    }
 }
 
 /**
@@ -160,6 +161,16 @@ OneOfPrototype.remove = function remove(field) {
  */
 OneOfPrototype.onAdd = function onAdd(parent) {
     ReflectionObject.prototype.onAdd.call(this, parent);
+    var self = this;
+    // Collect present fields
+    this.oneof.forEach(function(fieldName) {
+        var field = parent.get(fieldName);
+        if (field && !field.partOf) {
+            field.partOf = self;
+            self._fieldsArray.push(field);
+        }
+    });
+    // Add not yet present fields
     addFieldsToParent(this);
 };
 

src/writer.js

@@ -513,17 +513,14 @@ WriterPrototype.reset = function reset() {
 
 /**
  * Resets to the last state and appends the fork state's current write length as a varint followed by its operations.
- * @param {number} [id] Id with wire type 2 to prepend as a tag where applicable
  * @returns {Writer} `this`
  */
-WriterPrototype.ldelim = function ldelim(id) {
+WriterPrototype.ldelim = function ldelim() {
     var head = this.head,
         tail = this.tail,
         len  = this.len;
-    this.reset();
-    if (typeof id === "number")
-        this.uint32((id << 3 | 2) >>> 0);
-    this.uint32(len);
+    this.reset()
+        .uint32(len);
     this.tail.next = head.next; // skip noop
     this.tail = tail;
     this.len += len;

tests/empty-inner.js

@@ -0,0 +1,36 @@
+var tape = require("tape");
+
+var protobuf = require("..");
+
+tape.test("empty inner message fields", function(test) {
+    var root = protobuf.Root.fromJSON({
+        nested: {
+            Inner: {
+                fields: {
+                }
+            },
+            Outer: {
+                oneofs: {
+                    child: {
+                        oneof: ["inner"]
+                    }
+                },
+                fields: {
+                    inner: {
+                        id: 1,
+                        type: "Inner"
+                    }
+                }
+            }
+        }
+    });
+    var Outer = root.lookup("Outer");
+    var msg = Outer.from({
+        inner: {}
+    });
+    var buf = Outer.encode(msg).finish();
+    test.equal(buf.length, 2, "should always be present on the wire");
+    test.equal(buf[0], 1 << 3 | 2, "should write id 1, wireType 2");
+    test.equal(buf[1], 0, "should write a length of 0");
+    test.end();
+});