Revert "Fix js conformance tests. (#3604)" (#3633)

This reverts commit 2bd55a9fbcd2815b3332bf309bc20f59eef0b36b.

4 files changed, 69 insertions, 15 deletions

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()});
   });