diff options
| -rw-r--r-- | conformance/failure_list_js.txt | 19 | ||||
| -rw-r--r-- | js/binary/decoder.js | 27 | ||||
| -rw-r--r-- | js/binary/decoder_test.js | 19 | ||||
| -rw-r--r-- | js/compatibility_tests/v3.0.0/binary/decoder_test.js | 19 | ||||
| -rw-r--r-- | js/compatibility_tests/v3.1.0/binary/decoder_test.js | 19 | ||||
| -rwxr-xr-x | src/google/protobuf/compiler/js/js_generator.cc | 34 |
6 files changed, 93 insertions, 44 deletions
diff --git a/conformance/failure_list_js.txt b/conformance/failure_list_js.txt index e69de29b..eb20f659 100644 --- a/conformance/failure_list_js.txt +++ b/conformance/failure_list_js.txt @@ -0,0 +1,19 @@ +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 a38a5011..6db28e7c 100644 --- a/js/binary/decoder.js +++ b/js/binary/decoder.js @@ -582,24 +582,27 @@ 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. + // 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); this.cursor_ += 5; goog.asserts.assert(this.cursor_ <= this.end_); return x >>> 0; } - // 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); + // 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); - this.cursor_ += i + 1; + this.cursor_ += 10; goog.asserts.assert(this.cursor_ <= this.end_); return x; }; diff --git a/js/binary/decoder_test.js b/js/binary/decoder_test.js index b19e1d1b..d0139e29 100644 --- a/js/binary/decoder_test.js +++ b/js/binary/decoder_test.js @@ -270,7 +270,24 @@ describe('binaryDecoderTest', function() { assertThrows(function() {decoder.readSignedVarint64()}); decoder.reset(); assertThrows(function() {decoder.readZigzagVarint64()}); - decoder.reset(); + + // 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]); 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 fce2fe18..ac312648 100644 --- a/js/compatibility_tests/v3.0.0/binary/decoder_test.js +++ b/js/compatibility_tests/v3.0.0/binary/decoder_test.js @@ -228,7 +228,24 @@ describe('binaryDecoderTest', function() { assertThrows(function() {decoder.readSignedVarint64()}); decoder.reset(); assertThrows(function() {decoder.readZigzagVarint64()}); - decoder.reset(); + + // 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]); 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 fce2fe18..ac312648 100644 --- a/js/compatibility_tests/v3.1.0/binary/decoder_test.js +++ b/js/compatibility_tests/v3.1.0/binary/decoder_test.js @@ -228,7 +228,24 @@ describe('binaryDecoderTest', function() { assertThrows(function() {decoder.readSignedVarint64()}); decoder.reset(); assertThrows(function() {decoder.readZigzagVarint64()}); - decoder.reset(); + + // 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]); 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 a25de76c..73d32762 100755 --- a/src/google/protobuf/compiler/js/js_generator.cc +++ b/src/google/protobuf/compiler/js/js_generator.cc @@ -2876,29 +2876,6 @@ 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$} */ " @@ -2910,15 +2887,14 @@ void Generator::GenerateClassDeserializeBinaryField( JSBinaryReadWriteMethodName(field, /* is_writer = */ false)); } - if (field->is_repeated() && - (field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE || - field->cpp_type() == FieldDescriptor::CPPTYPE_STRING)) { + if (field->is_repeated() && !field->is_packed()) { printer->Print( " msg.add$name$(value);\n", "name", JSGetterName(options, field, BYTES_DEFAULT, /* drop_list = */ true)); - } else if (!field->is_repeated()) { - // Singular fields, receive a |value| as the field's value ; set this as - // the field's value directly. + } 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. printer->Print( " msg.set$name$(value);\n", "name", JSGetterName(options, field)); |