Isolate property arrays in vocabulary clones

CHANGES.md

@@ -8,6 +8,23 @@ Version 2.0.31
 
 To be released.
 
+### @fedify/vocab
+
+ -  Fixed shared property arrays in vocabulary objects so dereferencing a
+    clone no longer changes its source's properties or serialization.
+    Constructors and `clone()` also copy supplied plural-value arrays,
+    allowing frozen arrays and preventing changes to the caller's arrays.
+    Nested objects and URLs retain their identity.  [[#1207], [#1208]]
+
+[#1207]: https://github.com/fedify-dev/fedify/issues/1207
+[#1208]: https://github.com/fedify-dev/fedify/pull/1208
+
+### @fedify/vocab-tools
+
+ -  Fixed generated constructors and `clone()` methods to copy property
+    arrays, preventing a clone's remote lookups from changing its source and
+    preserving arrays supplied by callers.  [[#1207], [#1208]]
+
 
 Version 2.0.30
 --------------

changes.d/vocab-tools/clone-array-ownership.md

@@ -0,0 +1,8 @@
+---
+links:
+  '#1207': https://github.com/fedify-dev/fedify/issues/1207
+  '#1208': https://github.com/fedify-dev/fedify/pull/1208
+---
+ -  Fixed generated constructors and `clone()` methods to copy property
+    arrays, preventing a clone's remote lookups from changing its source and
+    preserving arrays supplied by callers.  [[#1207], [#1208]]

changes.d/vocab/clone-array-ownership.md

@@ -0,0 +1,10 @@
+---
+links:
+  '#1207': https://github.com/fedify-dev/fedify/issues/1207
+  '#1208': https://github.com/fedify-dev/fedify/pull/1208
+---
+ -  Fixed shared property arrays in vocabulary objects so dereferencing a
+    clone no longer changes its source's properties or serialization.
+    Constructors and `clone()` also copy supplied plural-value arrays,
+    allowing frozen arrays and preventing changes to the caller's arrays.
+    Nested objects and URLs retain their identity.  [[#1207], [#1208]]

packages/vocab-tools/src/constructor.ts

@@ -175,7 +175,7 @@ export async function* generateConstructor(
           if (Array.isArray(values.${property.pluralName}) &&
               values.${property.pluralName}.every(v => ${typeGuards})) {
             // @ts-ignore: type is checked above.
-            this.${fieldName} = values.${property.pluralName};
+            this.${fieldName} = values.${property.pluralName}.slice();
       `;
       if (!allScalarTypes) {
         yield `
@@ -208,7 +208,8 @@ export async function* generateCloner(
    * Clones this instance, optionally updating it with the given values.
    * @param values The values to update the clone with.
    * @param options The options to use for cloning.
-   * @returns The cloned instance.
+   * @returns The cloned instance with its own property arrays.  Nested objects
+   *          and URLs are shared with this instance.
    */
   ${emitOverride(typeUri, types)} clone(
     values:
@@ -245,7 +246,7 @@ export async function* generateCloner(
     const fieldName = await getFieldName(property.uri);
     const trustFieldName = await getFieldName(property.uri, "#_trust");
     const allScalarTypes = areAllScalarTypes(property.range, types);
-    yield `clone.${fieldName} = this.${fieldName};`;
+    yield `clone.${fieldName} = this.${fieldName}.slice();`;
     if (!allScalarTypes) {
       yield `clone.${trustFieldName} = new Set(this.${trustFieldName});`;
     }
@@ -309,7 +310,7 @@ export async function* generateCloner(
           if (Array.isArray(values.${property.pluralName}) &&
               values.${property.pluralName}.every(v => ${typeGuards})) {
             // @ts-ignore: type is checked above.
-            clone.${fieldName} = values.${property.pluralName};
+            clone.${fieldName} = values.${property.pluralName}.slice();
       `;
       if (!allScalarTypes) {
         yield `

packages/vocab/src/vocab.test.ts

@@ -12,11 +12,13 @@ import {
   notDeepStrictEqual,
   ok,
   rejects,
+  strictEqual,
   throws,
 } from "node:assert/strict";
 import { assertInstanceOf } from "./utils.ts";
 import * as vocab from "./vocab.ts";
 import {
+  Accept,
   Activity,
   Announce,
   Collection,
@@ -1279,6 +1281,216 @@ test("FEP-fe34: Trust tracking in object cloning", () => {
   );
 });
 
+for (const property of ["object", "attachment"] as const) {
+  for (const fetchOriginal of [false, true]) {
+    test(
+      `clone() isolates ${property} dereferencing (${
+        fetchOriginal ? "original" : "clone"
+      } first)`,
+      async () => {
+        const embedded = {
+          id: "https://b.example/notes/1",
+          type: "Note",
+          content: "embedded",
+        };
+        const json = {
+          "@context": "https://www.w3.org/ns/activitystreams",
+          id: "https://a.example/activities/1",
+          type: "Create",
+          [property]: embedded,
+        };
+        const original = await Create.fromJsonLd(json, {
+          contextLoader: mockDocumentLoader,
+        });
+        const copy = original.clone();
+        const fetching = fetchOriginal ? original : copy;
+        const untouched = fetchOriginal ? copy : original;
+        let fetches = 0;
+        // deno-lint-ignore require-await
+        const documentLoader = async (url: string) => {
+          fetches++;
+          return {
+            contextUrl: null,
+            documentUrl: url,
+            document: {
+              "@context": "https://www.w3.org/ns/activitystreams",
+              ...embedded,
+              content: "fetched",
+            },
+          };
+        };
+        const options = { documentLoader, contextLoader: mockDocumentLoader };
+        const fetched = property === "object"
+          ? await fetching.getObject(options)
+          : (await Array.fromAsync(fetching.getAttachments(options)))[0];
+        assertInstanceOf(fetched, Note);
+        deepStrictEqual(fetched.content, "fetched");
+
+        // Inspect the other instance before it fetches anything itself.
+        const trusted = { crossOrigin: "trust" as const };
+        const preserved = property === "object"
+          ? await untouched.getObject(trusted)
+          : (await Array.fromAsync(untouched.getAttachments(trusted)))[0];
+        assertInstanceOf(preserved, Note);
+        deepStrictEqual(preserved.content, "embedded");
+        const serialized = await original.toJsonLd() as Record<string, unknown>;
+        deepStrictEqual(
+          serialized[property],
+          fetchOriginal ? { ...embedded, content: "fetched" } : embedded,
+        );
+        const accept = new Accept({ object: untouched });
+        const acceptJson = await accept.toJsonLd() as {
+          object: Record<string, unknown>;
+        };
+        const nested = acceptJson.object;
+        deepStrictEqual(nested[property], embedded);
+
+        // Successful lookups and their trust metadata belong to each instance.
+        const cached = property === "object"
+          ? await fetching.getObject(options)
+          : (await Array.fromAsync(fetching.getAttachments(options)))[0];
+        strictEqual(cached, fetched);
+        deepStrictEqual(fetches, 1);
+        const independentlyFetched = property === "object"
+          ? await untouched.getObject(options)
+          : (await Array.fromAsync(untouched.getAttachments(options)))[0];
+        assertInstanceOf(independentlyFetched, Note);
+        deepStrictEqual(independentlyFetched.content, "fetched");
+        deepStrictEqual(fetches, 2);
+        const warmedClone = fetching.clone();
+        strictEqual(
+          property === "object"
+            ? await warmedClone.getObject(options)
+            : (await Array.fromAsync(warmedClone.getAttachments(options)))[0],
+          fetched,
+        );
+        deepStrictEqual(fetches, 2);
+      },
+    );
+  }
+}
+
+for (const clone of [false, true]) {
+  for (const frozen of [false, true]) {
+    test(
+      `${clone ? "clone()" : "constructor"} copies ${
+        frozen ? "frozen" : "mutable"
+      } plural arrays`,
+      async () => {
+        const items = [
+          new URL("https://example.com/object"),
+          new URL("https://example.com/object"),
+        ];
+        const before = items.slice();
+        if (frozen) globalThis.Object.freeze(items);
+        const base = new Collection({});
+        const collection = clone
+          ? base.clone({ items })
+          : new Collection({ items });
+        const sibling = clone
+          ? base.clone({ items })
+          : new Collection({ items });
+        const fetched = await Array.fromAsync(collection.getItems({
+          documentLoader: mockDocumentLoader,
+          contextLoader: mockDocumentLoader,
+        }));
+        deepStrictEqual(fetched.length, 2);
+        deepStrictEqual(fetched.map((item) => item.name), [
+          "Fetched object",
+          "Fetched object",
+        ]);
+        deepStrictEqual(items, before);
+        strictEqual(items[0], before[0]);
+        strictEqual(items[1], before[1]);
+        const siblingJson = await sibling.toJsonLd() as { items: unknown };
+        deepStrictEqual(siblingJson.items, before.map(String));
+        if (!frozen) {
+          items.push(new URL("https://example.com/extra"));
+          deepStrictEqual(collection.itemIds, before);
+          deepStrictEqual(sibling.itemIds, before);
+        }
+      },
+    );
+  }
+}
+
+test("clone() copies scalar arrays without changing sparse inputs", () => {
+  const names = ["original"];
+  const original = new Object({ names });
+  names.push("caller mutation");
+  deepStrictEqual(original.names, ["original"]);
+  const copy = original.clone();
+  copy.names.push("clone mutation");
+  deepStrictEqual(original.names, ["original"]);
+  deepStrictEqual(copy.names, ["original", "clone mutation"]);
+  const replacements = ["replacement"];
+  const replaced = original.clone({ names: replacements });
+  replacements.push("caller mutation");
+  deepStrictEqual(replaced.names, ["replacement"]);
+
+  const sparseNames = new Array<string>(2);
+  sparseNames[1] = "kept";
+  const sparse = new Object({ names: sparseNames });
+  for (
+    const instance of [
+      sparse,
+      sparse.clone(),
+      sparse.clone({ names: sparse.names }),
+    ]
+  ) {
+    deepStrictEqual(instance.names.length, 2);
+    deepStrictEqual(0 in instance.names, false);
+    deepStrictEqual(instance.names[1], "kept");
+  }
+});
+
+test("clone() preserves embedded object identity and trust", async () => {
+  const note = new Note({ id: new URL("https://b.example/note") });
+  const original = new Create({
+    id: new URL("https://a.example/create"),
+    object: note,
+  });
+  // deno-lint-ignore require-await
+  const documentLoader = async () => {
+    throw new Error("Trusted embedded objects must not be fetched.");
+  };
+  for (
+    const instance of [
+      original,
+      original.clone(),
+      original.clone({ objects: [note] }),
+    ]
+  ) {
+    strictEqual(await instance.getObject({ documentLoader }), note);
+    strictEqual(instance.objectId, note.id);
+  }
+});
+
+test("clone() isolates crossOrigin trust when fetching", async () => {
+  const original = new Create({
+    id: new URL("https://a.example/create"),
+    object: new URL("https://b.example/note"),
+  });
+  const copy = original.clone();
+  // deno-lint-ignore require-await
+  const documentLoader = async (url: string) => ({
+    contextUrl: null,
+    documentUrl: url,
+    document: {
+      "@context": "https://www.w3.org/ns/activitystreams",
+      id: "https://other.example/note",
+      type: "Note",
+      content: "cross-origin",
+    },
+  });
+  const options = { documentLoader, contextLoader: mockDocumentLoader };
+  const trusted = await copy.getObject({ ...options, crossOrigin: "trust" });
+  assertInstanceOf(trusted, Note);
+  deepStrictEqual(trusted.content, "cross-origin");
+  deepStrictEqual(await original.getObject(options), null);
+  deepStrictEqual(original.objectId, new URL("https://b.example/note"));
+});
+
 test("FEP-fe34: crossOrigin ignore behavior (default)", async () => {
   // Create a mock document loader that returns objects with different origins
   // deno-lint-ignore require-await