Skip to content

Commit a9823f7

Browse files
fix(lint): a double-quoted media src with an apostrophe is read whole, not reported missing (#5411)
* fix(lint): a double-quoted media src with an apostrophe is read whole, not reported missing * fix(lint): every media src read goes through the attribute parser, decoded * refactor(lint): the src pattern only finds the tag; the attribute parser reads the value * fix(lint): a media src whose name starts with a quote character is checked too * fix(lint): the missing-audio check skips audio tags that scripts build as strings * docs(lint): mediaSrcTagRe's comment states the src rule without the history
1 parent faa4913 commit a9823f7

5 files changed

Lines changed: 108 additions & 24 deletions

File tree

‎packages/lint/src/hevcPreviewLint.test.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,16 @@ describe("collectLocalVideoCandidates", () => {
4040
expect([...candidates.keys()]).toEqual([join(dir, "design/styleframes/clip.mp4")]);
4141
});
4242

43+
it("finds a double-quoted src whose name contains an apostrophe", () => {
44+
const dir = makeProject(["assets/Ann's clip.mp4"]);
45+
46+
const candidates = collectLocalVideoCandidates(dir, [
47+
{ html: `<video src="assets/Ann's clip.mp4"></video>` },
48+
]);
49+
50+
expect([...candidates.keys()]).toEqual([join(dir, "assets/Ann's clip.mp4")]);
51+
});
52+
4353
it("still resolves project-root refs that have no sibling", () => {
4454
const dir = makeProject(["assets/clip.mp4"]);
4555

‎packages/lint/src/hevcPreviewLint.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import {
1313
} from "@hyperframes/parsers/asset-resolution";
1414
import { parseHTML } from "linkedom";
1515
import type { HyperframeLintFinding } from "./types.js";
16-
import { mediaSrcTagRe } from "./utils";
16+
import { mediaSrcOf, mediaSrcTagRe } from "./utils";
1717

1818
/** Structurally compatible with `project.ts`'s (unexported) `HtmlSource` —
1919
* duplicated as a shape, not imported, to avoid a circular import between
@@ -126,7 +126,7 @@ export function collectLocalVideoCandidates(
126126
const re = new RegExp(videoSrcRe.source, videoSrcRe.flags);
127127
let match: RegExpExecArray | null;
128128
while ((match = re.exec(scannable)) !== null) {
129-
const rawSrc = match[2] ?? "";
129+
const rawSrc = mediaSrcOf(match);
130130
const reference = resolveLocalVideoReference(projectDir, rawSrc, compSrcPath);
131131
if (!reference || candidates.has(reference.resolved)) continue;
132132
candidates.set(reference.resolved, reference.src);

‎packages/lint/src/project.test.ts‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -781,6 +781,86 @@ describe("audio_src_not_found with templating tokens", () => {
781781
});
782782
});
783783

784+
describe("a double-quoted src that contains an apostrophe", () => {
785+
const name = "Narrator's voice take 1.mp3";
786+
787+
it("finds the existing audio file instead of cutting the src at the apostrophe", async () => {
788+
const project = makeProject(`<html><body>
789+
<div data-composition-id="main" data-width="1920" data-height="1080" data-start="0" data-duration="10"></div>
790+
<audio id="a1" class="clip" data-start="0" data-duration="3" data-track-index="10" src="assets/${name}"></audio>
791+
</body></html>`);
792+
mkdirSync(join(project, "assets"), { recursive: true });
793+
writeFileSync(join(project, "assets", name), "");
794+
const { results } = await lintProject(project);
795+
const codes = results.flatMap((entry) => entry.result.findings).map((f) => f.code);
796+
expect(codes).not.toContain("audio_src_not_found");
797+
});
798+
799+
async function lintCodes(body: string, files: string[]): Promise<string[]> {
800+
const project = makeProject(`<html><body>
801+
<div data-composition-id="main" data-width="1920" data-height="1080" data-start="0" data-duration="10"></div>
802+
${body}
803+
</body></html>`);
804+
mkdirSync(join(project, "assets"), { recursive: true });
805+
for (const file of files) writeFileSync(join(project, "assets", file), "");
806+
const { results } = await lintProject(project);
807+
return results.flatMap((entry) => entry.result.findings).map((f) => f.code);
808+
}
809+
810+
it("finds an existing audio file whose apostrophe is written as a character reference", async () => {
811+
const codes = await lintCodes(
812+
`<audio id="a1" class="clip" data-start="0" data-duration="3" data-track-index="10" src="assets/Narrator&#39;s take.mp3"></audio>`,
813+
["Narrator's take.mp3"],
814+
);
815+
expect(codes).not.toContain("audio_src_not_found");
816+
});
817+
818+
it("tells two layered files apart when their names share text up to an apostrophe", async () => {
819+
const audio = (id: string, file: string) =>
820+
`<audio id="${id}" class="clip" data-start="0" data-duration="5" data-track-index="10" src="assets/${file}"></audio>`;
821+
const codes = await lintCodes(
822+
audio("a1", "Narrator's take 1.mp3") + audio("a2", "Narrator's take 2.mp3"),
823+
["Narrator's take 1.mp3", "Narrator's take 2.mp3"],
824+
);
825+
expect(codes).toContain("duplicate_audio_track");
826+
});
827+
828+
it("still reports a missing file whose name starts with a quote character", async () => {
829+
const codes = await lintCodes(
830+
`<audio id="a1" class="clip" data-start="0" data-duration="3" data-track-index="10" src="'90s theme.mp3"></audio>`,
831+
[],
832+
);
833+
expect(codes).toContain("audio_src_not_found");
834+
});
835+
836+
it("does not read an audio tag that an inline script builds as a string", async () => {
837+
const codes = await lintCodes(
838+
`<script>var s = '<audio src="' + u + '">'; var t = "<audio src='" + u + "'>";</script>`,
839+
[],
840+
);
841+
expect(codes).not.toContain("audio_src_not_found");
842+
});
843+
844+
it("does not read a src out of another attribute's text", async () => {
845+
const codes = await lintCodes(`<img id="i1" alt="x src='ghost.png'" />`, []);
846+
expect(codes).not.toContain("missing_local_asset");
847+
});
848+
849+
it("finds the existing image through the shared src pattern too", async () => {
850+
const image = "Ann's photo.png";
851+
const project = makeProject(`<html><body>
852+
<div data-composition-id="main" data-width="1920" data-height="1080" data-start="0" data-duration="10">
853+
<img id="i1" class="clip" data-start="0" data-duration="3" data-track-index="0" src="assets/${image}">
854+
</div>
855+
</body></html>`);
856+
mkdirSync(join(project, "assets"), { recursive: true });
857+
writeFileSync(join(project, "assets", image), "");
858+
const { results } = await lintProject(project);
859+
const codes = results.flatMap((entry) => entry.result.findings).map((f) => f.code);
860+
expect(codes).not.toContain("missing_local_asset");
861+
});
862+
});
863+
784864
describe("templating tokens are checked on the raw src, before cleanAssetUrl", () => {
785865
// cleanAssetUrl splits on ?/#, which also chops inside a ${...} expression
786866
// (e.g. `${asset?.url}` -> `${asset`). The token skip must run on the RAW value or

‎packages/lint/src/project.ts‎

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ import type {
2929
HyperframeLinterOptions,
3030
} from "./types.js";
3131
import type { ParsableDocumentLike } from "@hyperframes/parsers/sub-composition-validity";
32-
import { isAudibleVideoTag, mediaSrcTagRe } from "./utils";
32+
import { isAudibleVideoTag, mediaSrcOf, mediaSrcTagRe, readAttr, readDecodedAttr } from "./utils";
3333

3434
/** Adapts linkedom's `parseHTML` to the `checkSubCompositionUsability` contract. */
3535
function parseSubCompHtml(html: string): ParsableDocumentLike {
@@ -370,9 +370,10 @@ function lintAudioSrcNotFound(
370370

371371
const missingByFile = new Map<string, Set<string>>();
372372
for (const { html, compSrcPath } of htmlSources) {
373+
const scannable = maskNonScannableRanges(html);
373374
let match: RegExpExecArray | null;
374-
while ((match = audioSrcRe.exec(html)) !== null) {
375-
const src = match[2]!;
375+
while ((match = audioSrcRe.exec(scannable)) !== null) {
376+
const src = mediaSrcOf(match);
376377
if (/^(https?:|data:|blob:)/i.test(src)) continue;
377378
if (isUnresolvedAssetPlaceholder(src)) continue;
378379
const rootRelative = compSrcPath
@@ -422,7 +423,7 @@ function lintMissingLocalAsset(
422423
let match: RegExpExecArray | null;
423424
while ((match = re.exec(scannable)) !== null) {
424425
const tagName = (match[1] ?? "").toLowerCase();
425-
const rawSrc = match[2] ?? "";
426+
const rawSrc = mediaSrcOf(match);
426427
// Placeholder check runs on the RAW value: cleanAssetUrl() splits on ?/# and would chop inside a ${...} token.
427428
if (isUnresolvedAssetPlaceholder(rawSrc)) continue;
428429
const src = cleanAssetUrl(rawSrc);
@@ -544,12 +545,6 @@ function lintMultipleRootCompositions(projectDir: string): HyperframeLintFinding
544545

545546
function lintDuplicateAudioTracks(htmlSources: HtmlSource[]): HyperframeLintFinding[] {
546547
const findings: HyperframeLintFinding[] = [];
547-
function extractAttr(tag: string, name: string): string | null {
548-
const re = new RegExp(`\\b${name}\\s*=\\s*["']([^"']+)["']`, "i");
549-
const m = tag.match(re);
550-
return m?.[1] ?? null;
551-
}
552-
553548
const tracks: Array<{ trackIndex: number; start: number; end: number; src: string }> = [];
554549
const seen = new Set<string>();
555550

@@ -559,10 +554,10 @@ function lintDuplicateAudioTracks(htmlSources: HtmlSource[]): HyperframeLintFind
559554
while ((match = soundTagRe.exec(html)) !== null) {
560555
const tag = match[0];
561556
if (/^<video/i.test(tag) && !isAudibleVideoTag(tag)) continue;
562-
const trackStr = extractAttr(tag, "data-track-index");
563-
const startStr = extractAttr(tag, "data-start");
564-
const durStr = extractAttr(tag, "data-duration");
565-
const src = extractAttr(tag, "src") ?? "unknown";
557+
const trackStr = readAttr(tag, "data-track-index");
558+
const startStr = readAttr(tag, "data-start");
559+
const durStr = readAttr(tag, "data-duration");
560+
const src = readDecodedAttr(tag, "src") || "unknown";
566561
if (!trackStr || !startStr) continue;
567562

568563
const trackIndex = parseInt(trackStr, 10);

‎packages/lint/src/utils.ts‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -858,14 +858,13 @@ export function truncateSnippet(value: string, maxLength = 220): string | undefi
858858

859859
/**
860860
* Matches a media tag carrying a real `src` attribute, capturing the tag name in
861-
* group 1 and the src value in group 2.
862-
*
863-
* The leading whitespace before `src` is load-bearing: `\bsrc\s*=` also matches
864-
* the tail of `data-var-src="bg"` (a hyphen/`s` boundary is a word boundary), and
865-
* since `[^>]*` is greedy it wins over a real `src` earlier in the same tag. Every
866-
* element using a variable binding was therefore reported as referencing a missing
867-
* file named after the variable id.
861+
* group 1 and the quoted src in group 2; {@link mediaSrcOf} reads its value.
862+
* `src` must follow whitespace so the tail of `data-var-src="bg"` is not read as one.
868863
*/
869864
export function mediaSrcTagRe(tagAlternation: string): RegExp {
870-
return new RegExp(`<(${tagAlternation})\\b[^>]*\\ssrc\\s*=\\s*["']([^"']+)["'][^>]*>`, "gi");
865+
return new RegExp(`<(${tagAlternation})\\b[^>]*\\ssrc\\s*=\\s*("[^"]*"|'[^']*')[^>]*>`, "gi");
866+
}
867+
868+
export function mediaSrcOf(match: RegExpExecArray): string {
869+
return readDecodedAttr(match[0], "src") ?? "";
871870
}

0 commit comments

Comments
 (0)