From 61f11e534d77ae778a051606c584777f3ebfc8c3 Mon Sep 17 00:00:00 2001 From: Anton Wiklund Date: Sun, 8 Nov 2026 05:54:39 +0100 Subject: [PATCH] fix(domain): read a deck group position too large to hold as none A deck group's sm:position is an xsd:integer of 0 or more, and the reader dropped only a negative one. A valid integer too large for a JavaScript number read as Infinity, or as a rounded number, and stayed on the group. Renaming the group, or moving a deck into it, wrote that value back, and the write check refused the result, so the edit failed until the group itself was moved. A position now counts only when it is a whole number of 0 or more that a number holds exactly. Any other is read as none, for a group and a deck alike, so the node sorts last and a group written again is written without it, or renumbered when its level is reordered. Refs: GHSA-438g-wf4q-xj9g --- docs/data-model.md | 8 +++-- packages/domain/src/deckGroupRecord.test.ts | 10 ++++-- packages/domain/src/deckGroupRecord.ts | 17 +++++++-- packages/solid/src/deckTree.test.ts | 35 +++++++++++++++++++ .../solid/src/mappers/deckTreeMapper.test.ts | 20 +++++++++++ packages/solid/src/mappers/deckTreeMapper.ts | 9 +++-- 6 files changed, 89 insertions(+), 10 deletions(-) diff --git a/docs/data-model.md b/docs/data-model.md index dfebe567e..1926c41f2 100644 --- a/docs/data-model.md +++ b/docs/data-model.md @@ -980,8 +980,12 @@ A cycle is broken by moving its lowest-URL group to the top level. A group nested deeper than 16 groups (`MAX_GROUP_DEPTH`) is read at the top level, with the groups it holds counting their depth from there. The editor doesn't stop a move that nests a group deeper, so the next -read shows that group at the top level. A negative position counts as none; on a tie of positions, decks come -before groups, each kind in document order. A group in a format newer +read shows that group at the top level. A negative position counts as +none, and so does one larger than the app holds exactly (2^53 − 1): +the node sorts last, and a group written again is written without it, +or renumbered when its level is reordered. On a tie of positions, +decks come before groups, each kind in document order. A group in a +format newer than the app's makes the arrangement read-only. So does, under the "Set invalid data aside" policy (`sm:blockSubject`, the default), a catalog or group that doesn't conform, or that check still running diff --git a/packages/domain/src/deckGroupRecord.test.ts b/packages/domain/src/deckGroupRecord.test.ts index d4b775b81..b9d3605a3 100644 --- a/packages/domain/src/deckGroupRecord.test.ts +++ b/packages/domain/src/deckGroupRecord.test.ts @@ -35,10 +35,14 @@ describe("deck group records", () => { expect(deckGroupFromRecord(URL, record)).toEqual(stored); }); - it("read a negative position as none", () => { + it("read a negative position, or one too large to hold exactly, as none", () => { const stored = { group: { url: URL, title: { en: "Languages" } }, decks: [], groups: [] }; - const record = deckGroupToRecord({ ...stored, position: -1 }, PUBLISHER); - expect(deckGroupFromRecord(URL, record)).toEqual(stored); + for (const position of [-1, Infinity, Number.MAX_SAFE_INTEGER + 1, 0.5]) { + const record = deckGroupToRecord({ ...stored, position }, PUBLISHER); + expect(deckGroupFromRecord(URL, record)).toEqual(stored); + } + const largest = deckGroupToRecord({ ...stored, position: Number.MAX_SAFE_INTEGER }, PUBLISHER); + expect(deckGroupFromRecord(URL, largest).position).toBe(Number.MAX_SAFE_INTEGER); }); it("keep a description the user wrote, and renew the default one on a rename", () => { diff --git a/packages/domain/src/deckGroupRecord.ts b/packages/domain/src/deckGroupRecord.ts index 657ebdc9b..cd338d0e9 100644 --- a/packages/domain/src/deckGroupRecord.ts +++ b/packages/domain/src/deckGroupRecord.ts @@ -6,12 +6,23 @@ import type { DeckGroupV1 } from "@solid-memo/vocab/types.generated"; import { defaultDeckGroupDescription, isDefaultDeckGroupDescription } from "./dcat"; import type { StoredGroup } from "./deckTree"; +/** + * Whether a stored `sm:position` is one the tree keeps: a whole number, + * 0 or more, that a JavaScript number holds exactly. A negative one, + * which the shape does not allow, and one too large to hold, which reads + * as Infinity or as a rounded number, count as none, so that no node is + * written back with one. + */ +export function usablePosition(position: number | null | undefined): position is number { + return Number.isSafeInteger(position) && position! >= 0; +} + /** * A deck group between its shape record and the model (domain/deckTree.ts). * The record states what the model leaves to the pod: the group's * description, which DCAT-AP asks for and the app writes for the user, - * and its publisher, the catalog's. A negative position, which the - * shape does not allow and the tree reads as none, is left out, so that + * and its publisher, the catalog's. A position that is not usable + * (usablePosition) is left out, so that the tree reads it as none and * the group is not written back with it. */ export function deckGroupFromRecord(url: string, data: DeckGroupV1): StoredGroup { @@ -19,7 +30,7 @@ export function deckGroupFromRecord(url: string, data: DeckGroupV1): StoredGroup group: { url, title: data.title }, decks: [...data.dataset], groups: [...data.catalog], - ...(data.position === undefined || data.position < 0 ? {} : { position: data.position }), + ...(usablePosition(data.position) ? { position: data.position } : {}), }; } diff --git a/packages/solid/src/deckTree.test.ts b/packages/solid/src/deckTree.test.ts index 12abc667e..78327f041 100644 --- a/packages/solid/src/deckTree.test.ts +++ b/packages/solid/src/deckTree.test.ts @@ -5,6 +5,8 @@ import { describe, expect, it, vi } from "vitest"; import type { Deck } from "@solid-memo/domain/deck"; import { decksOf, nodeId, type DeckTree, type TreeNode } from "@solid-memo/domain/deckTree"; +import { SHAPE_SOURCES, shapesFetch } from "@solid-memo/vocab/tooling/testing/shapeSources"; +import { createShaclShapeValidator } from "./shaclShapeValidator"; import { createSolidDeckRepository } from "./solidDeckRepository"; import { fakePod } from "./testing/fakePod"; import type { WriteCheck } from "./writeCheck"; @@ -198,4 +200,37 @@ describe("the deck tree in the pod", () => { expect(pod.triples(CATALOG)!.some((triple) => triple.includes(`<${a.url}>`))).toBe(false); expect(after.children.map(nodeId)).toEqual([group.url, at("c")]); }); + + it("renames and fills a group whose position is too large to read, under the real write check", async () => { + // A valid xsd:integer this long reads as Infinity: it counts as none and is not written back. + const huge = "9".repeat(400); + const pod = await podWith( + `<#catalog> dcat:catalog <#group-1> . + <#group-1> a sm:DeckGroup, dcat:Catalog ; sm:formatVersion 1 ; dcterms:title "Languages"@en ; + dcterms:description "Deck group: Languages."@en ; dcterms:publisher ; + dcat:dataset <#a> ; sm:position ${huge} . + <#a> sm:position ${huge} . + a ; + "Me" .`, + ); + const validator = createShaclShapeValidator({ + fetch: pod.fetch, + shapesFetch, + ...SHAPE_SOURCES, + }).over(pod.fetch); + const repository = repositoryOn(pod.fetch, validator.checkSubjects); + const renamed = await repository.editDeckTree(INSTANCE, { + kind: "rename", + group: group.url, + title: { en: "Words" }, + }); + expect(shape(renamed.children)).toEqual(["b", "c", { "group-1": ["a"] }]); + const filled = await repository.editDeckTree(INSTANCE, { + kind: "move", + node: at("b"), + to: { parent: group.url, after: at("a") }, + }); + expect(shape(filled.children)).toEqual(["c", { "group-1": ["a", "b"] }]); + expect(pod.triples(CATALOG)!.some((triple) => triple.includes(huge))).toBe(false); + }); }); diff --git a/packages/solid/src/mappers/deckTreeMapper.test.ts b/packages/solid/src/mappers/deckTreeMapper.test.ts index f129a3196..b94b61741 100644 --- a/packages/solid/src/mappers/deckTreeMapper.test.ts +++ b/packages/solid/src/mappers/deckTreeMapper.test.ts @@ -162,6 +162,26 @@ describe("withTreeChanges", () => { expect(getInteger(y, SM.position)).toBeNull(); }); + it("reads a position too large to hold as none, on a group and a deck, and writes the group without it", async () => { + const huge = "9".repeat(400); + const dataset = await document( + rootTtl("; dcat:catalog <#group-x>") + + deckTtl("a", `; sm:position ${huge} `) + + deckTtl("b", "; sm:position 9007199254740992 ") + + deckTtl("c") + + groupTtl("group-x", "X", "; dcat:dataset <#a>", `; sm:position ${huge} `), + ); + const layout = toStoredLayout(dataset, CATALOG); + expect(layout.groups[0]).not.toHaveProperty("position"); + expect([...layout.deckPositions]).toEqual([]); + const renamed = edited(dataset, { + kind: "rename", + group: at("group-x"), + title: { en: "Y" }, + }).dataset; + expect(getInteger(getThing(renamed, at("group-x"))!, SM.position)).toBeNull(); + }); + it("drops a negative position, which the shape does not allow, when it writes the group", async () => { const dataset = await document( rootTtl("; dcat:catalog <#group-x>") + diff --git a/packages/solid/src/mappers/deckTreeMapper.ts b/packages/solid/src/mappers/deckTreeMapper.ts index a530b60e0..07b93418f 100644 --- a/packages/solid/src/mappers/deckTreeMapper.ts +++ b/packages/solid/src/mappers/deckTreeMapper.ts @@ -12,7 +12,11 @@ import { getUrlAll, } from "../client/rdf"; import { editOf } from "../datasetEdit"; -import { deckGroupFromRecord, deckGroupToRecord } from "@solid-memo/domain/deckGroupRecord"; +import { + deckGroupFromRecord, + deckGroupToRecord, + usablePosition, +} from "@solid-memo/domain/deckGroupRecord"; import type { StoredGroup, StoredLayout, TreeChanges } from "@solid-memo/domain/deckTree"; import { migrate } from "@solid-memo/domain/shapes/migrations"; import { DECK_GROUP_V1 } from "@solid-memo/vocab/descriptors.generated"; @@ -60,7 +64,8 @@ export function toStoredLayout(dataset: Dataset, catalogUrl: string): StoredLayo const deckPositions = new Map(); for (const deck of decks) { const position = getInteger(getThing(dataset, deck.url)!, SM.position); - if (position !== null) deckPositions.set(deck.url, position); + // A position the tree would not keep is none, as a group's is (usablePosition). + if (usablePosition(position)) deckPositions.set(deck.url, position); } const root = getThing(dataset, `${catalogUrl}#catalog`); return {