From 0f1f73eb351353c4363641b7ddd5b3fa50c30b9a Mon Sep 17 00:00:00 2001 From: Eemeli Aro Date: Mon, 18 Apr 2022 13:08:18 +0300 Subject: [PATCH 1/2] =?UTF-8?q?feat:=20Allow=20for=20pathlike=20aliases,?= =?UTF-8?q?=20=C3=A0=20la=20*foo/42/bar?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/doc/Document.ts | 7 +++-- src/nodes/Alias.ts | 71 +++++++++++++++++++++++++++++++-------------- src/nodes/toJS.ts | 19 ++++++------ tests/next.ts | 55 +++++++++++++++++++++++++++++++++++ 4 files changed, 120 insertions(+), 32 deletions(-) create mode 100644 tests/next.ts diff --git a/src/doc/Document.ts b/src/doc/Document.ts index 1ad738a9..9b33c662 100644 --- a/src/doc/Document.ts +++ b/src/doc/Document.ts @@ -424,11 +424,14 @@ export class Document { mapAsMap: mapAsMap === true, mapKeyWarned: false, maxAliasCount: typeof maxAliasCount === 'number' ? maxAliasCount : 100, + resolved: new WeakMap(), stringify } const res = toJS(this.contents, jsonArg ?? '', ctx) - if (typeof onAnchor === 'function') - for (const { count, res } of ctx.anchors.values()) onAnchor(res, count) + if (typeof onAnchor === 'function') { + for (const [node, { count }] of ctx.anchors) + onAnchor(ctx.resolved.get(node), count) + } return typeof reviver === 'function' ? applyReviver(reviver, { '': res }, '', res) : res diff --git a/src/nodes/Alias.ts b/src/nodes/Alias.ts index 4c242cf3..b1679838 100644 --- a/src/nodes/Alias.ts +++ b/src/nodes/Alias.ts @@ -8,6 +8,7 @@ import { isAlias, isCollection, isPair, + isScalar, Node, NodeBase, Range @@ -24,6 +25,8 @@ export declare namespace Alias { } } +const RESOLVE = Symbol('_resolve') + export class Alias extends NodeBase { source: string @@ -39,46 +42,72 @@ export class Alias extends NodeBase { }) } - /** - * Resolve the value of this alias within `doc`, finding the last - * instance of the `source` anchor before this node. - */ - resolve(doc: Document): Scalar | YAMLMap | YAMLSeq | undefined { + [RESOLVE](doc: Document) { let found: Scalar | YAMLMap | YAMLSeq | undefined = undefined + // @ts-expect-error - TS doesn't notice the assignment in the visitor + let root: Node & { anchor: string } = undefined + const pathLike = this.source.includes('/') visit(doc, { Node: (_key: unknown, node: Node) => { if (node === this) return visit.BREAK - if (node.anchor === this.source) found = node + const { anchor } = node + if (anchor === this.source) { + found = node + } else if ( + doc.directives?.yaml.version === 'next' && + anchor && + pathLike && + this.source.startsWith(anchor + '/') && + (!root || root.anchor.length <= anchor.length) + ) { + root = node as Node & { anchor: string } + } } }) - return found + if (found) return { node: found, root: found } + + if (isCollection(root)) { + const parts = this.source.substring(root.anchor.length + 1).split('/') + const node = root.getIn(parts, true) + if (isCollection(node) || isScalar(node)) return { node, root } + } + + return { node: undefined, root } + } + + /** + * Resolve the value of this alias within `doc`, finding the last + * instance of the `source` anchor before this node. + */ + resolve(doc: Document): Scalar | YAMLMap | YAMLSeq | undefined { + return this[RESOLVE](doc).node } toJSON(_arg?: unknown, ctx?: ToJSContext) { if (!ctx) return { source: this.source } - const { anchors, doc, maxAliasCount } = ctx - const source = this.resolve(doc) - if (!source) { + const { anchors, doc, maxAliasCount, resolved } = ctx + const { node, root } = this[RESOLVE](doc) + if (!node) { const msg = `Unresolved alias (the anchor must be set before the alias): ${this.source}` throw new ReferenceError(msg) } - const data = anchors.get(source) - /* istanbul ignore if */ - if (!data || data.res === undefined) { - const msg = 'This should not happen: Alias anchor was not resolved?' - throw new ReferenceError(msg) - } + if (maxAliasCount >= 0) { + const data = anchors.get(root) + if (!data) { + const msg = 'This should not happen: Alias anchor was not resolved?' + throw new ReferenceError(msg) + } data.count += 1 - if (data.aliasCount === 0) - data.aliasCount = getAliasCount(doc, source, anchors) + data.aliasCount ||= getAliasCount(doc, root, anchors) if (data.count * data.aliasCount > maxAliasCount) { const msg = 'Excessive alias count indicates a resource exhaustion attack' throw new ReferenceError(msg) } } - return data.res + + return resolved.get(node) } toString( @@ -105,8 +134,8 @@ function getAliasCount( anchors: ToJSContext['anchors'] ): number { if (isAlias(node)) { - const source = node.resolve(doc) - const anchor = anchors && source && anchors.get(source) + const { root } = node[RESOLVE](doc) + const anchor = root && anchors?.get(root) return anchor ? anchor.count * anchor.aliasCount : 0 } else if (isCollection(node)) { let count = 0 diff --git a/src/nodes/toJS.ts b/src/nodes/toJS.ts index 26cd005c..c3a06277 100644 --- a/src/nodes/toJS.ts +++ b/src/nodes/toJS.ts @@ -5,7 +5,6 @@ import { hasAnchor, Node } from './Node.js' export interface AnchorData { aliasCount: number count: number - res: unknown } export interface ToJSContext { @@ -16,6 +15,7 @@ export interface ToJSContext { mapKeyWarned: boolean maxAliasCount: number onCreate?: (res: unknown) => void + resolved: WeakMap /** Requiring this directly in Pair would create circular dependencies */ stringify: typeof stringify @@ -35,16 +35,17 @@ export function toJS(value: any, arg: string | null, ctx?: ToJSContext): any { // eslint-disable-next-line @typescript-eslint/no-unsafe-return if (Array.isArray(value)) return value.map((v, i) => toJS(v, String(i), ctx)) if (value && typeof value.toJSON === 'function') { - // eslint-disable-next-line @typescript-eslint/no-unsafe-call - if (!ctx || !hasAnchor(value)) return value.toJSON(arg, ctx) - const data: AnchorData = { aliasCount: 0, count: 1, res: undefined } - ctx.anchors.set(value, data) - ctx.onCreate = res => { - data.res = res - delete ctx.onCreate + if (ctx) { + if (hasAnchor(value)) ctx.anchors.set(value, { aliasCount: 0, count: 1 }) + ctx.onCreate = res => { + ctx.onCreate = undefined + ctx.resolved.set(value, res) + } } + + // eslint-disable-next-line @typescript-eslint/no-unsafe-call const res = value.toJSON(arg, ctx) - if (ctx.onCreate) ctx.onCreate(res) + if (ctx?.onCreate) ctx.onCreate(res) return res } if (typeof value === 'bigint' && !ctx?.keep) return Number(value) diff --git a/tests/next.ts b/tests/next.ts new file mode 100644 index 00000000..1ee46d9c --- /dev/null +++ b/tests/next.ts @@ -0,0 +1,55 @@ +import { parse, parseDocument } from 'yaml' +import { source } from './_utils' + +describe('relative-path alias', () => { + test('resolves a map value by key', () => { + const src = source` + - &a { foo: 1 } + - *a/foo + ` + expect(parse(src, { version: 'next' })).toEqual([{ foo: 1 }, 1]) + }) + + test('resolves a sequence value by index', () => { + const src = source` + - &a [ 2, 4, 8 ] + - *a/1 + ` + expect(parse(src, { version: 'next' })).toEqual([[2, 4, 8], 4]) + }) + + test('resolves a deeper value', () => { + const src = source` + - &a { foo: [1, 42] } + - *a/foo/1 + ` + expect(parse(src, { version: 'next' })).toEqual([{ foo: [1, 42] }, 42]) + }) + + test('resolves to an equal value', () => { + const src = source` + - &a { foo: [42] } + - *a/foo + ` + const res = parse(src, { version: 'next' }) + expect(res[1]).toBe(res[0].foo) + }) + + test('does not resolve an alias value', () => { + const src = source` + - &a { foo: *a } + - *a/foo + ` + const doc = parseDocument(src, { version: 'next' }) + expect(() => doc.toJS()).toThrow(ReferenceError) + }) + + test('does not resolve a later value', () => { + const src = source` + - *a/foo + - &a { foo: 1 } + ` + const doc = parseDocument(src, { version: 'next' }) + expect(() => doc.toJS()).toThrow(ReferenceError) + }) +}) From ca1ce09e2643de4146085c88eb955056e0fb2444 Mon Sep 17 00:00:00 2001 From: Eemeli Aro Date: Wed, 20 Apr 2022 09:53:48 +0300 Subject: [PATCH 2/2] feat: Require unique anchors during composition --- src/compose/compose-doc.ts | 1 + src/compose/compose-node.ts | 23 +++++++++++++++++++++-- src/errors.ts | 1 + tests/next.ts | 21 +++++++++++++++++++++ 4 files changed, 44 insertions(+), 2 deletions(-) diff --git a/src/compose/compose-doc.ts b/src/compose/compose-doc.ts index ab58c138..fb9e4755 100644 --- a/src/compose/compose-doc.ts +++ b/src/compose/compose-doc.ts @@ -24,6 +24,7 @@ export function composeDoc( const opts = Object.assign({ _directives: directives }, options) const doc = new Document(undefined, opts) as Document.Parsed const ctx: ComposeContext = { + anchors: options.version === 'next' ? new Map() : null, atRoot: true, directives: doc.directives, options: doc.options, diff --git a/src/compose/compose-node.ts b/src/compose/compose-node.ts index 2aea4383..449bcf66 100644 --- a/src/compose/compose-node.ts +++ b/src/compose/compose-node.ts @@ -11,6 +11,7 @@ import { resolveEnd } from './resolve-end.js' import { emptyScalarPosition } from './util-empty-scalar-position.js' export interface ComposeContext { + anchors: Map | null atRoot: boolean directives: Directives options: Readonly>> @@ -52,13 +53,13 @@ export function composeNode( case 'double-quoted-scalar': case 'block-scalar': node = composeScalar(ctx, token, tag, onError) - if (anchor) node.anchor = anchor.source.substring(1) + if (anchor) setAnchor(ctx, node, anchor, onError) break case 'block-map': case 'block-seq': case 'flow-collection': node = composeCollection(CN, ctx, token, tag, onError) - if (anchor) node.anchor = anchor.source.substring(1) + if (anchor) setAnchor(ctx, node, anchor, onError) break default: { const message = @@ -138,3 +139,21 @@ function composeAlias( if (re.comment) alias.comment = re.comment return alias as Alias.Parsed } + +function setAnchor( + { anchors }: ComposeContext, + node: ParsedNode, + anchor: SourceToken, + onError: ComposeErrorHandler +) { + const name = anchor.source.substring(1) + if (anchors) { + if (anchors.has(name)) { + const msg = `Anchors must be unique, ${name} is repeated` + onError(node.range, 'DUPLICATE_ANCHOR', msg) + } else { + anchors.set(name, node) + } + } + node.anchor = name +} diff --git a/src/errors.ts b/src/errors.ts index a1db8a9c..84551e1b 100644 --- a/src/errors.ts +++ b/src/errors.ts @@ -10,6 +10,7 @@ export type ErrorCode = | 'BAD_SCALAR_START' | 'BLOCK_AS_IMPLICIT_KEY' | 'BLOCK_IN_FLOW' + | 'DUPLICATE_ANCHOR' | 'DUPLICATE_KEY' | 'IMPOSSIBLE' | 'KEY_OVER_1024_CHARS' diff --git a/tests/next.ts b/tests/next.ts index 1ee46d9c..f4e4bf7e 100644 --- a/tests/next.ts +++ b/tests/next.ts @@ -53,3 +53,24 @@ describe('relative-path alias', () => { expect(() => doc.toJS()).toThrow(ReferenceError) }) }) + +describe('unique anchors', () => { + test('repeats are fine without flag', () => { + const src = source` + - &a 1 + - &a 2 + - *a + ` + expect(parse(src)).toEqual([1, 2, 2]) + }) + + test("repeats are an error with 'next'", () => { + const src = source` + - &a 1 + - &a 2 + - *a + ` + const doc = parseDocument(src, { version: 'next' }) + expect(doc.errors).toMatchObject([{ code: 'DUPLICATE_ANCHOR' }]) + }) +})