diff --git a/CHANGELOG.md b/CHANGELOG.md index dcd02fe..5e1c5cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,9 +31,8 @@ `collection`, `id` and `occurrenceKey` directly where they rode the opaque carry. - Fix an image inside another image's description: it flattens into the alt text, where it was refused. -- Fix a cyclic document, in which a node or an attribute value holds itself: `adfToMarkdown` and - `adfToPlainMarkdown` refuse it as `not-an-adf-document` and `isAdfDocument` returns `false`, - where all three hung. +- Fix a cyclic document, in which an object holds itself: `adfToMarkdown` and `adfToPlainMarkdown` + refuse it as `not-an-adf-document` and `isAdfDocument` returns `false`, where all three hung. ## 0.1.0 diff --git a/README.md b/README.md index 40826c5..71cde23 100644 --- a/README.md +++ b/README.md @@ -185,7 +185,7 @@ Emitting — `adfToMarkdown`, and `adfToHtml` at `0.2.0`: | Code | Fires when | What you can do | | --- | --- | --- | -| `not-an-adf-document` | the value handed in is no ADF document — a missing or wrong `type`, a stray key, a node that is not a node, a node or attribute value holding itself | guard the boundary you receive JSON at with `isAdfDocument`; the message names the branch that refused | +| `not-an-adf-document` | the value handed in is no ADF document — a missing or wrong `type`, a stray key, a node that is not a node, an object in the document holding itself | guard the boundary you receive JSON at with `isAdfDocument`; the message names the branch that refused | | `unsupported-document-version` | the document's `version` is not 1 | keep the ADF and pass the document over, or show it read-only; the version is the site's, not yours to change | Either direction. The emitter's own refusals are in this group — a parse reaches them by asking it diff --git a/docs/decisions.md b/docs/decisions.md index 789633c..415f3a2 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -385,9 +385,9 @@ input reads `message`. it names the escape that unclaims the form claimed: `\!adf:` for a directive, block line and inline alike, `\|` for every pipe row. - `not-an-adf-document` carries the document's own path throughout: seven of the guard's nine - branches read the document's own shape, and threading a path to the other two — a malformed node, - or an object holding itself, anywhere in the tree — wants the manual stack §Nothing recurses unbounded forces. The message - names the violation instead. + branches read the document's own shape, and threading a path to the other two, which look anywhere + in the tree — a malformed node, or an object holding itself — wants the manual stack §Nothing + recurses unbounded forces. The message names the violation instead. ## Publish on a version bump diff --git a/src/adf/document.test.ts b/src/adf/document.test.ts index bb9ccab..af13f46 100644 --- a/src/adf/document.test.ts +++ b/src/adf/document.test.ts @@ -84,7 +84,7 @@ test("reads a node's siblings as a walk rather than as one call's arguments", () }) test('refuses a node or an attribute value holding itself, and accepts one shared', () => { - const holdsItself = 'an ADF document is a tree: a node or an attribute value holds itself' + const holdsItself = 'an ADF document is a tree: an object in it holds itself' const paragraph: { content: unknown[]; type: string } = { content: [], type: 'paragraph' } paragraph.content.push(paragraph) assert.equal(fault({ content: [paragraph], type: 'doc', version: 1 }), holdsItself) @@ -96,6 +96,18 @@ test('refuses a node or an attribute value holding itself, and accepts one share assert.equal(fault({ content: [{ attrs, type: 'paragraph' }], type: 'doc', version: 1 }), holdsItself) assert.equal(fault({ content: [{ marks: [{ attrs, type: 'link' }], text: 'x', type: 'text' }], type: 'doc', version: 1 }), holdsItself) assert.equal(isAdfDocument({ content: [{ attrs, type: 'paragraph' }], type: 'doc', version: 1 }), false) + const notANode = "an ADF document's content holds ADF nodes: one of them is not" + const inherits: { content?: unknown[] } = {} + const heir: Record = Object.create(inherits) + heir['type'] = 'paragraph' + inherits.content = [heir] + assert.equal(fault({ content: [heir], type: 'doc', version: 1 }), notANode) + const hidden: Record = { type: 'paragraph' } + Object.defineProperty(hidden, 'content', { value: [hidden] }) + assert.equal(fault({ content: [hidden], type: 'doc', version: 1 }), notANode) + const link: Record = Object.create({ attrs }) + link['type'] = 'link' + assert.equal(fault({ content: [{ marks: [link], text: 'x', type: 'text' }], type: 'doc', version: 1 }), notANode) const bold = { type: 'strong' } const text = { marks: [bold], text: 'x', type: 'text' } const shared = { content: [text, text], type: 'paragraph' } diff --git a/src/adf/document.ts b/src/adf/document.ts index 3e138eb..7c4060b 100644 --- a/src/adf/document.ts +++ b/src/adf/document.ts @@ -43,7 +43,7 @@ export function adfDocumentFault(value: unknown): ConvertFault | undefined { const held: unknown = value['content'] if (!Array.isArray(held)) return notADocument(`an ADF document's content is an array: found ${describe(held)}`) const content: readonly unknown[] = held - if (holdsItself(content)) return notADocument('an ADF document is a tree: a node or an attribute value holds itself') + if (holdsItself(content)) return notADocument('an ADF document is a tree: an object in it holds itself') if (!isNodeArray(content)) return notADocument("an ADF document's content holds ADF nodes: one of them is not") return nestingFault(content) } @@ -106,7 +106,7 @@ export function isAdfNode(value: unknown): value is AdfNode { } export function isAdfMark(value: unknown): value is AdfMark { - if (!isRecord(value) || !holdsOnly(value, markKeys)) return false + if (!isRecord(value) || !holdsOnly(value, markKeys) || !ownsWhatItHolds(value, markKeys)) return false if (typeof value['type'] !== 'string') return false return !('attrs' in value) || isAttributes(value['attrs']) } @@ -123,15 +123,15 @@ export function nodeMarks(node: { marks?: AdfMark[] }): readonly AdfMark[] { return node.marks ?? [] } -// Ancestors alone: a tree may share an object, but an object holding itself walks the guards forever. +// Tracks ancestors alone: a tree may share an object, but an object holding itself walks the guards forever. function holdsItself(root: object): boolean { const pending = [{ depth: 0, item: root }] const ancestors: object[] = [] - const onPath = new Set() + const onPath = new Set() while (pending.length > 0) { const entry = pending.pop() if (entry === undefined) continue - for (const left of ancestors.splice(entry.depth)) onPath.delete(left) + while (ancestors.length > entry.depth) onPath.delete(ancestors.pop()) if (onPath.has(entry.item)) return true ancestors.push(entry.item) onPath.add(entry.item) @@ -144,7 +144,7 @@ function isNodeArray(value: readonly unknown[]): value is readonly AdfNode[] { const pending: unknown[] = [...value] while (pending.length > 0) { const node = pending.pop() - if (!isRecord(node) || !holdsOnly(node, nodeKeys)) return false + if (!isRecord(node) || !holdsOnly(node, nodeKeys) || !ownsWhatItHolds(node, nodeKeys)) return false if (typeof node['type'] !== 'string') return false if ('attrs' in node && !isAttributes(node['attrs'])) return false if ('marks' in node && !isArrayOf(node['marks'], isAdfMark)) return false @@ -219,6 +219,11 @@ function extraKey(value: Record, keys: readonly string[]): stri return Object.keys(value).find((key) => !keys.includes(key)) } +// holdsItself walks own enumerable keys alone, so a guard reading an inherited one could walk a cycle forever. +function ownsWhatItHolds(value: Record, keys: readonly string[]): boolean { + return keys.every((key) => !(key in value) || Object.prototype.propertyIsEnumerable.call(value, key)) +} + function holdsOnly(value: Record, keys: readonly string[]): boolean { return extraKey(value, keys) === undefined }