Skip to content

Commit cea807f

Browse files
committed
Keep vocabulary property arrays independent
Clones shared property arrays with their sources, so dereferencing a clone replaced values on the original without updating its trust state or cached JSON-LD. Plural constructor and clone inputs also retained caller-owned arrays. Copy arrays at these three ownership boundaries. Keep nested objects and URLs shared, preserve sparse inputs, and retain the public API and trust policy. Cover lookup and serialization isolation, caller and frozen arrays, cache reuse, and embedded object identity in regressions. Regenerate all runtime snapshots and add changelog fragments. Fixes #1207 Assisted-by: Codex:gpt-6-astra Assisted-by: Claude Code:claude-fable-5-1
1 parent fd218a8 commit cea807f

8 files changed

Lines changed: 1704 additions & 1258 deletions

File tree

‎CHANGES.md‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,23 @@ Version 2.0.31
88

99
To be released.
1010

11+
### @fedify/vocab
12+
13+
- Fixed shared property arrays in vocabulary objects so dereferencing a
14+
clone no longer changes its source's properties or serialization.
15+
Constructors and `clone()` also copy supplied plural-value arrays,
16+
allowing frozen arrays and preventing changes to the caller's arrays.
17+
Nested objects and URLs retain their identity. [[#1207], [#1208]]
18+
19+
[#1207]: https://github.com/fedify-dev/fedify/issues/1207
20+
[#1208]: https://github.com/fedify-dev/fedify/pull/1208
21+
22+
### @fedify/vocab-tools
23+
24+
- Fixed generated constructors and `clone()` methods to copy property
25+
arrays, preventing a clone's remote lookups from changing its source and
26+
preserving arrays supplied by callers. [[#1207], [#1208]]
27+
1128

1229
Version 2.0.30
1330
--------------
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
---
2+
links:
3+
'#1207': https://github.com/fedify-dev/fedify/issues/1207
4+
'#1208': https://github.com/fedify-dev/fedify/pull/1208
5+
---
6+
- Fixed generated constructors and `clone()` methods to copy property
7+
arrays, preventing a clone's remote lookups from changing its source and
8+
preserving arrays supplied by callers. [[#1207], [#1208]]
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
links:
3+
'#1207': https://github.com/fedify-dev/fedify/issues/1207
4+
'#1208': https://github.com/fedify-dev/fedify/pull/1208
5+
---
6+
- Fixed shared property arrays in vocabulary objects so dereferencing a
7+
clone no longer changes its source's properties or serialization.
8+
Constructors and `clone()` also copy supplied plural-value arrays,
9+
allowing frozen arrays and preventing changes to the caller's arrays.
10+
Nested objects and URLs retain their identity. [[#1207], [#1208]]

‎packages/vocab-tools/src/__snapshots__/class.test.ts.deno.snap‎

Lines changed: 484 additions & 418 deletions
Large diffs are not rendered by default.

‎packages/vocab-tools/src/__snapshots__/class.test.ts.node.snap‎

Lines changed: 484 additions & 418 deletions
Large diffs are not rendered by default.

‎packages/vocab-tools/src/__snapshots__/class.test.ts.snap‎

Lines changed: 484 additions & 418 deletions
Large diffs are not rendered by default.

‎packages/vocab-tools/src/constructor.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ export async function* generateConstructor(
175175
if (Array.isArray(values.${property.pluralName}) &&
176176
values.${property.pluralName}.every(v => ${typeGuards})) {
177177
// @ts-ignore: type is checked above.
178-
this.${fieldName} = values.${property.pluralName};
178+
this.${fieldName} = values.${property.pluralName}.slice();
179179
`;
180180
if (!allScalarTypes) {
181181
yield `
@@ -208,7 +208,8 @@ export async function* generateCloner(
208208
* Clones this instance, optionally updating it with the given values.
209209
* @param values The values to update the clone with.
210210
* @param options The options to use for cloning.
211-
* @returns The cloned instance.
211+
* @returns The cloned instance with its own property arrays. Nested objects
212+
* and URLs are shared with this instance.
212213
*/
213214
${emitOverride(typeUri, types)} clone(
214215
values:
@@ -245,7 +246,7 @@ export async function* generateCloner(
245246
const fieldName = await getFieldName(property.uri);
246247
const trustFieldName = await getFieldName(property.uri, "#_trust");
247248
const allScalarTypes = areAllScalarTypes(property.range, types);
248-
yield `clone.${fieldName} = this.${fieldName};`;
249+
yield `clone.${fieldName} = this.${fieldName}.slice();`;
249250
if (!allScalarTypes) {
250251
yield `clone.${trustFieldName} = new Set(this.${trustFieldName});`;
251252
}
@@ -309,7 +310,7 @@ export async function* generateCloner(
309310
if (Array.isArray(values.${property.pluralName}) &&
310311
values.${property.pluralName}.every(v => ${typeGuards})) {
311312
// @ts-ignore: type is checked above.
312-
clone.${fieldName} = values.${property.pluralName};
313+
clone.${fieldName} = values.${property.pluralName}.slice();
313314
`;
314315
if (!allScalarTypes) {
315316
yield `

‎packages/vocab/src/vocab.test.ts‎

Lines changed: 212 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,13 @@ import {
1212
notDeepStrictEqual,
1313
ok,
1414
rejects,
15+
strictEqual,
1516
throws,
1617
} from "node:assert/strict";
1718
import { assertInstanceOf } from "./utils.ts";
1819
import * as vocab from "./vocab.ts";
1920
import {
21+
Accept,
2022
Activity,
2123
Announce,
2224
Collection,
@@ -1279,6 +1281,216 @@ test("FEP-fe34: Trust tracking in object cloning", () => {
12791281
);
12801282
});
12811283

1284+
for (const property of ["object", "attachment"] as const) {
1285+
for (const fetchOriginal of [false, true]) {
1286+
test(
1287+
`clone() isolates ${property} dereferencing (${
1288+
fetchOriginal ? "original" : "clone"
1289+
} first)`,
1290+
async () => {
1291+
const embedded = {
1292+
id: "https://b.example/notes/1",
1293+
type: "Note",
1294+
content: "embedded",
1295+
};
1296+
const json = {
1297+
"@context": "https://www.w3.org/ns/activitystreams",
1298+
id: "https://a.example/activities/1",
1299+
type: "Create",
1300+
[property]: embedded,
1301+
};
1302+
const original = await Create.fromJsonLd(json, {
1303+
contextLoader: mockDocumentLoader,
1304+
});
1305+
const copy = original.clone();
1306+
const fetching = fetchOriginal ? original : copy;
1307+
const untouched = fetchOriginal ? copy : original;
1308+
let fetches = 0;
1309+
// deno-lint-ignore require-await
1310+
const documentLoader = async (url: string) => {
1311+
fetches++;
1312+
return {
1313+
contextUrl: null,
1314+
documentUrl: url,
1315+
document: {
1316+
"@context": "https://www.w3.org/ns/activitystreams",
1317+
...embedded,
1318+
content: "fetched",
1319+
},
1320+
};
1321+
};
1322+
const options = { documentLoader, contextLoader: mockDocumentLoader };
1323+
const fetched = property === "object"
1324+
? await fetching.getObject(options)
1325+
: (await Array.fromAsync(fetching.getAttachments(options)))[0];
1326+
assertInstanceOf(fetched, Note);
1327+
deepStrictEqual(fetched.content, "fetched");
1328+
1329+
// Inspect the other instance before it fetches anything itself.
1330+
const trusted = { crossOrigin: "trust" as const };
1331+
const preserved = property === "object"
1332+
? await untouched.getObject(trusted)
1333+
: (await Array.fromAsync(untouched.getAttachments(trusted)))[0];
1334+
assertInstanceOf(preserved, Note);
1335+
deepStrictEqual(preserved.content, "embedded");
1336+
const serialized = await original.toJsonLd() as Record<string, unknown>;
1337+
deepStrictEqual(
1338+
serialized[property],
1339+
fetchOriginal ? { ...embedded, content: "fetched" } : embedded,
1340+
);
1341+
const accept = new Accept({ object: untouched });
1342+
const acceptJson = await accept.toJsonLd() as {
1343+
object: Record<string, unknown>;
1344+
};
1345+
const nested = acceptJson.object;
1346+
deepStrictEqual(nested[property], embedded);
1347+
1348+
// Successful lookups and their trust metadata belong to each instance.
1349+
const cached = property === "object"
1350+
? await fetching.getObject(options)
1351+
: (await Array.fromAsync(fetching.getAttachments(options)))[0];
1352+
strictEqual(cached, fetched);
1353+
deepStrictEqual(fetches, 1);
1354+
const independentlyFetched = property === "object"
1355+
? await untouched.getObject(options)
1356+
: (await Array.fromAsync(untouched.getAttachments(options)))[0];
1357+
assertInstanceOf(independentlyFetched, Note);
1358+
deepStrictEqual(independentlyFetched.content, "fetched");
1359+
deepStrictEqual(fetches, 2);
1360+
const warmedClone = fetching.clone();
1361+
strictEqual(
1362+
property === "object"
1363+
? await warmedClone.getObject(options)
1364+
: (await Array.fromAsync(warmedClone.getAttachments(options)))[0],
1365+
fetched,
1366+
);
1367+
deepStrictEqual(fetches, 2);
1368+
},
1369+
);
1370+
}
1371+
}
1372+
1373+
for (const clone of [false, true]) {
1374+
for (const frozen of [false, true]) {
1375+
test(
1376+
`${clone ? "clone()" : "constructor"} copies ${
1377+
frozen ? "frozen" : "mutable"
1378+
} plural arrays`,
1379+
async () => {
1380+
const items = [
1381+
new URL("https://example.com/object"),
1382+
new URL("https://example.com/object"),
1383+
];
1384+
const before = items.slice();
1385+
if (frozen) globalThis.Object.freeze(items);
1386+
const base = new Collection({});
1387+
const collection = clone
1388+
? base.clone({ items })
1389+
: new Collection({ items });
1390+
const sibling = clone
1391+
? base.clone({ items })
1392+
: new Collection({ items });
1393+
const fetched = await Array.fromAsync(collection.getItems({
1394+
documentLoader: mockDocumentLoader,
1395+
contextLoader: mockDocumentLoader,
1396+
}));
1397+
deepStrictEqual(fetched.length, 2);
1398+
deepStrictEqual(fetched.map((item) => item.name), [
1399+
"Fetched object",
1400+
"Fetched object",
1401+
]);
1402+
deepStrictEqual(items, before);
1403+
strictEqual(items[0], before[0]);
1404+
strictEqual(items[1], before[1]);
1405+
const siblingJson = await sibling.toJsonLd() as { items: unknown };
1406+
deepStrictEqual(siblingJson.items, before.map(String));
1407+
if (!frozen) {
1408+
items.push(new URL("https://example.com/extra"));
1409+
deepStrictEqual(collection.itemIds, before);
1410+
deepStrictEqual(sibling.itemIds, before);
1411+
}
1412+
},
1413+
);
1414+
}
1415+
}
1416+
1417+
test("clone() copies scalar arrays without changing sparse inputs", () => {
1418+
const names = ["original"];
1419+
const original = new Object({ names });
1420+
names.push("caller mutation");
1421+
deepStrictEqual(original.names, ["original"]);
1422+
const copy = original.clone();
1423+
copy.names.push("clone mutation");
1424+
deepStrictEqual(original.names, ["original"]);
1425+
deepStrictEqual(copy.names, ["original", "clone mutation"]);
1426+
const replacements = ["replacement"];
1427+
const replaced = original.clone({ names: replacements });
1428+
replacements.push("caller mutation");
1429+
deepStrictEqual(replaced.names, ["replacement"]);
1430+
1431+
const sparseNames = new Array<string>(2);
1432+
sparseNames[1] = "kept";
1433+
const sparse = new Object({ names: sparseNames });
1434+
for (
1435+
const instance of [
1436+
sparse,
1437+
sparse.clone(),
1438+
sparse.clone({ names: sparse.names }),
1439+
]
1440+
) {
1441+
deepStrictEqual(instance.names.length, 2);
1442+
deepStrictEqual(0 in instance.names, false);
1443+
deepStrictEqual(instance.names[1], "kept");
1444+
}
1445+
});
1446+
1447+
test("clone() preserves embedded object identity and trust", async () => {
1448+
const note = new Note({ id: new URL("https://b.example/note") });
1449+
const original = new Create({
1450+
id: new URL("https://a.example/create"),
1451+
object: note,
1452+
});
1453+
// deno-lint-ignore require-await
1454+
const documentLoader = async () => {
1455+
throw new Error("Trusted embedded objects must not be fetched.");
1456+
};
1457+
for (
1458+
const instance of [
1459+
original,
1460+
original.clone(),
1461+
original.clone({ objects: [note] }),
1462+
]
1463+
) {
1464+
strictEqual(await instance.getObject({ documentLoader }), note);
1465+
strictEqual(instance.objectId, note.id);
1466+
}
1467+
});
1468+
1469+
test("clone() isolates crossOrigin trust when fetching", async () => {
1470+
const original = new Create({
1471+
id: new URL("https://a.example/create"),
1472+
object: new URL("https://b.example/note"),
1473+
});
1474+
const copy = original.clone();
1475+
// deno-lint-ignore require-await
1476+
const documentLoader = async (url: string) => ({
1477+
contextUrl: null,
1478+
documentUrl: url,
1479+
document: {
1480+
"@context": "https://www.w3.org/ns/activitystreams",
1481+
id: "https://other.example/note",
1482+
type: "Note",
1483+
content: "cross-origin",
1484+
},
1485+
});
1486+
const options = { documentLoader, contextLoader: mockDocumentLoader };
1487+
const trusted = await copy.getObject({ ...options, crossOrigin: "trust" });
1488+
assertInstanceOf(trusted, Note);
1489+
deepStrictEqual(trusted.content, "cross-origin");
1490+
deepStrictEqual(await original.getObject(options), null);
1491+
deepStrictEqual(original.objectId, new URL("https://b.example/note"));
1492+
});
1493+
12821494
test("FEP-fe34: crossOrigin ignore behavior (default)", async () => {
12831495
// Create a mock document loader that returns objects with different origins
12841496
// deno-lint-ignore require-await

0 commit comments

Comments
 (0)