From 82a4e4e3d3abf9c40bdbc54135d75d27eb5ef5a0 Mon Sep 17 00:00:00 2001 From: Yilun Chong Date: Tue, 31 Jul 2018 11:35:48 -0700 Subject: Fix js reader.js's skipGroup --- js/binary/reader.js | 12 +++++------- js/binary/reader_test.js | 2 ++ js/package.json | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/js/binary/reader.js b/js/binary/reader.js index 2dc3eb70..5e48f19b 100644 --- a/js/binary/reader.js +++ b/js/binary/reader.js @@ -389,7 +389,7 @@ jspb.BinaryReader.prototype.skipFixed64Field = function() { */ 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 +397,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..7fdf81b9 100644 --- a/js/binary/reader_test.js +++ b/js/binary/reader_test.js @@ -680,8 +680,10 @@ describe('binaryReaderTest', function() { var dummyMessage = /** @type {!jspb.BinaryMessage} */({}); writer.writeGroup(5, dummyMessage, function() { writer.writeInt64(42, 42); + writer.writeString(43, 'The quick brown fox jumps over the lazy dog'); writer.writeGroup(6, dummyMessage, function() { writer.writeInt64(84, 42); + writer.writeString(85, 'The quick brown fox jumps over the lazy dog'); }); }); diff --git a/js/package.json b/js/package.json index 325f2dcc..d2afcb3c 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" } -- cgit v1.2.3 From ed1cd2c5c63985128e582569c4a5d59a3ca71ebc Mon Sep 17 00:00:00 2001 From: Yilun Chong Date: Tue, 31 Jul 2018 15:14:08 -0700 Subject: fix --- js/binary/reader.js | 1 - js/binary/reader_test.js | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/js/binary/reader.js b/js/binary/reader.js index 5e48f19b..cf80830f 100644 --- a/js/binary/reader.js +++ b/js/binary/reader.js @@ -388,7 +388,6 @@ 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 previousField = this.nextField_; do { if (!this.nextField()) { diff --git a/js/binary/reader_test.js b/js/binary/reader_test.js index 7fdf81b9..0818109f 100644 --- a/js/binary/reader_test.js +++ b/js/binary/reader_test.js @@ -680,6 +680,7 @@ describe('binaryReaderTest', function() { var dummyMessage = /** @type {!jspb.BinaryMessage} */({}); writer.writeGroup(5, dummyMessage, function() { writer.writeInt64(42, 42); + writer.writeInt64(44, 44); writer.writeString(43, 'The quick brown fox jumps over the lazy dog'); writer.writeGroup(6, dummyMessage, function() { writer.writeInt64(84, 42); -- cgit v1.2.3 From 2ab6cb457028b322a9daba45e356f3fab26107e7 Mon Sep 17 00:00:00 2001 From: Yilun Chong Date: Wed, 1 Aug 2018 10:06:27 -0700 Subject: fix --- js/binary/reader_test.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/js/binary/reader_test.js b/js/binary/reader_test.js index 0818109f..28f135bf 100644 --- a/js/binary/reader_test.js +++ b/js/binary/reader_test.js @@ -681,10 +681,12 @@ describe('binaryReaderTest', function() { writer.writeGroup(5, dummyMessage, function() { writer.writeInt64(42, 42); writer.writeInt64(44, 44); - writer.writeString(43, 'The quick brown fox jumps over the lazy dog'); + writer.writeBytes(43, [255, 255, 255, 255, 255, 255, 255, 255, 255, 255]); writer.writeGroup(6, dummyMessage, function() { writer.writeInt64(84, 42); - writer.writeString(85, 'The quick brown fox jumps over the lazy dog'); + writer.writeInt64(84, 44); + writer.writeBytes( + 43, [255, 255, 255, 255, 255, 255, 255, 255, 255, 255]); }); }); -- cgit v1.2.3 From 1bec76f04cbaee8c48fecb2c21ff1015f398caf5 Mon Sep 17 00:00:00 2001 From: Yilun Chong Date: Wed, 1 Aug 2018 13:24:54 -0700 Subject: fix --- js/binary/reader_test.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/js/binary/reader_test.js b/js/binary/reader_test.js index 28f135bf..e1e6efa1 100644 --- a/js/binary/reader_test.js +++ b/js/binary/reader_test.js @@ -679,8 +679,13 @@ describe('binaryReaderTest', function() { writer.writeInt32(5, sentinel); var dummyMessage = /** @type {!jspb.BinaryMessage} */({}); writer.writeGroup(5, dummyMessage, function() { - writer.writeInt64(42, 42); + // 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. writer.writeInt64(44, 44); + writer.writeInt64(42, 42); + // This is for making the previous implementation failed by an invalid + // varint. writer.writeBytes(43, [255, 255, 255, 255, 255, 255, 255, 255, 255, 255]); writer.writeGroup(6, dummyMessage, function() { writer.writeInt64(84, 42); -- cgit v1.2.3 From 600e4e5f3b2d1a852a07de61d69e7245b78a5bf3 Mon Sep 17 00:00:00 2001 From: Yilun Chong Date: Wed, 1 Aug 2018 15:39:03 -0700 Subject: fix --- js/binary/reader_test.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/js/binary/reader_test.js b/js/binary/reader_test.js index e1e6efa1..69739727 100644 --- a/js/binary/reader_test.js +++ b/js/binary/reader_test.js @@ -682,10 +682,15 @@ describe('binaryReaderTest', 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. + // 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); -- cgit v1.2.3