Change goog.crypt.stringToByteArray to throw asynchronously when it encounters a multi-byte character in goog.DEBUG mode. We will flip the default to true in the near future.

shicks · copybara-github · commit e439bfd5ff09 · 2022-09-29T11:32:31.000-07:00
RELNOTES: `goog.crypt.stringToByteArray` (along with `base64.encodeString` and `hash32.encodeString`) will now throw asynchronously on multi-byte characters in development.
PiperOrigin-RevId: 477778082
Change-Id: I968b138e02424f76ef7af65c32c7c474d79879ff
diff --git a/closure/goog/crypt/crypt.js b/closure/goog/crypt/crypt.js
@@ -11,6 +11,28 @@
 goog.provide('goog.crypt');
 
 goog.require('goog.asserts');
+goog.require('goog.async.throwException');
+
+
+/**
+ * Whether to async-throw on unicode input to the legacy versions of
+ * `goog.crypt.stringToByteArray` (i.e. when `throwSync` is false).
+ * NOTE: The default will change to `true` soon, after notifying users.
+ * @define {boolean}
+ */
+goog.crypt.ASYNC_THROW_ON_UNICODE_TO_BYTE =
+    goog.define('goog.crypt.ASYNC_THROW_ON_UNICODE_TO_BYTE', goog.DEBUG);
+
+
+/**
+ * Test-only stub to make our use of async.throwException more testable.
+ * @const
+ */
+goog.crypt.TEST_ONLY = {};
+
+
+/** Remappable alias. */
+goog.crypt.TEST_ONLY.throwException = goog.async.throwException;
 
 
 /**
@@ -43,6 +65,8 @@ goog.crypt.stringToByteArray = function(str, throwSync) {
       var err = new Error('go/unicode-to-byte-error');
       if (throwSync) {
         throw err;
+      } else if (goog.crypt.ASYNC_THROW_ON_UNICODE_TO_BYTE) {
+        goog.crypt.TEST_ONLY.throwException(err);
       }
       output[p++] = c & 0xff;
       c >>= 8;
diff --git a/closure/goog/crypt/crypt_test.js b/closure/goog/crypt/crypt_test.js
@@ -7,8 +7,10 @@
 goog.module('goog.cryptTest');
 goog.setTestOnly();
 
+const PropertyReplacer = goog.require('goog.testing.PropertyReplacer');
 const crypt = goog.require('goog.crypt');
 const googString = goog.require('goog.string');
+const recordFunction = goog.require('goog.testing.recordFunction');
 const testSuite = goog.require('goog.testing.testSuite');
 
 const UTF8_RANGES_BYTE_ARRAY =
@@ -194,13 +196,24 @@ testSuite({
   },
 
   testStringToByteArray() {
-    assertArrayEquals([], crypt.stringToByteArray(''));
-    assertArrayEquals([97, 98, 99], crypt.stringToByteArray('abc'));
-    assertArrayEquals(
-        [0xa0, 0x12, 0xa1], crypt.stringToByteArray('\xa0\x12\xa1'));
-
-    // NOTE: Currently allowed, but will become an async throw soon.
-    assertArrayEquals([2, 1], crypt.stringToByteArray('\u0102'));
+    const stubs = new PropertyReplacer();
+    const stubThrowException = recordFunction();
+    stubs.replace(crypt.TEST_ONLY, 'throwException', stubThrowException);
+    try {
+      assertArrayEquals([], crypt.stringToByteArray(''));
+      assertArrayEquals([97, 98, 99], crypt.stringToByteArray('abc'));
+      assertArrayEquals(
+          [0xa0, 0x12, 0xa1], crypt.stringToByteArray('\xa0\x12\xa1'));
+      if (stubThrowException.getCallCount() > 0) {
+        throw stubThrowException.getLastCall().getArgument(0);
+      }
+
+      // Test async throwing behavior.
+      assertArrayEquals([2, 1], crypt.stringToByteArray('\u0102'));
+      assertEquals(1, stubThrowException.getCallCount());
+    } finally {
+      stubs.reset();
+    }
   },
 
   testBinaryStringToByteArray() {
