Revert "Fix js conformance tests. (#3604)" (#3633)
This reverts commit 2bd55a9fbcd2815b3332bf309bc20f59eef0b36b.
diff --git a/conformance/failure_list_js.txt b/conformance/failure_list_js.txt
index e69de29..eb20f65 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 a38a501..6db28e7 100644
--- a/js/binary/decoder.js
+++ b/js/binary/decoder.js
@@ -582,24 +582,27 @@
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.
- this.cursor_ += i + 1;
+ 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_ += 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 b19e1d1..d0139e2 100644
--- a/js/binary/decoder_test.js
+++ b/js/binary/decoder_test.js
@@ -270,7 +270,24 @@
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 fce2fe1..ac31264 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 @@
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 fce2fe1..ac31264 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 @@
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 a25de76..73d3276 100755
--- a/src/google/protobuf/compiler/js/js_generator.cc
+++ b/src/google/protobuf/compiler/js/js_generator.cc
@@ -2876,29 +2876,6 @@
"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 @@
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));
