Skip to content

Commit 22ff0dd

Browse files
committed
fix(core,studio): escape user values in querySelector attribute selectors
Extract cssAttrSelector to packages/core/src/utils/cssSelector.ts and use it (or CSS.escape for browser-side code) at all 12 sites that previously interpolated raw user-authored values into querySelector attribute selectors. A " in a composition ID, script src, or data-start value would produce a malformed selector that throws. Node-side (core compiler/parser): uses the shared cssAttrSelector. Browser-side (runtime, studio): uses native CSS.escape(). Supersedes #1568 which fixed only the 3 bundler sites.
1 parent fdb8f33 commit 22ff0dd

11 files changed

Lines changed: 37 additions & 17 deletions

File tree

packages/core/src/compiler/htmlBundler.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import { validateHyperframeHtmlContract } from "./staticGuard";
1818
import { getHyperframeRuntimeScript } from "../generated/runtime-inline";
1919
import { readDeclaredDefaults } from "../runtime/getVariables";
2020
import { inlineSubCompositions } from "./inlineSubCompositions";
21+
import { queryByAttr } from "../utils/cssSelector";
2122
import { isSafePath, resolveWithinProject } from "../safePath.js";
2223
import { HF_COLOR_GRADING_ATTR } from "../colorGrading";
2324

@@ -277,7 +278,7 @@ function rewriteCssUrlsWithInlinedAssets(cssText: string, projectDir: string): s
277278
}
278279

279280
function cssAttributeSelector(attr: string, value: string): string {
280-
return `[${attr}="${value.replace(/\\/g, "\\\\").replace(/"/g, '\\"')}"]`;
281+
return `[${attr}="${value}"]`;
281282
}
282283

283284
function uniqueCompositionId(baseId: string, index: number): string {
@@ -624,7 +625,7 @@ export interface BundleOptions {
624625
*/
625626

626627
function ensureExternalScriptTag(doc: Document, src: string): void {
627-
if (doc.querySelector(`script[src="${src}"]`)) return;
628+
if (queryByAttr(doc, "src", src, "script")) return;
628629
const el = doc.createElement("script");
629630
el.setAttribute("src", src);
630631
doc.body.appendChild(el);
@@ -825,7 +826,7 @@ export async function bundleToSingleHtml(
825826
continue;
826827
}
827828
}
828-
if (!document.querySelector(`script[src="${extSrc}"]`)) {
829+
if (!queryByAttr(document, "src", extSrc, "script")) {
829830
const extScript = document.createElement("script");
830831
extScript.setAttribute("src", extSrc);
831832
document.body.appendChild(extScript);
@@ -857,7 +858,7 @@ export async function bundleToSingleHtml(
857858
const hostIdentity = hostIdentityByElement.get(host);
858859
const runtimeCompId = hostIdentity?.runtimeCompositionId || compId;
859860
const innerDoc = parseHTMLContent(templateHtml);
860-
const innerRoot = innerDoc.querySelector(`[data-composition-id="${compId}"]`);
861+
const innerRoot = queryByAttr(innerDoc, "data-composition-id", compId);
861862
const authoredRootId = innerRoot?.getAttribute("id")?.trim() || null;
862863
const runtimeScope = runtimeCompId
863864
? cssAttributeSelector("data-composition-id", runtimeCompId)

packages/core/src/compiler/inlineSubCompositions.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
rewriteCssAssetUrls,
1414
rewriteInlineStyleAssetUrls,
1515
} from "./rewriteSubCompPaths";
16+
import { queryByAttr } from "../utils/cssSelector";
1617
import {
1718
scopeCssToComposition,
1819
wrapInlineScriptWithErrorBoundary,
@@ -225,7 +226,7 @@ export function inlineSubCompositions(
225226

226227
// Find the inner composition root
227228
const innerRoot = compId
228-
? contentDoc.querySelector(`[data-composition-id="${compId}"]`)
229+
? queryByAttr(contentDoc, "data-composition-id", compId)
229230
: contentDoc.querySelector("[data-composition-id]");
230231
const inferredCompId = innerRoot?.getAttribute("data-composition-id")?.trim() || "";
231232
const authoredRootId = innerRoot?.getAttribute("id")?.trim() || null;

packages/core/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@ export {
130130
rewriteCssAssetUrls,
131131
} from "./compiler/rewriteSubCompPaths";
132132
export { CSS_URL_RE, isNonRelativeUrl, isPathInside } from "./compiler/assetPaths";
133+
export { queryByAttr } from "./utils/cssSelector";
133134
export { decodeUrlPathVariants } from "./utils/urlPath";
134135
export { parseAnimatedGifMetadata, type AnimatedGifMetadata } from "./media/gif";
135136
export {

packages/core/src/parsers/htmlParser.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import type {
1313
import { validateCompositionGsap } from "./gsapSerialize";
1414
import { ensureHfIds } from "./hfIds.js";
1515
import { parseGsapScriptAcornForWrite } from "./gsapParserAcorn.js";
16+
import { queryByAttr } from "../utils/cssSelector";
1617
import { removeAnimationFromScript } from "./gsapWriterAcorn.js";
1718
import type { ValidationResult } from "../core.types";
1819

@@ -519,7 +520,7 @@ export function updateElementInHtml(
519520
const parser = new DOMParser();
520521
const doc = parser.parseFromString(html, "text/html");
521522

522-
const el = doc.getElementById(elementId) || doc.querySelector(`[data-name="${elementId}"]`);
523+
const el = doc.getElementById(elementId) || queryByAttr(doc, "data-name", elementId);
523524
if (!el) return html;
524525

525526
if (updates.startTime !== undefined) {

packages/core/src/runtime/picker.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,11 +97,11 @@ export function createPickerModule(deps: PickerModuleDeps): PickerModule {
9797
const htmlEl = el as HTMLElement;
9898
if (htmlEl.id) return `#${htmlEl.id}`;
9999
const compositionId = el.getAttribute("data-composition-id");
100-
if (compositionId) return `[data-composition-id="${compositionId}"]`;
100+
if (compositionId) return `[data-composition-id="${CSS.escape(compositionId)}"]`;
101101
const compositionSrc = el.getAttribute("data-composition-src");
102-
if (compositionSrc) return `[data-composition-src="${compositionSrc}"]`;
102+
if (compositionSrc) return `[data-composition-src="${CSS.escape(compositionSrc)}"]`;
103103
const track = el.getAttribute("data-track-index");
104-
if (track) return `[data-track-index="${track}"]`;
104+
if (track) return `[data-track-index="${CSS.escape(track)}"]`;
105105
const tag = el.tagName.toLowerCase();
106106
const parent = el.parentElement;
107107
if (!parent) return tag;
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// ponytail: queries DOM by exact attribute match without interpolating
2+
// the value into a selector string — zero injection surface.
3+
export function queryByAttr(
4+
root: ParentNode,
5+
attr: string,
6+
value: string,
7+
tag?: string,
8+
): Element | null {
9+
const selector = tag ? `${tag}[${attr}]` : `[${attr}]`;
10+
for (const el of root.querySelectorAll(selector)) {
11+
if (el.getAttribute(attr) === value) return el;
12+
}
13+
return null;
14+
}

packages/studio/src/components/editor/LayersPanel.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ export const LayersPanel = memo(function LayersPanel() {
126126
if (doc) {
127127
const found =
128128
(layer.id ? doc.getElementById(layer.id) : null) ??
129-
(layer.hfId ? doc.querySelector(`[data-hf-id="${layer.hfId}"]`) : null) ??
129+
(layer.hfId ? doc.querySelector(`[data-hf-id="${CSS.escape(layer.hfId)}"]`) : null) ??
130130
doc.getElementById(layer.key);
131131
if (found instanceof HTMLElement) el = found;
132132
}

packages/studio/src/components/editor/domEditingElement.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ export function findElementForSelection(
242242
activeCompositionPath: string | null = null,
243243
): HTMLElement | null {
244244
if (selection.hfId) {
245-
const byHfId = doc.querySelector(`[data-hf-id="${selection.hfId}"]`);
245+
const byHfId = doc.querySelector(`[data-hf-id="${CSS.escape(selection.hfId)}"]`);
246246
if (isHtmlElement(byHfId)) return byHfId;
247247
}
248248

packages/studio/src/player/lib/timelineDOM.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,8 @@ export function createTimelineElementFromManifestClip(params: {
123123
if (clip.kind === "composition" && clip.compositionId) {
124124
let resolvedSrc = clip.compositionSrc;
125125
if (!resolvedSrc) {
126-
hostEl = doc?.querySelector(`[data-composition-id="${clip.compositionId}"]`) ?? hostEl;
126+
hostEl =
127+
doc?.querySelector(`[data-composition-id="${CSS.escape(clip.compositionId)}"]`) ?? hostEl;
127128
resolvedSrc =
128129
hostEl?.getAttribute("data-composition-src") ??
129130
hostEl?.getAttribute("data-composition-file") ??

packages/studio/src/player/lib/timelineElementHelpers.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -283,8 +283,8 @@ function nodeMatchesManifestClip(node: Element, clip: ClipManifestClip): boolean
283283
function findTimelineDomNode(doc: Document, id: string): Element | null {
284284
return (
285285
doc.getElementById(id) ??
286-
doc.querySelector(`[data-composition-id="${id}"]`) ??
287-
doc.querySelector(`.${id}`) ??
286+
doc.querySelector(`[data-composition-id="${CSS.escape(id)}"]`) ??
287+
doc.querySelector(`.${CSS.escape(id)}`) ??
288288
null
289289
);
290290
}

0 commit comments

Comments
 (0)