fix: use an ArrayBuffer for delegation bits in AgentData (storacha#1219)

we noticed that Safari users couldn't log in:

storacha/console#46

I tracked this down to a seemingly unreported WebKit bug where IndexedDB
throws an error when saving `Uint8Array`s in an object that uses
inline-keys. I know these two things don't seem related, but there does
seem to be some similar precedent:

dexie/Dexie.js#656 (comment)

This change tweaks `AgentData`'s export format to use `ArrayBuffer`s
rather than `Uint8Array`s, which should solve the issue. I was able to
handle coercing back into Uint8Arrays pretty elegantly, and introduced a
new `AgentDataImport` type to keep things typesafe.

packages/access-client/src/agent-data.js

@@ -5,6 +5,7 @@ import * as Ucanto from '@ucanto/interface'
 import { CID } from 'multiformats'
 import { UCAN } from '@web3-storage/capabilities'
 import { isExpired } from './delegations.js'
+import { uint8ArrayToArrayBuffer } from './utils/buffers.js'
 
 /** @typedef {import('./types.js').AgentDataModel} AgentDataModel */
 
@@ -65,7 +66,8 @@ export class AgentData {
         delegation: importDAG(
           value.delegation.map((d) => ({
             cid: CID.parse(d.cid).toV1(),
-            bytes: d.bytes,
+            bytes:
+              d.bytes instanceof Uint8Array ? d.bytes : new Uint8Array(d.bytes),
           }))
         ),
         meta: value.meta,
@@ -102,7 +104,7 @@ export class AgentData {
         meta: value.meta,
         delegation: [...value.delegation.export()].map((b) => ({
           cid: b.cid.toString(),
-          bytes: b.bytes,
+          bytes: uint8ArrayToArrayBuffer(b.bytes),
         })),
       })
     }

packages/access-client/src/types.ts

@@ -168,7 +168,7 @@ export type AgentDataExport = Pick<
     CIDString,
     {
       meta: DelegationMeta
-      delegation: Array<{ cid: CIDString; bytes: Uint8Array }>
+      delegation: Array<{ cid: CIDString; bytes: ArrayBuffer }>
     }
   >
 }

packages/access-client/src/utils/buffers.js

@@ -0,0 +1,21 @@
+/**
+ * Convert a Uint8Array to an ArrayBuffer, taking into account
+ * that we may be looking at a "data view".
+ * thanks, https://stackoverflow.com/a/54646864
+ *
+ * If we aren't looking at a data view, simply returns the underlying ArrayBuffer
+ * directly.
+ *
+ * @param {Uint8Array} array
+ * @returns ArrayBuffer
+ */
+export function uint8ArrayToArrayBuffer(array) {
+  if (array.byteOffset === 0 && array.byteLength === array.buffer.byteLength) {
+    return array.buffer
+  } else {
+    return array.buffer.slice(
+      array.byteOffset,
+      array.byteLength + array.byteOffset
+    )
+  }
+}

packages/access-client/test/utils/buffers.test.js

@@ -0,0 +1,60 @@
+import assert from 'assert'
+import { uint8ArrayToArrayBuffer } from '../../src/utils/buffers.js'
+
+/**
+ *
+ * @param {any[]} array
+ * @returns ArrayBuffer
+ */
+function arrayBufferFromArray(array) {
+  return Uint8Array.from(array).buffer
+}
+
+describe('uint8ArrayToArrayBuffer', function () {
+  it('should convert an empty Uint8Array to an empty ArrayBuffer', function () {
+    assert.deepEqual(
+      uint8ArrayToArrayBuffer(Uint8Array.from([])),
+      new ArrayBuffer(0)
+    )
+  })
+
+  it('should convert a Uint8Array with a few elements to an ArrayBuffer with the same elements', function () {
+    assert.deepEqual(
+      uint8ArrayToArrayBuffer(Uint8Array.from([1, 2, 3])),
+      arrayBufferFromArray([1, 2, 3])
+    )
+  })
+
+  it('should return the ArrayBuffer instance underlying the Uint8Array if the Uint8Array represents the whole array', function () {
+    const uint8Array = Uint8Array.from([1, 2, 3])
+    assert.strictEqual(uint8ArrayToArrayBuffer(uint8Array), uint8Array.buffer)
+  })
+
+  it('should convert a Uint8Array that is a view over an ArrayBuffer to an ArrayBuffer with the same elements', function () {
+    const originalArrayBuffer = Uint8Array.from([1, 2, 3, 4, 5, 6, 7, 8]).buffer
+    const uint8Array = new Uint8Array(originalArrayBuffer, 3, 3)
+    // uint8Array is a view on the original buffer
+    assert.deepEqual(uint8Array, Uint8Array.from([4, 5, 6]))
+    // if we just get the uint8Array.buffer it is equal to the whole original buffer, not the uint8array's view
+    assert.deepEqual(
+      uint8Array.buffer,
+      arrayBufferFromArray([1, 2, 3, 4, 5, 6, 7, 8])
+    )
+    // but if we use uint8ArrayToArraybuffer it just returns an ArrayBuffer of the Uint8Array's view
+    assert.deepEqual(
+      uint8ArrayToArrayBuffer(uint8Array),
+      arrayBufferFromArray([4, 5, 6])
+    )
+  })
+})
+
+// this test simply verifies that a Uint8Array doesn't make a copy of an ArrayBuffer
+// passed to its constructor - technically this is verifying the implementation
+// of the JavaScript platform behaves as advertised, but it's an important part
+// of our performance assumptions about AgentData so worth proving here
+describe('new Uint8Array()', function () {
+  it('should use a passed ArrayBuffer as its underlying buffer', function () {
+    const arrayBuffer = arrayBufferFromArray([1, 2, 3])
+    assert.strictEqual(new Uint8Array(arrayBuffer).buffer, arrayBuffer)
+  })
+})