diff --git a/CHANGELOG.md b/CHANGELOG.md index 3123e48..70581f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +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 document holding itself, through a node's `content` or an attribute: `adfToMarkdown`, + `adfToPlainMarkdown` and `isAdfDocument` refuse it as `not-an-adf-document`, where they hung. ## 0.1.0 diff --git a/src/adf/document.test.ts b/src/adf/document.test.ts index 20e3b37..5a7d9b1 100644 --- a/src/adf/document.test.ts +++ b/src/adf/document.test.ts @@ -82,3 +82,23 @@ test("reads a node's siblings as a walk rather than as one call's arguments", () assert.equal(isAdfDocument(wide), true) assert.equal(fault(wide), 'accepted') }) + +test('refuses a document that holds itself, and accepts one sharing a node or a mark', () => { + const notANode = "an ADF document's content holds ADF nodes: one of them is not" + const paragraph: { content: unknown[]; type: string } = { content: [], type: 'paragraph' } + paragraph.content.push(paragraph) + assert.equal(fault({ content: [paragraph], type: 'doc', version: 1 }), notANode) + const document: { content: unknown[]; type: string; version: number } = { content: [], type: 'doc', version: 1 } + document.content.push(document) + assert.equal(fault(document), notANode) + const attrs: Record = {} + attrs['a'] = [attrs] + assert.equal(fault({ content: [{ attrs, type: 'paragraph' }], type: 'doc', version: 1 }), notANode) + assert.equal(fault({ content: [{ marks: [{ attrs, type: 'link' }], text: 'x', type: 'text' }], type: 'doc', version: 1 }), notANode) + assert.equal(isAdfDocument({ content: [{ attrs, type: 'paragraph' }], type: 'doc', version: 1 }), false) + const bold = { type: 'strong' } + const text = { marks: [bold], text: 'x', type: 'text' } + const shared = { content: [text, text], type: 'paragraph' } + const values = { b: 1 } + assert.equal(fault({ content: [shared, shared, { attrs: { a: [values, values] }, type: 'paragraph' }], type: 'doc', version: 1 }), 'accepted') +}) diff --git a/src/adf/document.ts b/src/adf/document.ts index 772dd34..aa67faa 100644 --- a/src/adf/document.ts +++ b/src/adf/document.ts @@ -1,5 +1,5 @@ import type { ConvertFault } from '../result.ts' -import { isJsonValue, overNested, type JsonValue } from '../json-value.ts' +import { everyInTree, isJsonValue, overNested, type JsonValue } from '../json-value.ts' import { largestNesting } from '../nesting.ts' import { serializeCanonicalJson } from '../canonical-json.ts' @@ -123,21 +123,16 @@ export function nodeMarks(node: { marks?: AdfMark[] }): readonly AdfMark[] { } 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 (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 - if ('text' in node && typeof node['text'] !== 'string') return false - if ('content' in node) { - const content = node['content'] - if (!Array.isArray(content)) return false - for (const child of content) pending.push(child) - } - } - return true + return everyInTree(value, (node) => { + if (!isRecord(node) || !holdsOnly(node, nodeKeys)) return undefined + if (typeof node['type'] !== 'string') return undefined + if ('attrs' in node && !isAttributes(node['attrs'])) return undefined + if ('marks' in node && !isArrayOf(node['marks'], isAdfMark)) return undefined + if ('text' in node && typeof node['text'] !== 'string') return undefined + if (!('content' in node)) return [] + const content = node['content'] + return Array.isArray(content) ? [...content] : undefined + }) } function marksKey(marks: readonly AdfMark[]): string { diff --git a/src/json-value.ts b/src/json-value.ts index 6448b4d..f444ee1 100644 --- a/src/json-value.ts +++ b/src/json-value.ts @@ -2,23 +2,36 @@ import { largestNesting } from './nesting.ts' export type JsonValue = JsonValue[] | boolean | null | number | string | { [key: string]: JsonValue } -export function isJsonValue(value: unknown): value is JsonValue { - const pending: unknown[] = [value] +// Tracks ancestors alone: a tree may share an object, but one holding itself would walk forever. +export function everyInTree(roots: readonly unknown[], children: (item: unknown) => readonly unknown[] | undefined): boolean { + const pending = Array.from(roots, (item) => ({ depth: 0, item })) + const ancestors: unknown[] = [] + const onPath = new Set() while (pending.length > 0) { - const item = pending.pop() - if (item === null || typeof item === 'boolean' || typeof item === 'string') continue - if (typeof item === 'number') { - if (!Number.isFinite(item)) return false - continue - } - // A hole is not a JSON value, and Array.prototype methods skip holes — spreading materialises them. - if (Array.isArray(item)) for (const child of [...item]) pending.push(child) - else if (typeof item === 'object') for (const child of Object.values(item)) pending.push(child) - else return false + const entry = pending.pop() + if (entry === undefined) continue + while (ancestors.length > entry.depth) onPath.delete(ancestors.pop()) + if (onPath.has(entry.item)) return false + const held = children(entry.item) + if (held === undefined) return false + if (held.length === 0) continue + ancestors.push(entry.item) + onPath.add(entry.item) + for (const child of held) pending.push({ depth: entry.depth + 1, item: child }) } return true } +export function isJsonValue(value: unknown): value is JsonValue { + return everyInTree([value], (item) => { + if (item === null || typeof item === 'boolean' || typeof item === 'string') return [] + if (typeof item === 'number') return Number.isFinite(item) ? [] : undefined + // A hole is not a JSON value, and Array.prototype methods skip holes — spreading materialises them. + if (Array.isArray(item)) return [...item] + return typeof item === 'object' ? Object.values(item) : undefined + }) +} + export function nestingDepth(value: unknown): number { const pending: { depth: number; item: unknown }[] = [{ depth: 0, item: value }] let deepest = 0 diff --git a/todo.md b/todo.md index 54b9978..a367e4f 100644 --- a/todo.md +++ b/todo.md @@ -24,7 +24,6 @@ | ID | Release | Exempt | Item | R | S | A | G | Goals | Score | |---|---|---|---|---|---|---|---|---|---| -| 63 | 0.2.0 | defect | **Refuse a cyclic input as `not-an-adf-document`.** | 2 | 2 | 6 | 9 | 1 | 27.5 | | 7 | 0.2.0 | | **Ship HTML: `adfToHtml`, `htmlToAdf`, and `markdownToHtml` / `htmlToMarkdown` composed through ADF.** | 6 | 9 | 9 | 9 | 2, 3 | 25.8 | | 55 | 0.2.0 | defect | **Spell every document the emitter refuses today for a carriage return, a NUL, a code-span line start, a newline inside an emoji, mention or status, or a text node whose text is empty or missing, or which holds content.** | 5 | 5 | 7 | 9 | 1 | 25.8 | | 45 | 0.2.0 | | **Replace `isAdfDocument` with a reader returning `Result`.** | 2 | 3 | 6 | 8 | 1 | 25.2 | @@ -57,13 +56,6 @@ ## Details -### 63. Refuse a cyclic input as `not-an-adf-document`. - -`adfDocumentFault` walks with `isNodeArray` (`adf/document.ts`) and `isJsonValue` (`json-value.ts`), -worklists that record no visited object, so a node whose `content` holds itself hangs -`adfToMarkdown`, `adfToPlainMarkdown` and `isAdfDocument`; README §The guarantees promises no input -loops forever. - ### 7. Ship HTML: `adfToHtml`, `htmlToAdf`, and `markdownToHtml` / `htmlToMarkdown` composed through ADF. Lands after items 6, 59, 60, 61 and 62. The CommonMark spec suite also runs against