From 2bd55a9fbcd2815b3332bf309bc20f59eef0b36b Mon Sep 17 00:00:00 2001 From: Paul Yang Date: Tue, 12 Sep 2017 15:09:47 -0700 Subject: [PATCH] Fix js conformance tests. (#3604) * Fix js conformance tests. * Remove old incorrect compatibility tests --- conformance/failure_list_js.txt | 19 ----------- js/binary/decoder.js | 27 +++++++-------- js/binary/decoder_test.js | 19 +---------- .../v3.0.0/binary/decoder_test.js | 19 +---------- .../v3.1.0/binary/decoder_test.js | 19 +---------- .../protobuf/compiler/js/js_generator.cc | 34 ++++++++++++++++--- 6 files changed, 44 insertions(+), 93 deletions(-) diff --git a/conformance/failure_list_js.txt b/conformance/failure_list_js.txt index eb20f659a2..e69de29bb2 100644 --- a/conformance/failure_list_js.txt +++ b/conformance/failure_list_js.txt @@ -1,19 +0,0 @@ -Required.Proto3.ProtobufInput.RepeatedScalarSelectsLast.INT32.ProtobufOutput -Required.Proto3.ProtobufInput.RepeatedScalarSelectsLast.UINT32.ProtobufOutput -Required.Proto3.ProtobufInput.ValidDataRepeated.BOOL.ProtobufOutput -Required.Proto3.ProtobufInput.ValidDataRepeated.DOUBLE.ProtobufOutput -Required.Proto3.ProtobufInput.ValidDataRepeated.FIXED32.ProtobufOutput -Required.Proto3.ProtobufInput.ValidDataRepeated.FIXED64.ProtobufOutput -Required.Proto3.ProtobufInput.ValidDataRepeated.FLOAT.ProtobufOutput -Required.Proto3.ProtobufInput.ValidDataRepeated.INT32.ProtobufOutput -Required.Proto3.ProtobufInput.ValidDataRepeated.INT64.ProtobufOutput -Required.Proto3.ProtobufInput.ValidDataRepeated.SFIXED32.ProtobufOutput -Required.Proto3.ProtobufInput.ValidDataRepeated.SFIXED64.ProtobufOutput -Required.Proto3.ProtobufInput.ValidDataRepeated.SINT32.ProtobufOutput -Required.Proto3.ProtobufInput.ValidDataRepeated.SINT64.ProtobufOutput -Required.Proto3.ProtobufInput.ValidDataRepeated.UINT32.ProtobufOutput -Required.Proto3.ProtobufInput.ValidDataRepeated.UINT64.ProtobufOutput -Required.Proto2.ProtobufInput.RepeatedScalarSelectsLast.INT32.ProtobufOutput -Required.Proto2.ProtobufInput.RepeatedScalarSelectsLast.UINT32.ProtobufOutput -Required.Proto2.ProtobufInput.ValidDataRepeated.INT32.ProtobufOutput -Required.Proto2.ProtobufInput.ValidDataRepeated.UINT32.ProtobufOutput diff --git a/js/binary/decoder.js b/js/binary/decoder.js index 6db28e7c33..a38a501149 100644 --- a/js/binary/decoder.js +++ b/js/binary/decoder.js @@ -582,27 +582,24 @@ jspb.BinaryDecoder.prototype.readUnsignedVarint32 = function() { x |= (temp & 0x0F) << 28; if (temp < 128) { // We're reading the high bits of an unsigned varint. The byte we just read - // also contains bits 33 through 35, which we're going to discard. Those - // bits _must_ be zero, or the encoding is invalid. - goog.asserts.assert((temp & 0xF0) == 0); + // also contains bits 33 through 35, which we're going to discard. this.cursor_ += 5; goog.asserts.assert(this.cursor_ <= this.end_); return x >>> 0; } - // If we get here, we're reading the sign extension of a negative 32-bit int. - // We can skip these bytes, as we know in advance that they have to be all - // 1's if the varint is correctly encoded. Since we also know the value is - // negative, we don't have to coerce it to unsigned before we return it. - - goog.asserts.assert((temp & 0xF0) == 0xF0); - goog.asserts.assert(bytes[this.cursor_ + 5] == 0xFF); - goog.asserts.assert(bytes[this.cursor_ + 6] == 0xFF); - goog.asserts.assert(bytes[this.cursor_ + 7] == 0xFF); - goog.asserts.assert(bytes[this.cursor_ + 8] == 0xFF); - goog.asserts.assert(bytes[this.cursor_ + 9] == 0x01); + // If we get here, we need to truncate coming bytes. However we need to make + // sure cursor place is correct. + var i = 5; + do { + goog.asserts.assert(i < 10); + if (bytes[this.cursor_ + i] < 128) { + break; + } + i++; + } while (1); - this.cursor_ += 10; + this.cursor_ += i + 1; goog.asserts.assert(this.cursor_ <= this.end_); return x; }; diff --git a/js/binary/decoder_test.js b/js/binary/decoder_test.js index d0139e293a..b19e1d1b27 100644 --- a/js/binary/decoder_test.js +++ b/js/binary/decoder_test.js @@ -270,24 +270,7 @@ describe('binaryDecoderTest', function() { assertThrows(function() {decoder.readSignedVarint64()}); decoder.reset(); assertThrows(function() {decoder.readZigzagVarint64()}); - - // Positive 32-bit varints encoded with 1 bits in positions 33 through 35 - // should trigger assertions. - decoder.setBlock([255, 255, 255, 255, 0x1F]); - assertThrows(function() {decoder.readUnsignedVarint32()}); - - decoder.setBlock([255, 255, 255, 255, 0x2F]); - assertThrows(function() {decoder.readUnsignedVarint32()}); - - decoder.setBlock([255, 255, 255, 255, 0x4F]); - assertThrows(function() {decoder.readUnsignedVarint32()}); - - // Negative 32-bit varints encoded with non-1 bits in the high dword should - // trigger assertions. - decoder.setBlock([255, 255, 255, 255, 255, 255, 0, 255, 255, 1]); - assertThrows(function() {decoder.readUnsignedVarint32()}); - - decoder.setBlock([255, 255, 255, 255, 255, 255, 255, 255, 255, 0]); + decoder.reset(); assertThrows(function() {decoder.readUnsignedVarint32()}); }); diff --git a/js/compatibility_tests/v3.0.0/binary/decoder_test.js b/js/compatibility_tests/v3.0.0/binary/decoder_test.js index ac31264847..fce2fe181c 100644 --- a/js/compatibility_tests/v3.0.0/binary/decoder_test.js +++ b/js/compatibility_tests/v3.0.0/binary/decoder_test.js @@ -228,24 +228,7 @@ describe('binaryDecoderTest', function() { assertThrows(function() {decoder.readSignedVarint64()}); decoder.reset(); assertThrows(function() {decoder.readZigzagVarint64()}); - - // Positive 32-bit varints encoded with 1 bits in positions 33 through 35 - // should trigger assertions. - decoder.setBlock([255, 255, 255, 255, 0x1F]); - assertThrows(function() {decoder.readUnsignedVarint32()}); - - decoder.setBlock([255, 255, 255, 255, 0x2F]); - assertThrows(function() {decoder.readUnsignedVarint32()}); - - decoder.setBlock([255, 255, 255, 255, 0x4F]); - assertThrows(function() {decoder.readUnsignedVarint32()}); - - // Negative 32-bit varints encoded with non-1 bits in the high dword should - // trigger assertions. - decoder.setBlock([255, 255, 255, 255, 255, 255, 0, 255, 255, 1]); - assertThrows(function() {decoder.readUnsignedVarint32()}); - - decoder.setBlock([255, 255, 255, 255, 255, 255, 255, 255, 255, 0]); + decoder.reset(); assertThrows(function() {decoder.readUnsignedVarint32()}); }); diff --git a/js/compatibility_tests/v3.1.0/binary/decoder_test.js b/js/compatibility_tests/v3.1.0/binary/decoder_test.js index ac31264847..fce2fe181c 100644 --- a/js/compatibility_tests/v3.1.0/binary/decoder_test.js +++ b/js/compatibility_tests/v3.1.0/binary/decoder_test.js @@ -228,24 +228,7 @@ describe('binaryDecoderTest', function() { assertThrows(function() {decoder.readSignedVarint64()}); decoder.reset(); assertThrows(function() {decoder.readZigzagVarint64()}); - - // Positive 32-bit varints encoded with 1 bits in positions 33 through 35 - // should trigger assertions. - decoder.setBlock([255, 255, 255, 255, 0x1F]); - assertThrows(function() {decoder.readUnsignedVarint32()}); - - decoder.setBlock([255, 255, 255, 255, 0x2F]); - assertThrows(function() {decoder.readUnsignedVarint32()}); - - decoder.setBlock([255, 255, 255, 255, 0x4F]); - assertThrows(function() {decoder.readUnsignedVarint32()}); - - // Negative 32-bit varints encoded with non-1 bits in the high dword should - // trigger assertions. - decoder.setBlock([255, 255, 255, 255, 255, 255, 0, 255, 255, 1]); - assertThrows(function() {decoder.readUnsignedVarint32()}); - - decoder.setBlock([255, 255, 255, 255, 255, 255, 255, 255, 255, 0]); + decoder.reset(); assertThrows(function() {decoder.readUnsignedVarint32()}); }); diff --git a/src/google/protobuf/compiler/js/js_generator.cc b/src/google/protobuf/compiler/js/js_generator.cc index 73d3276297..a25de76cd6 100755 --- a/src/google/protobuf/compiler/js/js_generator.cc +++ b/src/google/protobuf/compiler/js/js_generator.cc @@ -2876,6 +2876,29 @@ void Generator::GenerateClassDeserializeBinaryField( "Group" : "Message", "grpfield", (field->type() == FieldDescriptor::TYPE_GROUP) ? (SimpleItoa(field->number()) + ", ") : ""); + } else if (field->is_repeated() && + field->cpp_type() != FieldDescriptor::CPPTYPE_STRING) { + printer->Print( + " if (reader.getWireType() == 2) {\n" + " var value = /** @type {$fieldtype_packed$} */ " + "(reader.readPacked$reader$());\n" + " msg.set$list_name$(value);\n" + " } else {\n" + " var value = /** @type {$fieldtype$} */ " + "(reader.read$reader$());\n" + " msg.add$name$(value);\n" + " }\n", + "fieldtype_packed", JSFieldTypeAnnotation(options, field, false, true, + /* singular_if_not_packed */ false, + BYTES_U8), + "fieldtype", JSFieldTypeAnnotation(options, field, false, true, + /* singular_if_not_packed */ true, + BYTES_U8), + "reader", JSBinaryReaderMethodType(field), + "list_name", JSGetterName(options, field), + "name", JSGetterName(options, field, + BYTES_DEFAULT, /* drop_list = */ true) + ); } else { printer->Print( " var value = /** @type {$fieldtype$} */ " @@ -2887,14 +2910,15 @@ void Generator::GenerateClassDeserializeBinaryField( JSBinaryReadWriteMethodName(field, /* is_writer = */ false)); } - if (field->is_repeated() && !field->is_packed()) { + if (field->is_repeated() && + (field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE || + field->cpp_type() == FieldDescriptor::CPPTYPE_STRING)) { printer->Print( " msg.add$name$(value);\n", "name", JSGetterName(options, field, BYTES_DEFAULT, /* drop_list = */ true)); - } else { - // Singular fields, and packed repeated fields, receive a |value| either - // as the field's value or as the array of all the field's values; set - // this as the field's value directly. + } else if (!field->is_repeated()) { + // Singular fields, receive a |value| as the field's value ; set this as + // the field's value directly. printer->Print( " msg.set$name$(value);\n", "name", JSGetterName(options, field));