Repository: salesforce/tough-cookie
Pull request #366 (2024-02-20 21:26:41)

Handle unlikely edge case in unimportant util
Currently `safeToString([Object.create(null)])` throws, even though it shouldn't. This PR fixes that.


Patch:
diff --git a/lib/utils.ts b/lib/utils.ts
index 0a5bb07..83628f5 100644
--- a/lib/utils.ts
+++ b/lib/utils.ts
@@ -13,17 +13,44 @@ export interface ErrorCallback {
 export const objectToString = (obj: unknown) =>
   Object.prototype.toString.call(obj)
 
-/** Safely converts any value to string, using the value's own `toString` when available. */
-export const safeToString = (val: unknown) => {
-  // Ideally, we'd just use String() for everything, but it breaks if `toString` is missing (mostly
-  // values with no prototype), so we have to use Object#toString as a fallback.
+/**
+ * Converts an array to string, safely handling symbols, null prototype objects, and recursive arrays.
+ */
+const safeArrayToString = (
+  arr: unknown[],
+  seenArrays: WeakSet<object>,
+): string => {
+  // See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/toString#description
+  if (typeof arr.join !== 'function') return objectToString(arr)
+  seenArrays.add(arr)
+  const mapped = arr.map((val) =>
+    val === null || val === undefined || seenArrays.has(val)
+      ? ''
+      : safeToStringImpl(val, seenArrays),
+  )
+  return mapped.join()
+}
+
+const safeToStringImpl = (
+  val: unknown,
+  seenArrays?: WeakSet<object>,
+): string => {
+  // Using .toString() fails for null/undefined and implicit conversion (val + "") fails for symbols
+  // and objects with null prototype
   if (val === undefined || val === null || typeof val.toString === 'function') {
-    return String(val)
+    return Array.isArray(val)
+      ? // Arrays have a weird custom toString that we need to replicate
+        safeArrayToString(val, seenArrays ?? new WeakSet())
+      : String(val)
   } else {
+    // This case should just be objects with null prototype, so we can just use Object#toString
     return objectToString(val)
   }
 }
 
+/** Safely converts any value to string, using the value's own `toString` when available. */
+export const safeToString = (val: unknown) => safeToStringImpl(val)
+
 /** Utility object for promise/callback interop. */
 export interface PromiseCallback<T> {
   promise: Promise<T>


Test patch:
diff --git a/lib/__tests__/util.spec.ts b/lib/__tests__/util.spec.ts
new file mode 100644
index 0000000..afcda6c
--- /dev/null
+++ b/lib/__tests__/util.spec.ts
@@ -0,0 +1,29 @@
+import { safeToString } from '../utils'
+
+describe('safeToString', () => {
+  const recursiveArray: unknown[] = [1]
+  recursiveArray.push([[recursiveArray], 2, [[recursiveArray]]], 3)
+  const testCases = [
+    [undefined, 'undefined'],
+    [null, 'null'],
+    [true, 'true'],
+    ['string', 'string'],
+    [123, '123'],
+    [321n, '321'],
+    [{ object: 'yes' }, '[object Object]'],
+    [(a: number, b: number) => a + b, '(a, b) => a + b'],
+    [Symbol('safeToString'), 'Symbol(safeToString)'],
+    [Object.create(null), '[object Object]'],
+    // eslint-disable-next-line no-sparse-arrays
+    [[1, 'hello', , undefined, , true, null], '1,hello,,,,true,'],
+    [
+      [Object.create(null), Symbol('safeToString')],
+      '[object Object],Symbol(safeToString)',
+    ],
+    [recursiveArray, '1,,2,,3'],
+  ]
+
+  it.each(testCases)('works on %s', (input, output) => {
+    expect(safeToString(input)).toBe(String(output))
+  })
+})