feat(pass-style): feature flag: only well-formed strings are passable (#2002)

Author: erights

packages/pass-style/NEWS.md

@@ -1,5 +1,16 @@
 User-visible changes in `@endo/pass-style`:
 
+# Next release
+
+- Exports `isWellFormedString` and `assertWellFormedString`. Unfortunately the [standard `String.prototype.isWellFormed`](https://tc39.es/proposal-is-usv-string/) first coerces its input to string, leading it to claim that some non-strings are well-formed strings. By contrast, `isWellFormedString` and `assertWellFormedString` will not judge any non-strings to be well-formed strings.
+  - Previously, all JavaScript strings were considered Passable with `passStyleOf(str) === 'string'`. Our tentative plan is that only well-formed Unicode strings will be considered Passable. For all others, `passStyleOf(str)` throws a diagnostic error. This would bring us into closer conformance to the OCapN standard, which prohibits sending non-well-formed strings, and requires non-well-formed strings to be rejected when received. Applications that had previously handled non-well-formed strings successfully (even if inadvertantly) may then start experiences these failure. We are also uncertain about the performance impact of this extra check, since it is linear in the size of strings.
+  - Thus, in this release we introduce the environment option `ONLY_WELL_FORMED_STRINGS_PASSABLE` as a feature flag. To abstract over this switch, we also export `assertPassableString`. For now, if `ONLY_WELL_FORMED_STRINGS_PASSABLE` environment option is `'enabled'`, then `assertPassableString` is the same as `assertWellFormedString`. Otherwise `assertPassableString` just asserts that `str` is a string. In a bash shell, for example, you could set
+      ```sh
+      export ONLY_WELL_FORMED_STRINGS_PASSABLE=enabled
+      ```
+      to turn this feature on.
+  - Currently, `ONLY_WELL_FORMED_STRINGS_PASSABLE` defaults to `'disabled'` because we do not yet know the performance impact. Later, if we decide we can afford it, we'll first change the default to `'enabled'` and ultimately remove the switch altogether. Be prepared for these changes.
+
 # v1.2.0 (2024-02-22)
 
 - Now supports `AggegateError`, `error.errors`, `error.cause`.

packages/pass-style/index.js

@@ -18,6 +18,12 @@ export {
   passableSymbolForName,
 } from './src/symbol.js';
 
+export {
+  isWellFormedString,
+  assertWellFormedString,
+  assertPassableString,
+} from './src/string.js';
+
 export {
   passStyleOf,
   isPassable,

packages/pass-style/package.json

@@ -34,6 +34,7 @@
     "test": "ava"
   },
   "dependencies": {
+    "@endo/env-options": "^1.1.1",
     "@endo/errors": "^1.1.0",
     "@endo/eventual-send": "^1.1.2",
     "@endo/promise-kit": "^1.0.4",

packages/pass-style/src/passStyleOf.js

@@ -24,6 +24,7 @@ import { RemotableHelper } from './remotable.js';
 
 import { assertPassableSymbol } from './symbol.js';
 import { assertSafePromise } from './safe-promise.js';
+import { assertPassableString } from './string.js';
 
 /** @typedef {import('./internal-types.js').PassStyleHelper} PassStyleHelper */
 /** @typedef {import('./types.js').Passable} Passable */
@@ -134,12 +135,15 @@ const makePassStyleOf = passStyleHelpers => {
       const typestr = typeof inner;
       switch (typestr) {
         case 'undefined':
-        case 'string':
         case 'boolean':
         case 'number':
         case 'bigint': {
           return typestr;
         }
+        case 'string': {
+          assertPassableString(inner);
+          return 'string';
+        }
         case 'symbol': {
           assertPassableSymbol(inner);
           return 'symbol';

packages/pass-style/src/string.js

@@ -0,0 +1,88 @@
+import { getEnvironmentOption } from '@endo/env-options';
+import { Fail } from '@endo/errors';
+
+// @ts-expect-error TS builtin `String` type does not yet
+// know about`isWellFormed`
+const hasWellFormedStringMethod = !!String.prototype.isWellFormed;
+
+/**
+ * Is the argument a well-formed string?
+ *
+ * Unfortunately, the
+ * [standard built-in `String.prototype.isWellFormed`](https://github.com/tc39/proposal-is-usv-string)
+ * does a ToString on its input, causing it to judge non-strings to be
+ * well-formed strings if they coerce to a well-formed strings. This
+ * recapitulates the mistake in having the global `isNaN` coerce its inputs,
+ * causing it to judge non-string to be NaN if they coerce to NaN.
+ *
+ * This `isWellFormedString` function only judges well-formed strings to be
+ * well-formed strings. For all non-strings it returns false.
+ *
+ * @param {unknown} str
+ * @returns {str is string}
+ */
+export const isWellFormedString = hasWellFormedStringMethod
+  ? // @ts-expect-error TS does not yet know about `isWellFormed`
+    str => typeof str === 'string' && str.isWellFormed()
+  : str => {
+      if (typeof str !== 'string') {
+        return false;
+      }
+      for (const ch of str) {
+        // The string iterator iterates by Unicode code point, not
+        // UTF16 code unit. But if it encounters an unpaired surrogate,
+        // it will produce it.
+        const cp = /** @type {number} */ (ch.codePointAt(0));
+        if (cp >= 0xd800 && cp <= 0xdfff) {
+          // All surrogates are in this range. The string iterator only
+          // produces a character in this range for unpaired surrogates,
+          // which only happens if the string is not well-formed.
+          return false;
+        }
+      }
+      return true;
+    };
+harden(isWellFormedString);
+
+/**
+ * Returns normally when `isWellFormedString(str)` would return true.
+ * Throws a diagnostic error when `isWellFormedString(str)` would return false.
+ *
+ * @param {unknown} str
+ * @returns {asserts str is string}
+ */
+export const assertWellFormedString = str => {
+  isWellFormedString(str) || Fail`Expected well-formed unicode string: ${str}`;
+};
+harden(assertWellFormedString);
+
+const ONLY_WELL_FORMED_STRINGS_PASSABLE =
+  getEnvironmentOption('ONLY_WELL_FORMED_STRINGS_PASSABLE', 'disabled', [
+    'enabled',
+  ]) === 'enabled';
+
+/**
+ * For now,
+ * if `ONLY_WELL_FORMED_STRINGS_PASSABLE` environment option is `'enabled'`,
+ * then `assertPassableString` is the same as `assertWellFormedString`.
+ * Otherwise `assertPassableString` just asserts that `str` is a string.
+ *
+ * Currently, `ONLY_WELL_FORMED_STRINGS_PASSABLE` defaults to `'disabled'`
+ * because we do not yet know the performance impact. Later, if we decide we
+ * can afford it, we'll first change the default to `'enabled'` and ultimately
+ * remove the switch altogether. Be prepared for these changes.
+ *
+ * TODO once the switch is removed, simplify `assertPassableString` to
+ * simply be `assertWellFormedString`.
+ *
+ * TODO update https://github.com/Agoric/agoric-sdk/blob/master/docs/env.md
+ * which is unfortunately in the wrong repo to be updated in the same change.
+ *
+ * @param { unknown } str
+ * @returns {asserts str is string }
+ */
+export const assertPassableString = str => {
+  typeof str === 'string' || Fail`Expected string ${str}`;
+  !ONLY_WELL_FORMED_STRINGS_PASSABLE || assertWellFormedString(str);
+};
+harden(assertPassableString);

packages/pass-style/test/prepare-only-well-formed-strings-passable.js

@@ -0,0 +1,3 @@
+/* global process */
+
+process.env.ONLY_WELL_FORMED_STRINGS_PASSABLE = 'enabled';

packages/pass-style/test/test-passable-string.js

@@ -0,0 +1,51 @@
+/* eslint-disable no-useless-concat */
+import './prepare-only-well-formed-strings-passable.js';
+import test from '@endo/ses-ava/prepare-endo.js';
+
+import { isWellFormedString, assertWellFormedString } from '../src/string.js';
+import { passStyleOf } from '../src/passStyleOf.js';
+
+test('test string well formedness behaviors', t => {
+  const gcleff1 = '\u{1D11E}';
+  const gcleff2 = '\u{D834}\u{DD1E}';
+  const gcleff3 = '\u{D834}' + '\u{DD1E}';
+  const badcleff1 = '\u{D834}\u{D834}\u{DD1E}';
+  const badcleff2 = '\u{D834}\u{DD1E}\u{D834}';
+  const badcleff3 = '\u{D834}' + '\u{DD1E}\u{D834}';
+
+  // This test block ensures that the underlying platform behaves as we expect
+  t.is(gcleff1, gcleff2);
+  t.is(gcleff1, gcleff3);
+  t.is(gcleff1.length, 2);
+  t.is(gcleff2.length, 2);
+  t.is(gcleff3.length, 2);
+  // Uses string iterator, which iterates code points if possible, not
+  // UTF16 code units
+  t.deepEqual([...gcleff1], [gcleff1]);
+  t.not(badcleff1, badcleff2);
+  t.is(badcleff2, badcleff3);
+  t.is(badcleff1.length, 3);
+  // But if the string contains lone surrogates, the string iterator will
+  // produce those as characters
+  t.deepEqual([...badcleff1], ['\u{D834}', gcleff1]);
+  t.deepEqual([...badcleff2], [gcleff1, '\u{D834}']);
+
+  t.is(passStyleOf(gcleff1), 'string');
+  t.true(isWellFormedString(gcleff1));
+  t.notThrows(() => assertWellFormedString(gcleff1));
+
+  t.throws(() => passStyleOf(badcleff1), {
+    message: 'Expected well-formed unicode string: "\\ud834𝄞"',
+  });
+  t.throws(() => passStyleOf(badcleff2), {
+    message: 'Expected well-formed unicode string: "𝄞\\ud834"',
+  });
+  t.false(isWellFormedString(badcleff1));
+  t.false(isWellFormedString(badcleff2));
+  t.throws(() => assertWellFormedString(badcleff1), {
+    message: 'Expected well-formed unicode string: "\\ud834𝄞"',
+  });
+  t.throws(() => assertWellFormedString(badcleff2), {
+    message: 'Expected well-formed unicode string: "𝄞\\ud834"',
+  });
+});