From 84ec40f1af8ea7d74b757d7b50b391aec8482f8a Mon Sep 17 00:00:00 2001 From: xingzhou Date: Tue, 13 Oct 2026 21:23:33 +0800 Subject: [PATCH] fix(browser): distinguish SVG links from HTML XLink attributes (#2480) * fix(browser): respect SVG desc and title HTML integration points * fix(browser): preserve moved SVG XLink destinations * fix(browser): bound concurrent SVG identity reads --- .../CdpCaptureDocuments.baseUrl.test.ts | 284 ++++++++++++++-- src/browser/CdpCaptureDocuments.ts | 160 +++++++-- src/browser/CdpCaptureDomSvg.ts | 74 ++-- src/browser/CdpPageCapture.ts | 54 ++- .../browser/cdpDomSvgIdentity.test.ts | 315 ++++++++++++++++++ .../boundary/browser/cdpSvgXlinkHref.test.ts | 57 +++- 6 files changed, 821 insertions(+), 123 deletions(-) create mode 100644 tests/boundary/browser/cdpDomSvgIdentity.test.ts diff --git a/src/browser/CdpCaptureDocuments.baseUrl.test.ts b/src/browser/CdpCaptureDocuments.baseUrl.test.ts index a5481c161..e3201eade 100644 --- a/src/browser/CdpCaptureDocuments.baseUrl.test.ts +++ b/src/browser/CdpCaptureDocuments.baseUrl.test.ts @@ -2,6 +2,8 @@ import { describe, expect, it } from "vitest"; import { inspectWebPageInputSchema } from "../domain/browserObservation.js"; import { captureDom } from "./CdpCaptureDocuments.js"; +import { capturedSvgNodeKind } from "./CdpCaptureDomSvg.js"; +import { CdpCaptureCompleteness } from "./CdpCaptureCompleteness.js"; const origin = "https://app.example.test"; const documentUrl = `${origin}/screens/app`; @@ -50,8 +52,11 @@ const snapshot = ( ], }); -const capture = (value: ReturnType) => - captureDom(value, new Set([origin]), input); +const capture = (value: ReturnType) => { + const step = captureDom(value, new Set([origin]), input).next(); + if (!step.done) throw new Error("Unexpected SVG identity query"); + return step.value; +}; describe("DOM metadata document base URLs", () => { it("resolves links, actions and agent hints against the captured document base", () => { @@ -282,6 +287,7 @@ interface SvgSnapshotNode { readonly name: string; readonly parent: number; readonly attributes?: readonly (readonly [string, string])[]; + readonly svg?: boolean; } const svgDocument = ( @@ -300,7 +306,13 @@ const svgDocument = ( const nodeValue = [intern("")]; const parentIndex = [-1]; const attributes: number[][] = [[]]; + const backendNodeId = [1]; + const svgNodeKinds = new Map(); for (const element of elements) { + const id = backendNodeId.length + 1; + backendNodeId.push(id); + if (element.svg !== undefined) + svgNodeKinds.set(`${String(id)}:${element.name}`, element.svg); nodeType.push(1); nodeName.push(intern(element.name)); nodeValue.push(intern("")); @@ -316,14 +328,35 @@ const svgDocument = ( { documentURL: 0, baseURL: 1, - nodes: { nodeType, nodeName, nodeValue, parentIndex, attributes }, + nodes: { + nodeType, + nodeName, + nodeValue, + parentIndex, + attributes, + backendNodeId, + }, + svgNodeKinds, }, ], }; }; -const captureSvg = (value: ReturnType) => - captureDom(value, new Set([origin]), input); +const captureSvg = (value: ReturnType) => { + const kinds = new Map( + value.documents.flatMap((document) => [...document.svgNodeKinds]), + ); + const capture = captureDom(value, new Set([origin]), input); + let step = capture.next(); + while (!step.done) { + step = capture.next( + step.value.map((query) => + kinds.get(`${String(query.backendNodeId)}:${query.nodeName}`), + ), + ); + } + return step.value; +}; const urlFor = ( result: ReturnType, @@ -331,6 +364,136 @@ const urlFor = ( ): (typeof result.urls)[number] | undefined => result.urls.find((url) => url.node_index === nodeIndex); +describe("browser SVG identity", () => { + const query = { backendNodeId: 42, nodeName: "a" }; + const node = { ...query, nodeType: 1 }; + + it("uses only a matching described element's SVG flag", () => { + expect(capturedSvgNodeKind(query, { node: { ...node, isSVG: true } })).toBe( + true, + ); + expect( + capturedSvgNodeKind(query, { node: { ...node, isSVG: false } }), + ).toBe(false); + expect(capturedSvgNodeKind(query, { node })).toBe(false); + for (const invalid of [ + {}, + { node: { ...node, backendNodeId: 43, isSVG: true } }, + { node: { ...node, nodeName: "A", isSVG: true } }, + { node: { ...node, nodeType: 3, isSVG: true } }, + { node: { ...node, isSVG: "true" } }, + ]) + expect(capturedSvgNodeKind(query, invalid)).toBeUndefined(); + }); + + it("distinguishes identically named SVG and MathML elements regardless of parent", () => { + const result = captureSvg( + svgDocument([ + { name: "svg", parent: 0 }, + { name: "desc", parent: 1 }, + { + name: "a", + svg: true, + parent: 2, + attributes: [["xlink:href", "/svg"]], + }, + { + name: "a", + svg: false, + parent: 2, + attributes: [["xlink:href", "/math"]], + }, + { + name: "A", + svg: false, + parent: 1, + attributes: [["xlink:href", "/html"]], + }, + { + name: "a", + svg: true, + parent: 0, + attributes: [["xlink:href", "/orphan"]], + }, + ]), + ); + expect(result.urls.map(({ url }) => url)).toEqual([ + `${origin}/svg`, + `${origin}/orphan`, + ]); + }); + + it("records unresolved identity without inventing a destination or hiding attributes", () => { + const value = svgDocument([ + { name: "a", parent: 0, attributes: [["xlink:href", "/unknown"]] }, + ]); + const completeness = new CdpCaptureCompleteness(); + const capture = captureDom(value, new Set([origin]), input, completeness); + expect(capture.next()).toEqual({ + done: false, + value: [{ backendNodeId: 2, nodeName: "a" }], + }); + const step = capture.next([undefined]); + expect(step.done).toBe(true); + if (!step.done) throw new Error("Unexpected second query"); + expect(step.value.unresolvedSvgNodes).toBe(1); + expect(step.value.urls).toEqual([]); + expect(step.value.nodes[1]?.attribute_names).toEqual(["xlink:href"]); + expect(completeness.snapshot().excluded).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + section: "metadata", + reason: "provider_unavailable", + count: 1, + }), + ]), + ); + }); +}); + +it("keeps batched SVG identities aligned around missing IDs and ordinary links", () => { + const value = svgDocument([ + { name: "a", parent: 0, attributes: [["xlink:href", "/svg"]] }, + { name: "A", parent: 0, attributes: [["href", "/ordinary"]] }, + { name: "a", parent: 0, attributes: [["xlink:href", "/missing-id"]] }, + { name: "a", parent: 0, attributes: [["xlink:href", "/non-svg"]] }, + ]); + const completeness = new CdpCaptureCompleteness(); + const capture = captureDom( + { + ...value, + documents: value.documents.map((document) => ({ + ...document, + nodes: { ...document.nodes, backendNodeId: [1, 2, 3, null, 5] }, + })), + }, + new Set([origin]), + input, + completeness, + ); + expect(capture.next()).toEqual({ + done: false, + value: [ + { backendNodeId: 2, nodeName: "a" }, + { backendNodeId: 5, nodeName: "a" }, + ], + }); + const step = capture.next([true, false]); + if (!step.done) throw new Error("Unexpected second batch"); + expect( + step.value.urls.map(({ node_index, url }) => [node_index, url]), + ).toEqual([ + [1, `${origin}/svg`], + [2, `${origin}/ordinary`], + ]); + expect(step.value.unresolvedSvgNodes).toBe(1); + expect(completeness.snapshot().excluded).toContainEqual({ + section: "metadata", + reason: "provider_unavailable", + count: 1, + }); +}); + describe("SVG xlink:href destinations", () => { it("reports an SVG anchor that only has xlink:href, keeping the first value", () => { const result = captureSvg( @@ -339,6 +502,7 @@ describe("SVG xlink:href destinations", () => { { name: "g", parent: 1 }, { name: "a", + svg: true, parent: 2, attributes: [ ["xlink:href", "/xl-s2h"], @@ -371,6 +535,7 @@ describe("SVG xlink:href destinations", () => { { name: "svg", parent: 0 }, { name: "a", + svg: true, parent: 1, attributes: [ [first, valueFor(first)], @@ -395,18 +560,21 @@ describe("SVG xlink:href destinations", () => { svgDocument([ { name: "A", + svg: false, parent: 0, attributes: [["xlink:href", "/xl-html"]], }, { name: "math", parent: 0 }, { name: "mrow", + svg: false, parent: 2, attributes: [["xlink:href", "/xl-math"]], }, { name: "svg", parent: 0 }, { name: "a", + svg: true, parent: 4, attributes: [["xlink:href", "/xl-s2h"]], }, @@ -425,6 +593,7 @@ describe("SVG xlink:href destinations", () => { { name: "svg", parent: 0 }, { name: "a", + svg: true, parent: 1, attributes: [["xlink:href", "https://cdn.example.test/out"]], }, @@ -444,8 +613,18 @@ describe("SVG xlink:href destinations", () => { const result = captureSvg( svgDocument([ { name: "svg", parent: 0 }, - { name: "image", parent: 1, attributes: [["xlink:href", "guide"]] }, - { name: "use", parent: 1, attributes: [["xlink:href", "icon.svg"]] }, + { + name: "image", + svg: true, + parent: 1, + attributes: [["xlink:href", "guide"]], + }, + { + name: "use", + svg: true, + parent: 1, + attributes: [["xlink:href", "icon.svg"]], + }, ]), ); expect(result.urls).toEqual([ @@ -466,14 +645,24 @@ describe("SVG xlink:href destinations", () => { }); describe("SVG xlink:href context", () => { - it("keeps forward ancestor context local to each document", () => { + it("binds SVG identity to the selected element across documents", () => { const svg = svgDocument([ - { name: "a", parent: 2, attributes: [["xlink:href", "/svg-link"]] }, + { + name: "a", + svg: true, + parent: 2, + attributes: [["xlink:href", "/svg-link"]], + }, { name: "g", parent: 3 }, { name: "svg", parent: 0 }, ]); const html = svgDocument([ - { name: "A", parent: 2, attributes: [["xlink:href", "/html-link"]] }, + { + name: "A", + svg: false, + parent: 2, + attributes: [["xlink:href", "/html-link"]], + }, { name: "DIV", parent: 3 }, { name: "BODY", parent: 0 }, ]); @@ -507,32 +696,39 @@ describe("SVG xlink:href context", () => { ]); }); - it("stops SVG context at foreignObject and resumes it for a nested svg", () => { - const result = captureSvg( - svgDocument([ - { name: "svg", parent: 0 }, - { name: "foreignObject", parent: 1 }, - { - name: "A", - parent: 2, - attributes: [["xlink:href", "/xl-html-fo"]], - }, - { name: "svg", parent: 2 }, - { - name: "a", - parent: 4, - attributes: [["xlink:href", "/xl-nested"]], - }, - ]), - ); - expect(urlFor(result, 3)).toBeUndefined(); - expect(urlFor(result, 5)).toMatchObject({ - attribute: "href", - url: `${origin}/xl-nested`, - destination_scope: "approved", - }); - }); + it.each(["foreignObject", "desc", "title"])( + "preserves browser SVG identity across %s HTML integration points", + (integrationPoint) => { + const result = captureSvg( + svgDocument([ + { name: "svg", parent: 0 }, + { name: integrationPoint, parent: 1 }, + { + name: "A", + svg: false, + parent: 2, + attributes: [["xlink:href", "/xl-html-fo"]], + }, + { name: "svg", parent: 2 }, + { + name: "a", + svg: true, + parent: 4, + attributes: [["xlink:href", "/xl-nested"]], + }, + ]), + ); + expect(urlFor(result, 3)).toBeUndefined(); + expect(urlFor(result, 5)).toMatchObject({ + attribute: "href", + url: `${origin}/xl-nested`, + destination_scope: "approved", + }); + }, + ); +}); +describe("SVG XLink precedence and unknown identity", () => { it("keeps an empty href ahead of xlink:href", () => { const baseUrl = `${origin}/screens/assets/`; const result = captureSvg( @@ -541,6 +737,7 @@ describe("SVG xlink:href context", () => { { name: "svg", parent: 0 }, { name: "a", + svg: true, parent: 1, attributes: [ ["href", ""], @@ -567,6 +764,7 @@ describe("SVG xlink:href context", () => { { name: "svg", parent: 0 }, { name: "a", + svg: true, parent: 1, attributes: [["xlink:href", "javascript:void(0)"]], }, @@ -582,7 +780,7 @@ describe("SVG xlink:href context", () => { ]); }); - it("does not treat a cyclic or dangling parent chain as SVG", () => { + it("keeps an unavailable SVG identity unknown despite ancestor names", () => { const cyclic = captureSvg( svgDocument([ { name: "g", parent: 2 }, @@ -652,7 +850,12 @@ describe("XML SVG roots and namespace scope restoration", () => { ["xmlns:p", XLINK_NAMESPACE], ], }, - { name: "a", parent: 1, attributes: [["p:href", "/linked"]] }, + { + name: "a", + svg: true, + parent: 1, + attributes: [["p:href", "/linked"]], + }, ]), ); expect(result.urls).toEqual([approvedHref(2, `${origin}/linked`)]); @@ -764,6 +967,7 @@ describe("XHTML and XML SVG namespace prefixes", () => { { name: "svg:foreignObject", parent: 2 }, { name: "a", + svg: true, parent: 3, attributes: [["xlink:href", "/xl-html-fo"]], }, @@ -799,6 +1003,7 @@ describe("XHTML and XML SVG namespace prefixes", () => { }, { name: "a", + svg: true, parent: 2, attributes: [ ["xlink:href", "/not-xlink"], @@ -918,11 +1123,13 @@ describe("XHTML SVG exclusions, HTML documents, and prefix scope", () => { }, { name: "a", + svg: true, parent: 2, attributes: [["xlink:href", "/xl-literal"]], }, { name: "a", + svg: true, parent: 2, attributes: [["x:href", "/xl-custom-html"]], }, @@ -937,6 +1144,7 @@ describe("XHTML SVG exclusions, HTML documents, and prefix scope", () => { }, { name: "A", + svg: false, parent: 1, attributes: [["xlink:href", "/xl-html-anchor"]], }, @@ -944,7 +1152,9 @@ describe("XHTML SVG exclusions, HTML documents, and prefix scope", () => { ); expect(result.urls).toEqual([approvedHref(3, `${origin}/xl-literal`)]); }); +}); +describe("XML namespace resolution ordering and capacity", () => { it("applies an element's own xmlns declarations to its name and href", () => { const result = captureSvg( svgDocument([ diff --git a/src/browser/CdpCaptureDocuments.ts b/src/browser/CdpCaptureDocuments.ts index 3c7fb98ed..f7eb4fe31 100644 --- a/src/browser/CdpCaptureDocuments.ts +++ b/src/browser/CdpCaptureDocuments.ts @@ -11,7 +11,11 @@ import { } from "./CdpCaptureValues.js"; import { exclusionReasonForUrl } from "./CdpCaptureEventHelpers.js"; import type { CdpCaptureCompleteness } from "./CdpCaptureCompleteness.js"; -import { createDocumentSvgAncestry, domSvgLink } from "./CdpCaptureDomSvg.js"; +import { + createDocumentSvgAncestry, + domSvgLink, + type DomSvgNodeQuery, +} from "./CdpCaptureDomSvg.js"; import { compareUnicodeCodePoints } from "../domain/unicodeCodePointOrder.js"; export type CapturedResource = Omit< @@ -135,18 +139,39 @@ export const walkFrameTrees = function* ( } }; -export const captureDom = ( +interface DomMetadataCapture { + readonly urls: WebPageInspection["metadata"]["dom_urls"]; + readonly agentHints: WebPageInspection["metadata"]["agent_hints"]; + readonly excludedUrls: number; + readonly unresolvedSvgNodes: number; +} + +export interface DomCapture extends DomMetadataCapture { + readonly total: number; + readonly nodes: WebPageInspection["dom"]["nodes"]; +} + +// Bound retained projections and outstanding CDP reads independently of page size. +// Eight leaves room for the session's other cancellation listeners. +const DOM_METADATA_BATCH_SIZE = 8; + +interface PendingDomMetadata { + readonly options: DomMetadataOptions; + // Undefined means no query is needed; null means the candidate has no valid ID. + readonly svgQuery: DomSvgNodeQuery | null | undefined; +} + +/** Project one snapshot, requesting bounded batches of SVG identity facts. */ +export function* captureDom( result: unknown, allowedOrigins: ReadonlySet, input: InspectWebPageInput, completeness?: CdpCaptureCompleteness, -): { - readonly total: number; - readonly nodes: WebPageInspection["dom"]["nodes"]; - readonly urls: WebPageInspection["metadata"]["dom_urls"]; - readonly agentHints: WebPageInspection["metadata"]["agent_hints"]; - readonly excludedUrls: number; -} => { +): Generator< + readonly DomSvgNodeQuery[], + DomCapture, + readonly (boolean | undefined)[] +> { const root = requiredRecord(result); const strings = Array.isArray(root.strings) ? root.strings.map((value) => cdpStringValue(value) ?? "") @@ -156,6 +181,7 @@ export const captureDom = ( const agentHints: WebPageInspection["metadata"]["agent_hints"] = []; let excludedUrls = 0; let total = 0; + let unresolvedSvgNodes = 0; for (const document of recordsValue(root.documents)) { const documentUrl = indexedString(strings, document.documentURL); if (allowedSanitizedUrl(documentUrl, allowedOrigins) === undefined) { @@ -172,6 +198,9 @@ export const captureDom = ( const attributes = Array.isArray(documentNodes.attributes) ? documentNodes.attributes : []; + const backendNodeIds = Array.isArray(documentNodes.backendNodeId) + ? documentNodes.backendNodeId + : []; const baseIndex = nodes.length; const ancestry = createDocumentSvgAncestry({ nodeTypes, @@ -181,12 +210,29 @@ export const captureDom = ( attributes, }); total += nodeTypes.length; + const pendingMetadata: PendingDomMetadata[] = []; for (let index = 0; index < nodeTypes.length; index += 1) { const attributeIndexes = numberArray(attributes[index]); const parent = Math.trunc(parents[index] ?? -1); const nodeIndex = nodes.length; const nodeName = indexedString(strings, nodeNames[index]); const attributeNames = uniqueAttributeNames(attributeIndexes, strings); + const svgLink = domSvgLink(ancestry, index); + let svgQuery: DomSvgNodeQuery | null | undefined; + if ( + !svgLink.xmlDocument && + nodeTypes[index] === 1 && + attributeNames.includes("xlink:href") && + !attributeNames.includes("href") + ) { + const backendNodeId = numberValue(backendNodeIds[index]); + svgQuery = + backendNodeId !== undefined && + Number.isSafeInteger(backendNodeId) && + backendNodeId > 0 + ? { backendNodeId, nodeName } + : null; + } nodes.push({ index: nodeIndex, parent_index: parent < 0 ? -1 : baseIndex + parent, @@ -195,30 +241,34 @@ export const captureDom = ( node_value_length: indexedString(strings, nodeValues[index]).length, attribute_names: attributeNames, }); - const metadata = domMetadata({ - strings, - attributes: attributeIndexes, - // The BASE element itself resolves against the document fallback URL. - baseUrl: nodeName.toLowerCase() === "base" ? documentUrl : baseUrl, - documentUrl, - nodeIndex, - nodeName, - allowedOrigins, - ...domSvgLink(ancestry, index), + pendingMetadata.push({ + svgQuery, + options: { + strings, + attributes: attributeIndexes, + // The BASE element itself resolves against the document fallback URL. + baseUrl: nodeName.toLowerCase() === "base" ? documentUrl : baseUrl, + documentUrl, + nodeIndex, + nodeName, + allowedOrigins, + ...svgLink, + }, }); - for (const url of metadata.urls) { - urls.push(url); - if (url.destination_scope !== "approved") { - excludedUrls += 1; - completeness?.exclude( - "metadata", - url.destination_scope === "outside_policy" - ? "disallowed_origin" - : "unsupported_url", - ); - } + if ( + pendingMetadata.length === DOM_METADATA_BATCH_SIZE || + index === nodeTypes.length - 1 + ) { + const metadata = yield* captureDomMetadata( + pendingMetadata, + completeness, + ); + urls.push(...metadata.urls); + agentHints.push(...metadata.agentHints); + excludedUrls += metadata.excludedUrls; + unresolvedSvgNodes += metadata.unresolvedSvgNodes; + pendingMetadata.length = 0; } - agentHints.push(...metadata.agentHints); } } return { @@ -227,8 +277,54 @@ export const captureDom = ( urls, agentHints, excludedUrls, + unresolvedSvgNodes, }; -}; +} + +function* captureDomMetadata( + pending: readonly PendingDomMetadata[], + completeness: CdpCaptureCompleteness | undefined, +): Generator< + readonly DomSvgNodeQuery[], + DomMetadataCapture, + readonly (boolean | undefined)[] +> { + const queries = pending.flatMap(({ svgQuery }) => + svgQuery == null ? [] : [svgQuery], + ); + const kinds = queries.length > 0 ? yield queries : []; + const urls: DomMetadataCapture["urls"] = []; + const agentHints: DomMetadataCapture["agentHints"] = []; + let queryIndex = 0; + let unresolvedSvgNodes = 0; + let excludedUrls = 0; + for (const { options, svgQuery } of pending) { + let svgElement = options.svgElement; + if (svgQuery !== undefined) { + const kind = svgQuery === null ? undefined : kinds[queryIndex++]; + svgElement = kind === true; + if (kind === undefined) { + unresolvedSvgNodes += 1; + completeness?.exclude("metadata", "provider_unavailable"); + } + } + const metadata = domMetadata({ ...options, svgElement }); + for (const url of metadata.urls) { + urls.push(url); + if (url.destination_scope !== "approved") { + excludedUrls += 1; + completeness?.exclude( + "metadata", + url.destination_scope === "outside_policy" + ? "disallowed_origin" + : "unsupported_url", + ); + } + } + agentHints.push(...metadata.agentHints); + } + return { urls, agentHints, unresolvedSvgNodes, excludedUrls }; +} export const captureAccessibility = ( results: readonly unknown[], diff --git a/src/browser/CdpCaptureDomSvg.ts b/src/browser/CdpCaptureDomSvg.ts index 3d1a5c977..4d021766c 100644 --- a/src/browser/CdpCaptureDomSvg.ts +++ b/src/browser/CdpCaptureDomSvg.ts @@ -1,4 +1,4 @@ -import { numberValue } from "./CdpCaptureValues.js"; +import { numberValue, recordValue } from "./CdpCaptureValues.js"; const SVG_NAMESPACE = "http://www.w3.org/2000/svg"; const XLINK_NAMESPACE = "http://www.w3.org/1999/xlink"; @@ -18,11 +18,34 @@ interface XmlSvgLink { interface DocumentSvgAncestry { readonly xml: boolean; - readonly nodes: DomSnapshotNodes; - readonly svgContexts: Map; readonly xmlLinks: Map; } +export interface DomSvgNodeQuery { + readonly backendNodeId: number; + readonly nodeName: string; +} + +/** Bind the browser's SVG flag to the exact snapshotted element. */ +export const capturedSvgNodeKind = ( + query: DomSvgNodeQuery, + result: unknown, +): boolean | undefined => { + const node = recordValue(recordValue(result)?.node); + if ( + node?.backendNodeId !== query.backendNodeId || + node.nodeName !== query.nodeName || + node.nodeType !== 1 + ) + return undefined; + // CDP omits isSVG for non-SVG nodes. Other values are malformed, not false. + return node.isSVG === undefined + ? false + : typeof node.isSVG === "boolean" + ? node.isSVG + : undefined; +}; + interface DomSvgLink { readonly svgElement: boolean; readonly xmlDocument: boolean; @@ -79,8 +102,6 @@ export const createDocumentSvgAncestry = ( const xml = xmlDocument(nodes); return { xml, - nodes, - svgContexts: new Map(), xmlLinks: xml ? resolveXmlElements(nodes) : new Map(), }; }; @@ -91,11 +112,7 @@ export const domSvgLink = ( ): DomSvgLink => { if (!ancestry.xml) { return { - svgElement: elementInSvgContext( - index, - ancestry.nodes, - ancestry.svgContexts, - ), + svgElement: false, xmlDocument: false, xlinkHref: undefined, }; @@ -108,43 +125,6 @@ export const domSvgLink = ( }; }; -// Chrome's HTML parser records SVG local names (`svg`, `foreignObject`). -// Descendants of foreignObject are HTML until a nested `svg`. -const elementInSvgContext = ( - index: number, - nodes: DomSnapshotNodes, - contexts: Map, -): boolean => { - let current = index; - const seen = new Set(); - let inSvg = false; - while ( - current >= 0 && - current < nodes.nodeNames.length && - !seen.has(current) - ) { - const cached = contexts.get(current); - if (cached !== undefined) { - inSvg = cached; - break; - } - seen.add(current); - if (stringAt(nodes.strings, nodes.nodeNames[current]) === "svg") { - inSvg = true; - break; - } - const parent = Math.trunc(nodes.parents[current] ?? -1); - if (parent < 0 || parent === current) break; - if (stringAt(nodes.strings, nodes.nodeNames[parent]) === "foreignObject") - break; - current = parent; - } - // Cache the entire examined chain, including forward references and cycles, - // so shared ancestors are examined once per document. - for (const node of seen) contexts.set(node, inSvg); - return inSvg; -}; - interface NamespaceRestore { readonly prefix: string; readonly previous: string | undefined; diff --git a/src/browser/CdpPageCapture.ts b/src/browser/CdpPageCapture.ts index 0d97576b5..f49d4e186 100644 --- a/src/browser/CdpPageCapture.ts +++ b/src/browser/CdpPageCapture.ts @@ -17,8 +17,10 @@ import { captureFrames, captureResources, type CapturedResource, + type DomCapture, mainFrameUrl, } from "./CdpCaptureDocuments.js"; +import { capturedSvgNodeKind } from "./CdpCaptureDomSvg.js"; import { allowedSanitizedUrl, delayWithCancellation, @@ -52,7 +54,7 @@ interface CaptureState { interface CapturedSections { readonly attachedUrl: string; readonly frames: WebPageInspection["frames"]; - readonly dom: ReturnType; + readonly dom: DomCapture; readonly accessibility: ReturnType; readonly scripts: WebPageInspection["scripts"]; readonly resources: WebPageInspection["resources"]; @@ -260,9 +262,7 @@ const authorizeObservationWindow = async ( throw new BrowserObservationError("inspect_web_page", "target_not_allowed"); }; -const capturePageDom = async ( - state: CaptureState, -): Promise> => { +const capturePageDom = async (state: CaptureState): Promise => { const { context, allowedOrigins } = state; const result = await context.connection.send( "DOMSnapshot.captureSnapshot", @@ -274,12 +274,56 @@ const capturePageDom = async ( context.sessionId, context.signal, ); - return captureDom( + const capture = captureDom( result, allowedOrigins, context.input, state.events.completeness, ); + let step = capture.next(); + let descriptionUnavailable = false; + const pendingController = new AbortController(); + const signal = + context.signal === undefined + ? pendingController.signal + : AbortSignal.any([context.signal, pendingController.signal]); + while (!step.done) { + const pending = step.value.map(async (query) => { + if (descriptionUnavailable) return undefined; + try { + return capturedSvgNodeKind( + query, + await context.connection.send( + "DOM.describeNode", + { backendNodeId: query.backendNodeId, depth: 0 }, + context.sessionId, + signal, + ), + ); + } catch (cause: unknown) { + // A detached node can disappear between snapshot and description. + // Preserve partial metadata, but propagate transport/cancellation errors. + if (!(cause instanceof CdpCommandRejection)) throw cause; + if (cause.code === -32_601) descriptionUnavailable = true; + else if (cause.code !== -32_000) throw cause; + return undefined; + } + }); + try { + // Promise.all preserves snapshot order even when CDP replies out of order. + step = capture.next(await Promise.all(pending)); + } catch (cause: unknown) { + // Stop and settle this bounded batch before the session can be detached. + pendingController.abort(); + await Promise.allSettled(pending); + throw cause; + } + } + if (step.value.unresolvedSvgNodes > 0) + state.limitations.push( + `SVG identity was unavailable for ${String(step.value.unresolvedSvgNodes)} snapshotted XLink candidate nodes; their URL destinations remain unknown.`, + ); + return step.value; }; const normalizedInspection = ( diff --git a/tests/boundary/browser/cdpDomSvgIdentity.test.ts b/tests/boundary/browser/cdpDomSvgIdentity.test.ts new file mode 100644 index 000000000..7d42ae336 --- /dev/null +++ b/tests/boundary/browser/cdpDomSvgIdentity.test.ts @@ -0,0 +1,315 @@ +import { describe, expect, it } from "vitest"; + +import { CdpBrowserProvider } from "../../../src/browser/CdpBrowserProvider.js"; +import { inspectWebPageInputSchema } from "../../../src/domain/browserObservation.js"; +import { + startFakeCdpBrowser, + type FakeCdpBrowser, + type FakeOptions, +} from "../../fixtures/fakeCdpBrowser.js"; +import { trackBrowser } from "./cdpBrowserProvider.support.js"; + +const startBrowser = async (options: FakeOptions = {}) => { + const browser = await startFakeCdpBrowser({ + ...options, + commandResult(command, origin, frameReads) { + const override = options.commandResult?.(command, origin, frameReads); + if (override !== undefined) return override; + if (command.method === "DOMSnapshot.captureSnapshot") { + const nodes = { + nodeType: [9, 1, 1, 1, 1], + nodeName: [1, 2, 3, 3, 3], + nodeValue: [4, 4, 4, 4, 4], + parentIndex: [-1, 0, 1, 1, 1], + backendNodeId: [1, 2, 3, 4, 5], + attributes: [[], [], [5, 6], [5, 7], [5, 6, 8, 9]], + }; + return { + strings: [ + `${origin}/app`, + "#document", + "HTML", + "a", + "", + "xlink:href", + "/first", + "/second", + "href", + "/ordinary", + "https://excluded.example/page", + ], + documents: [ + { documentURL: 0, nodes }, + { + documentURL: 10, + nodes: { ...nodes, backendNodeId: [11, 12, 13, 14, 15] }, + }, + ], + }; + } + if (command.method === "DOM.describeNode") + return { + node: { + backendNodeId: command.params.backendNodeId, + nodeType: 1, + nodeName: "a", + isSVG: true, + }, + }; + return undefined; + }, + }); + trackBrowser(browser); + return browser; +}; + +const inspect = (browser: FakeCdpBrowser, signal?: AbortSignal) => + new CdpBrowserProvider().inspectPage( + inspectWebPageInputSchema.parse({ + cdp_endpoint: browser.endpoint, + target_id: "allowed-page", + allowed_origins: [browser.allowedOrigin], + observation_ms: 0, + }), + signal === undefined ? undefined : { signal }, + ); + +describe("CDP SVG identity at the snapshot boundary", () => { + it("describes only in-scope XLink candidates without a winning href", async () => { + const browser = await startBrowser(); + const result = await inspect(browser); + if (!result.ok) throw result.error; + expect(result.value.metadata.dom_urls.map(({ url }) => url)).toEqual( + ["/first", "/second", "/ordinary"].map( + (path) => browser.allowedOrigin + path, + ), + ); + expect( + browser.commands + .filter(({ method }) => method === "DOM.describeNode") + .map(({ params }) => params), + ).toEqual([ + { backendNodeId: 3, depth: 0 }, + { backendNodeId: 4, depth: 0 }, + ]); + }); + + it.each(["removed", "mismatched"])( + "retains partial metadata when a candidate is %s", + async (failure) => { + const browser = await startBrowser({ + commandError: (command) => + failure === "removed" && + command.method === "DOM.describeNode" && + command.params.backendNodeId === 3 + ? { code: -32_000, message: "No node found for given backend id" } + : undefined, + commandResult: (command) => + failure === "mismatched" && + command.method === "DOM.describeNode" && + command.params.backendNodeId === 3 + ? { + node: { + backendNodeId: 99, + nodeType: 1, + nodeName: "a", + isSVG: true, + }, + } + : undefined, + }); + const result = await inspect(browser); + if (!result.ok) throw result.error; + expect(result.value.metadata.dom_urls.map(({ url }) => url)).toEqual( + ["/second", "/ordinary"].map((path) => browser.allowedOrigin + path), + ); + expect(result.value.completeness.excluded).toContainEqual({ + section: "metadata", + reason: "provider_unavailable", + count: 1, + }); + expect(result.value.limitations).toContain( + "SVG identity was unavailable for 1 snapshotted XLink candidate nodes; their URL destinations remain unknown.", + ); + }, + ); + + it("stops querying after the first batch reports an unsupported method", async () => { + const browser = await startBrowser({ + commandResult: manyCandidates, + unsupportedMethods: ["DOM.describeNode"], + }); + const result = await inspect(browser); + if (!result.ok) throw result.error; + expect(result.value.metadata.dom_urls).toEqual([]); + expect(result.value.completeness.excluded).toContainEqual({ + section: "metadata", + reason: "provider_unavailable", + count: 25, + }); + expect(descriptions(browser).length).toBeGreaterThan(0); + expect(descriptions(browser).length).toBeLessThanOrEqual(8); + }); + + it.each([ + { options: { closeOnMethod: "DOM.describeNode" }, reason: "disconnected" }, + { + options: { + commandError: (command: { method: string }) => + command.method === "DOM.describeNode" + ? { code: -32_602, message: "Invalid parameters" } + : undefined, + }, + reason: "protocol_error", + }, + ])( + "propagates $reason during node description", + async ({ options, reason }) => { + const browser = await startBrowser(options); + expect(await inspect(browser)).toMatchObject({ + ok: false, + error: { _tag: "BrowserObservationError", reason }, + }); + }, + ); + + it("cancels a pending description and detaches without closing the target", async () => { + const browser = await startBrowser({ hangOnMethod: "DOM.describeNode" }); + const controller = new AbortController(); + const pending = inspect(browser, controller.signal); + await expect.poll(() => descriptions(browser).length).toBeGreaterThan(1); + controller.abort(); + expect(await pending).toMatchObject({ + ok: false, + error: { _tag: "AnalysisCancelledError", operation: "inspect_web_page" }, + }); + const methods = browser.commands.map(({ method }) => method); + expect(methods).toContain("Target.detachFromTarget"); + expect(methods).not.toContain("Target.closeTarget"); + expect(methods).not.toContain("Browser.close"); + }); +}); + +const manyCandidates: NonNullable = ( + command, + origin, +) => { + if (command.method !== "DOMSnapshot.captureSnapshot") return undefined; + const count = 25; + return { + strings: [ + origin, + "#document", + "HTML", + "a", + "", + "xlink:href", + ...Array.from({ length: count }, (_, index) => `/link-${String(index)}`), + ], + documents: [ + { + documentURL: 0, + nodes: { + nodeType: [9, 1, ...Array(count).fill(1)], + nodeName: [1, 2, ...Array(count).fill(3)], + nodeValue: Array(count + 2).fill(4), + parentIndex: [-1, 0, ...Array(count).fill(1)], + backendNodeId: Array.from( + { length: count + 2 }, + (_, index) => index + 1, + ), + attributes: [ + [], + [], + ...Array.from({ length: count }, (_, index) => [5, index + 6]), + ], + }, + }, + ], + }; +}; + +const descriptions = (browser: FakeCdpBrowser) => + browser.commands.filter(({ method }) => method === "DOM.describeNode"); + +describe("CDP SVG query concurrency", () => { + it("bounds in-flight reads and preserves metadata order across reversed replies", async () => { + const browser = await startBrowser({ + commandResult: manyCandidates, + hangOnMethod: "DOM.describeNode", + }); + const controller = new AbortController(); + const pending = inspect(browser, controller.signal); + try { + await expect.poll(() => descriptions(browser).length).toBeGreaterThan(1); + let replied = 0; + while (replied < 25) { + await expect + .poll(() => descriptions(browser).length) + .toBeGreaterThan(replied); + const batch = descriptions(browser).slice(replied); + expect(batch.length).toBeLessThanOrEqual(8); + for (const command of [...batch].reverse()) { + const id = Number(command.params.backendNodeId); + browser.emitRawMessage( + JSON.stringify({ + id: command.id, + result: { + node: { + backendNodeId: id, + nodeType: 1, + nodeName: "a", + isSVG: id % 2 === 1, + }, + }, + }), + ); + } + replied += batch.length; + } + const result = await pending; + if (!result.ok) throw result.error; + expect(result.value.metadata.dom_urls.map(({ url }) => url)).toEqual( + Array.from( + { length: 13 }, + (_, index) => `${browser.allowedOrigin}/link-${String(index * 2)}`, + ), + ); + expect(descriptions(browser)).toHaveLength(25); + } finally { + controller.abort(); + await pending; + } + }); + + it("cancels sibling reads on a fatal reply without waiting for hung nodes", async () => { + const browser = await startBrowser({ + commandResult: manyCandidates, + hangOnMethod: "DOM.describeNode", + }); + const controller = new AbortController(); + const pending = inspect(browser, controller.signal); + try { + await expect.poll(() => descriptions(browser).length).toBeGreaterThan(1); + const batch = descriptions(browser); + browser.emitRawMessage( + JSON.stringify({ + id: batch.at(-1)?.id, + error: { code: -32_602, message: "Invalid parameters" }, + }), + ); + await expect(pending).resolves.toMatchObject({ + ok: false, + error: { _tag: "BrowserObservationError", reason: "protocol_error" }, + }); + expect(descriptions(browser)).toHaveLength(batch.length); + const methods = browser.commands.map(({ method }) => method); + expect(methods).toContain("Target.detachFromTarget"); + expect(methods).not.toContain("Target.closeTarget"); + expect(methods).not.toContain("Browser.close"); + } finally { + controller.abort(); + await pending; + } + }); +}); diff --git a/tests/boundary/browser/cdpSvgXlinkHref.test.ts b/tests/boundary/browser/cdpSvgXlinkHref.test.ts index 56c3fc3cf..38048861b 100644 --- a/tests/boundary/browser/cdpSvgXlinkHref.test.ts +++ b/tests/boundary/browser/cdpSvgXlinkHref.test.ts @@ -45,8 +45,38 @@ const pageHtml = `x out + move desc + move title + + not an SVG link + HTML link + nested + + + <a xlink:href="/xl-html-title">not an SVG link</a> + <a href="/html-title">HTML link</a> + <svg><a xlink:href="/xl-nested-title"><text>nested</text></a></svg> + html + `; it.skipIf(chromeExecutable === undefined)( @@ -143,17 +173,36 @@ it.skipIf(chromeExecutable === undefined)( url: null, destination_scope: "outside_policy", }), + ...[ + "/html-desc", + "/html-title", + "/xl-nested-desc", + "/xl-nested-title", + "/xl-moved-desc", + "/xl-moved-title", + "/xl-created-desc", + "/xl-created-title", + "/xl-created-nested-desc", + "/xl-created-nested-title", + ].map((path) => + expect.objectContaining({ + attribute: "href", + url: `${origin}${path}`, + destination_scope: "approved", + }), + ), ]), ); expect( urls.some( (url) => - url.url?.endsWith("/xl-html") === true || + url.url?.includes("/xl-html") === true || + url.url?.includes("/xl-math") === true || url.url?.endsWith("/xlink-loses") === true, ), ).toBe(false); expect(urls.every((url) => url.attribute === "href")).toBe(true); - expect(urls).toHaveLength(5); + expect(urls).toHaveLength(15); } finally { child.kill("SIGTERM"); if (!(await waitForExit(child, 2_000))) child.kill("SIGKILL"); @@ -173,6 +222,8 @@ const xhtmlPage = ` p + desc + title no no @@ -202,6 +253,8 @@ for (const { format, page, contentType, nodeNames, link, destinations } of [ destinations: [ "/xl-pa1", "/xl-image", + "/xl-xml-desc", + "/xl-xml-title", "/xl-fo-svg", "/xl-orphan", "/xl-custom",