Catch non-strings in BinaryEncoder.writeString

The javascript serializeBinary() function silently tosses away any non-string placed into what should be a string field, writing the equivalent of an empty string instead

Not sure if or why anyone would be relying on that existing arbitrary behavior today, but silent data loss seems very undesirable and worth defending against

Author: dweitzman

binary/decoder_test.js

@@ -376,6 +376,16 @@ describe('binaryDecoderTest', function() {
     assertEquals(utf8_four_bytes, decoder.readString(utf8_four_bytes.length));
    });
 
+  /**
+   * Verifies that passing a non-string to writeString raises an error.
+   */
+  it('testBadString', function() {
+    var encoder = new jspb.BinaryEncoder();
+
+    assertThrows(function() {encoder.writeString(42)});
+    assertThrows(function() {encoder.writeString(null)});
+  });
+
   /**
    * Verifies that misuse of the decoder class triggers assertions.
    */

binary/encoder.js

@@ -473,6 +473,9 @@ jspb.BinaryEncoder.prototype.writeFixedHash64 = function(hash) {
 jspb.BinaryEncoder.prototype.writeString = function(value) {
   var oldLength = this.buffer_.length;
 
+  // Protect against non-string values being silently ignored.
+  goog.asserts.assert(value.charCodeAt);
+
   for (var i = 0; i < value.length; i++) {
 
     var c = value.charCodeAt(i);