Merge pull request #4981 from BSBandme/fix_js_skipgroup

Fix js reader.js's skipGroup

3 files changed, 21 insertions, 9 deletions

diff --git a/js/binary/reader.js b/js/binary/reader.js
index 2dc3eb70..cf80830f 100644
--- a/js/binary/reader.js
+++ b/js/binary/reader.js
@@ -388,8 +388,7 @@ jspb.BinaryReader.prototype.skipFixed64Field = function() {
  * Skips over the next group field in the binary stream.
  */
 jspb.BinaryReader.prototype.skipGroup = function() {
-  // Keep a stack of start-group tags that must be matched by end-group tags.
-  var nestedGroups = [this.nextField_];
+  var previousField = this.nextField_;
   do {
     if (!this.nextField()) {
       goog.asserts.fail('Unmatched start-group tag: stream EOF');
@@ -397,19 +396,17 @@ jspb.BinaryReader.prototype.skipGroup = function() {
       return;
     }
     if (this.nextWireType_ ==
-        jspb.BinaryConstants.WireType.START_GROUP) {
-      // Nested group start.
-      nestedGroups.push(this.nextField_);
-    } else if (this.nextWireType_ ==
                jspb.BinaryConstants.WireType.END_GROUP) {
       // Group end: check that it matches top-of-stack.
-      if (this.nextField_ != nestedGroups.pop()) {
+      if (this.nextField_ != previousField) {
         goog.asserts.fail('Unmatched end-group tag');
         this.error_ = true;
         return;
       }
+      return;
     }
-  } while (nestedGroups.length > 0);
+    this.skipField();
+  } while (true);
 };
 
 
diff --git a/js/binary/reader_test.js b/js/binary/reader_test.js
index 95711385..69739727 100644
--- a/js/binary/reader_test.js
+++ b/js/binary/reader_test.js
@@ -679,9 +679,24 @@ describe('binaryReaderTest', function() {
     writer.writeInt32(5, sentinel);
     var dummyMessage = /** @type {!jspb.BinaryMessage} */({});
     writer.writeGroup(5, dummyMessage, function() {
+      // Previously the skipGroup implementation was wrong, which only consume 
+      // the decoder by nextField. This case is for making the previous 
+      // implementation failed in skipGroup by an early end group tag.
+      // The reason is 44 = 5 * 8 + 4, this will be translated in to a field 
+      // with number 5 and with type 4 (end group)
+      writer.writeInt64(44, 44);
+      // This will make previous implementation failed by invalid tag (7).
+      writer.writeInt64(42, 47);
       writer.writeInt64(42, 42);
+      // This is for making the previous implementation failed by an invalid 
+      // varint. The bytes have at least 9 consecutive minus byte, which will 
+      // fail in this.nextField for previous implementation.
+      writer.writeBytes(43, [255, 255, 255, 255, 255, 255, 255, 255, 255, 255]);
       writer.writeGroup(6, dummyMessage, function() {
         writer.writeInt64(84, 42);
+        writer.writeInt64(84, 44);
+        writer.writeBytes(
+          43, [255, 255, 255, 255, 255, 255, 255, 255, 255, 255]);
       });
     });
 
diff --git a/js/package.json b/js/package.json
index c24d9510..dd5b6371 100644
--- a/js/package.json
+++ b/js/package.json
@@ -22,5 +22,5 @@
     "url": "https://github.com/google/protobuf/tree/master/js"
   },
   "author": "Google Protocol Buffers Team",
-  "license" : "BSD-3-Clause"
+  "license": "BSD-3-Clause"
 }