Down-integrate from google3.

8 files changed, 73 insertions, 38 deletions

diff --git a/js/binary/constants.js b/js/binary/constants.js
index 21c5889c..5c53a360 100644
--- a/js/binary/constants.js
+++ b/js/binary/constants.js
@@ -174,7 +174,7 @@ jspb.PrunerFunction;
 
 /**
  * A comparer function returns true if two protos are equal.
- * @typedef {!function(?jspb.ConstBinaryMessage,
+ * @typedef {function(?jspb.ConstBinaryMessage,
  *                     ?jspb.ConstBinaryMessage):boolean}
  */
 jspb.ComparerFunction;
diff --git a/js/binary/reader.js b/js/binary/reader.js
index 2dc3eb70..731587c7 100644
--- a/js/binary/reader.js
+++ b/js/binary/reader.js
@@ -290,7 +290,9 @@ jspb.BinaryReader.prototype.nextField = function() {
       nextWireType != jspb.BinaryConstants.WireType.DELIMITED &&
       nextWireType != jspb.BinaryConstants.WireType.START_GROUP &&
       nextWireType != jspb.BinaryConstants.WireType.END_GROUP) {
-    goog.asserts.fail('Invalid wire type');
+    goog.asserts.fail(
+        'Invalid wire type: %s (at position %s)', nextWireType,
+        this.fieldCursor_);
     this.error_ = true;
     return false;
   }
@@ -388,8 +390,7 @@ 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 nestedGroups = [this.nextField_];
+  var previousField = this.nextField_;
   do {
     if (!this.nextField()) {
       goog.asserts.fail('Unmatched start-group tag: stream EOF');
@@ -397,19 +398,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..eb2af0a9 100644
--- a/js/binary/reader_test.js
+++ b/js/binary/reader_test.js
@@ -679,9 +679,24 @@ describe('binaryReaderTest', function() {
     writer.writeInt32(5, sentinel);
     var dummyMessage = /** @type {!jspb.BinaryMessage} */({});
     writer.writeGroup(5, dummyMessage, 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. 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);
+        writer.writeInt64(84, 44);
+        writer.writeBytes(
+          43, [255, 255, 255, 255, 255, 255, 255, 255, 255, 255]);
       });
     });
 
diff --git a/js/binary/utils.js b/js/binary/utils.js
index 55a9ccd4..173c416b 100644
--- a/js/binary/utils.js
+++ b/js/binary/utils.js
@@ -971,8 +971,9 @@ jspb.utils.byteSourceToUint8Array = function(data) {
     return /** @type {!Uint8Array} */(new Uint8Array(data));
   }
 
-  if (data.constructor === Buffer) {
-    return /** @type {!Uint8Array} */(new Uint8Array(data));
+  if (typeof Buffer != 'undefined' && data.constructor === Buffer) {
+    return /** @type {!Uint8Array} */ (
+        new Uint8Array(/** @type {?} */ (data)));
   }
 
   if (data.constructor === Array) {
diff --git a/js/map.js b/js/map.js
index 2fb14837..671bd516 100644
--- a/js/map.js
+++ b/js/map.js
@@ -136,7 +136,7 @@ jspb.Map.prototype.toArray = function() {
  *
  * @param {boolean=} includeInstance Whether to include the JSPB instance for
  *    transitional soy proto support: http://goto/soy-param-migration
- * @param {!function((boolean|undefined),V):!Object=} valueToObject
+ * @param {function((boolean|undefined),V):!Object=} valueToObject
  *    The static toObject() method, if V is a message type.
  * @return {!Array<!Array<!Object>>}
  */
@@ -165,9 +165,9 @@ jspb.Map.prototype.toObject = function(includeInstance, valueToObject) {
  *
  * @template K, V
  * @param {!Array<!Array<!Object>>} entries
- * @param {!function(new:V,?=)} valueCtor
+ * @param {function(new:V,?=)} valueCtor
  *    The constructor for type V.
- * @param {!function(!Object):V} valueFromObject
+ * @param {function(!Object):V} valueFromObject
  *    The fromObject function for type V.
  * @return {!jspb.Map<K, V>}
  */
@@ -410,9 +410,9 @@ jspb.Map.prototype.has = function(key) {
  * number.
  * @param {number} fieldNumber
  * @param {!jspb.BinaryWriter} writer
- * @param {!function(this:jspb.BinaryWriter,number,K)} keyWriterFn
+ * @param {function(this:jspb.BinaryWriter,number,K)} keyWriterFn
  *     The method on BinaryWriter that writes type K to the stream.
- * @param {!function(this:jspb.BinaryWriter,number,V,?=)|
+ * @param {function(this:jspb.BinaryWriter,number,V,?=)|
  *          function(this:jspb.BinaryWriter,number,V,?)} valueWriterFn
  *     The method on BinaryWriter that writes type V to the stream.  May be
  *     writeMessage, in which case the second callback arg form is used.
@@ -448,10 +448,10 @@ jspb.Map.prototype.serializeBinary = function(
  * @template K, V
  * @param {!jspb.Map} map
  * @param {!jspb.BinaryReader} reader
- * @param {!function(this:jspb.BinaryReader):K} keyReaderFn
+ * @param {function(this:jspb.BinaryReader):K} keyReaderFn
  *     The method on BinaryReader that reads type K from the stream.
  *
- * @param {!function(this:jspb.BinaryReader):V|
+ * @param {function(this:jspb.BinaryReader):V|
  *          function(this:jspb.BinaryReader,V,
  *                  function(V,!jspb.BinaryReader))} valueReaderFn
  *    The method on BinaryReader that reads type V from the stream. May be
diff --git a/js/message.js b/js/message.js
index 86d18295..6a37745b 100644
--- a/js/message.js
+++ b/js/message.js
@@ -439,9 +439,19 @@ jspb.Message.isArray_ = function(o) {
  * @private
  */
 jspb.Message.initPivotAndExtensionObject_ = function(msg, suggestedPivot) {
-  if (msg.array.length) {
-    var foundIndex = msg.array.length - 1;
-    var obj = msg.array[foundIndex];
+  // There are 3 variants that need to be dealt with which are the
+  // combination of whether there exists an extension object (EO) and
+  // whether there is a suggested pivot (SP).
+  //
+  // EO,    ?    : pivot is the index of the EO
+  // no-EO, no-SP: pivot is MAX_INT
+  // no-EO, SP   : pivot is the max(lastindex + 1, SP)
+
+  var msgLength = msg.array.length;
+  var lastIndex = -1;
+  if (msgLength) {
+    lastIndex = msgLength - 1;
+    var obj = msg.array[lastIndex];
     // Normal fields are never objects, so we can be sure that if we find an
     // object here, then it's the extension object. However, we must ensure that
     // the object is not an array, since arrays are valid field values.
@@ -449,14 +459,17 @@ jspb.Message.initPivotAndExtensionObject_ = function(msg, suggestedPivot) {
     // in Safari on iOS 8. See the description of CL/86511464 for details.
     if (obj && typeof obj == 'object' && !jspb.Message.isArray_(obj) &&
         !(jspb.Message.SUPPORTS_UINT8ARRAY_ && obj instanceof Uint8Array)) {
-      msg.pivot_ = jspb.Message.getFieldNumber_(msg, foundIndex);
+      msg.pivot_ = jspb.Message.getFieldNumber_(msg, lastIndex);
       msg.extensionObject_ = obj;
       return;
     }
   }
 
   if (suggestedPivot > -1) {
-    msg.pivot_ = suggestedPivot;
+    // If a extension object is not present, set the pivot value as being
+    // after the last value in the array to avoid overwriting values, etc.
+    msg.pivot_ = Math.max(
+        suggestedPivot, jspb.Message.getFieldNumber_(msg, lastIndex + 1));
     // Avoid changing the shape of the proto with an empty extension object by
     // deferring the materialization of the extension object until the first
     // time a field set into it (may be due to getting a repeated proto field
@@ -923,17 +936,6 @@ jspb.Message.setProto3IntField = function(msg, fieldNumber, value) {
 
 
 /**
- * Sets the value of a non-extension integer, handled as string, field of a proto3
- * @param {!jspb.Message} msg A jspb proto.
- * @param {number} fieldNumber The field number.
- * @param {number} value New value
- * @protected
- */
-jspb.Message.setProto3StringIntField = function(msg, fieldNumber, value) {
-  jspb.Message.setFieldIgnoringDefault_(msg, fieldNumber, value, '0');
-};
-
-/**
  * Sets the value of a non-extension floating point field of a proto3
  * @param {!jspb.Message} msg A jspb proto.
  * @param {number} fieldNumber The field number.
@@ -993,12 +995,22 @@ jspb.Message.setProto3EnumField = function(msg, fieldNumber, value) {
 };
 
 
+/**
+ * Sets the value of a non-extension int field of a proto3 that has jstype set
+ * to String.
+ * @param {!jspb.Message} msg A jspb proto.
+ * @param {number} fieldNumber The field number.
+ * @param {string} value New value
+ * @protected
+ */
+jspb.Message.setProto3StringIntField = function(msg, fieldNumber, value) {
+  jspb.Message.setFieldIgnoringDefault_(msg, fieldNumber, value, "0");
+};
 
 /**
  * Sets the value of a non-extension primitive field, with proto3 (non-nullable
  * primitives) semantics of ignoring values that are equal to the type's
  * default.
- * @template T
  * @param {!jspb.Message} msg A jspb proto.
  * @param {number} fieldNumber The field number.
  * @param {!Uint8Array|string|number|boolean|undefined} value New value
@@ -1007,7 +1019,7 @@ jspb.Message.setProto3EnumField = function(msg, fieldNumber, value) {
  */
 jspb.Message.setFieldIgnoringDefault_ = function(
     msg, fieldNumber, value, defaultValue) {
-  if (value != defaultValue) {
+  if (value !== defaultValue) {
     jspb.Message.setField(msg, fieldNumber, value);
   } else {
     msg.array[jspb.Message.getIndex_(msg, fieldNumber)] = null;
@@ -1127,7 +1139,7 @@ jspb.Message.getWrapperField = function(msg, ctor, fieldNumber, opt_required) {
  * @param {!jspb.Message} msg A jspb proto.
  * @param {function(new:jspb.Message, Array)} ctor Constructor for the field.
  * @param {number} fieldNumber The field number.
- * @return {Array<!jspb.Message>} The repeated field as an array of protos.
+ * @return {!Array<!jspb.Message>} The repeated field as an array of protos.
  * @protected
  */
 jspb.Message.getRepeatedWrapperField = function(msg, ctor, fieldNumber) {
diff --git a/js/message_test.js b/js/message_test.js
index 1be41093..2bfec62b 100644
--- a/js/message_test.js
+++ b/js/message_test.js
@@ -73,6 +73,7 @@ goog.require('proto.jspb.test.Simple1');
 goog.require('proto.jspb.test.Simple2');
 goog.require('proto.jspb.test.SpecialCases');
 goog.require('proto.jspb.test.TestClone');
+goog.require('proto.jspb.test.TestCloneExtension');
 goog.require('proto.jspb.test.TestEndsWithBytes');
 goog.require('proto.jspb.test.TestGroup');
 goog.require('proto.jspb.test.TestGroup1');
diff --git a/js/test.proto b/js/test.proto
index 7c881c0d..3b538b52 100644
--- a/js/test.proto
+++ b/js/test.proto
@@ -165,6 +165,13 @@ message TestClone {
   extensions 10 to max;
 }
 
+message TestCloneExtension {
+  extend TestClone {
+    optional TestCloneExtension low_ext = 11;
+  }
+  optional int32 f = 1;
+}
+
 message CloneExtension {
   extend TestClone {
     optional CloneExtension ext_field = 100;