Skip to content

fix(studio): stop file names with %, apostrophes or $ from breaking Studio - #5156

Merged
miguel-heygen merged 29 commits into
mainfrom
fix/studio-serves-files-with-a-bare-percent
Oct 8, 2026
Merged

miguel-heygen merged 29 commits into
mainfrom
fix/studio-serves-files-with-a-bare-percent

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

What

Special characters in file names keep working when Studio serves, edits or renames a file.

  • Files such as 100%.png are served by Studio and the CLI's static project server without a decoding exception.
  • Editing src="assets/it's.mp4" replaces the existing attribute. src never matches inside data-src, x-src, xml:src or srcset; replacement values containing $ remain literal.
  • Renaming preserves raw JS/JSON file values and raw data-composition-src paths. HTML URL fields, including srcset and SVG xlink:href, and CSS URLs receive escaping appropriate to their consumers, including newly introduced quotes, URL punctuation and parentheses. HTML URLs also escape scheme delimiters, backslashes and characters that browsers strip.
  • Dropping a file such as sale 50% off #1?.mp4 onto the timeline writes its src as a URL, probes its real length, and its thumbnail, waveform, peaks, inspector actions (Remove background, the audio check, color grading) load. Studio reads an authored src as a URL in one place, so each reader decodes it once. Renaming a file also renames references already spelled percent-encoded, such as assets/My%20clip.mp4.
  • Studio's media and loudness routes read a requested file the same way: the literal name when that file exists, otherwise the field as a URL. So a file named with %, # or ? gets its metadata, background removal and loudness normalize, and a ?v= query on an existing name is still ignored.
  • Studio finds an element by its own id, never by an earlier element whose data-id matches.
  • A raw file name containing %20 stays distinct from a name containing a space. References inside longer existing file names remain untouched. Numeric and named HTML entities reach the native decoder before candidate matching, and a second rename preserves literal backslashes in physical names.

How

One shared asset-path decoder handles well-formed escape runs and leaves malformed or bare percent text literal. Studio-server keeps its existing decoder export for compatibility.

One shared source scanner reads actual attributes by their own quote delimiters and exact names, skipping comments and raw text bodies. Native HTML entity decoding runs before URL decoding. The source patcher, timing compiler, composition collector and codec scan use that boundary. Function replacements preserve literal dollar signs, and linked audio reads use the same attribute reader.

The rename rewriter checks identity in the reference's actual grammar before replacing it. Core's URL consumers split query/fragment suffixes, decode once, then check containment and resolve the physical file. Linked CSS URLs decode CSS escapes before URL decoding and physical path resolution, then normalize raw URL separators before percent decoding and encode the rebased path for URL grammar. Decoded whitespace remains part of the file name, and query/fragment suffixes are escaped when written back into CSS. Composition paths and the shared asset-existence callback remain physical paths. Clip ids derived from a media src (the rename when a clip's media is replaced) decode the src first, so a dropped my clip.mp4, now written my%20clip.mp4, still renames to its new file's name.

Validation

Focused tests run serially on Linux with at most two workers. The rename suite passes three consecutive runs. Shared sub-composition paths, core bundling, media duration, source editing and serving routes have focused regressions. The linked-stylesheet escaped-space regression checks both the rebased URL and the inlined asset contents. Separate cases preserve escaped leading-space file identities, keep CSS line continuations from becoming filename characters, and serialize decoded suffixes safely. A linked CSS sibling-file witness distinguishes raw URL separators from percent-encoded physical backslashes. The CLI preview test fetches the rebased URL and checks the exact asset bytes, including percent-encoded @ names. Regressions invoke the real rename route and bundler, resolve raw JSON paths against real files, rename quoted references twice, and assert inlined asset contents from directories containing literal percent and URL punctuation. The source patcher and linked-audio suites cover exact attribute names, both quote kinds and dollar signs, an apostrophe in src, data-src before src, and an earlier data-id equal to the target id. The timeline drop test fails with the src written raw, and the encoded-rename test fails on main's rename route (the reference keeps its old name). The authored-src reader, both dropped-asset probes, the media-probe source, the media and loudness routes and a nested-composition snapshot link each have a test that fails without the change. The whole Studio suite (7751) and Studio server suite (1211) pass.

Old implementations are restored temporarily to prove the regressions fail at their intended assertions. Typechecks, lint, formatting and comment checks run against the final revision.

Before

The Studio “Remove background” action with “Keep sound” saves duplicate src attributes for an apostrophe-named video; reloading shows the original red video. The removal service and job progress use a deterministic fixture. Studio's source save and reload are real.

Before: reload retains the original red video

A file named sale 50% off #1?.mp4 (a 4 s test pattern) dropped onto the timeline of a fixture project built for this capture: the clip lands with the default 5 s length and an empty strip, because its URL is cut at #.

Before: dropped clip with an empty strip and a 5 s length

After

The same action replaces the source once and preserves the linked audio's original file path. Reloading shows the blue cutout fixture.

After: reload shows the blue cutout and retained audio

The same drop: the clip shows its frames and its real 4 s length.

After: dropped clip with its frames and a 4 s length

Known limits

A clip dropped before this change keeps a raw src. If that name contains #, ? or a literal %XX (a#b.mp4, 50%20off.mp4), replacing its media keeps the old clip id instead of renaming it. Nothing is lost; a src cannot say whether it was written raw or encoded.

Two neighbouring gaps exist on main today and get their own follow-up: choosing an imported font whose file name has a lone % fails to save, and the image field in the property panel writes a picked file's src and background url() without encoding.

A media or loudness request path that names no existing file is now read as a URL and decoded once, so a requested background-removal outputPath of assets/x%20y.webm writes assets/x y.webm. Studio itself never sends an output path.

JSX/TSX references keep their existing raw string handling; JSX attribute grammar is outside this change.

Quoted URLs containing ) in linked CSS still fall outside the existing rebasing grammar. Broader CSS parsing remains a follow-up.

The producer's existing file server keeps its per-segment fallback for malformed URL escapes. Its handling of a segment mixing a bare percent and an escape is unchanged.

A browser sends src="100%.png" with the % unescaped, and decoding the whole path threw, so Studio answered 500. Only well-formed escapes are decoded now.
…pshot server

One shared decode rule for request paths; the image thumbnail route's malformed-escape 400 could no longer fire, so a name that is not a file is a 404.
@github-actions

github-actions Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Edit accuracy: accurate 2059 (base branch 2059), smooth 1638 of those

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (0)

…ting attributes

An attribute value is matched by its own quote kind, so src="it's.png" is replaced instead of duplicated, and every insert uses a function replacer, so $1 or $& in a file name is written as is.
@miguel-heygen miguel-heygen changed the title fix(studio): serve files whose name has a bare percent sign fix(studio): stop file names with %, apostrophes or $ from breaking Studio Oct 7, 2026
@mintlify

mintlify Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated
hyperframes 🟢 Ready View Preview Oct 8, 2026, 3:13 PM

💡 Tip: Enable Automations to automatically generate PRs for you.

@miguel-heygen
miguel-heygen marked this pull request as ready for review October 7, 2026 09:59

@terencecho terencecho left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review at 2b2c4f177f58233796fbf06a022a83a26e4c1469 — requesting changes for a CSS asset regression.

packages/core/src/compiler/htmlBundler.ts:150-153: rebaseCssUrls now feeds the source CSS token directly to encodeUrlPath, which treats a CSS escape as a literal filesystem backslash. For a linked css/theme.css containing url(icon\ 1.png) and a physical css/icon 1.png, the effective base emits url(css/icon\ 1.png) (the backslash escapes the space in CSS); this head emits url(css/icon%5C%201.png). The latter requests a backslash-containing filename rather than css/icon 1.png, so the image disappears from the bundled composition. An exact-head bundleToSingleHtml(project, { inlineAssets: false }) run produced url(css/icon%5C%201.png); the actual static project server returned 404 for that bundled URL and 200 for /css/icon%201.png. I did not run a full browser render. Please decode CSS escapes in the CSS URL grammar before resolving/encoding the physical filename, and add a linked-stylesheet bundling regression for an escaped-space filename.

The required Linux Test and Windows test lanes also fail at packages/cli/src/server/studioServer.previewAssets.test.ts:70: the new URL rebase correctly emits assets/pic%402x.png, but the unchanged assertion expects the raw spelling assets/pic@2x.png. The preview asset route decodes %40 to @; this looks like a stale spelling assertion, not a second asset-serving defect. Please update the assertion while retaining the following fetchAsset(nested) byte check, then rerun the required checks. The capture check used an older PR body without ## Before/## After; those sections are now present, so that check also needs a fresh run. I did not run the full local suite.

— tai

@miguel-heygen

Copy link
Copy Markdown
Collaborator Author

Thanks. Both points are fixed at dfb749a:

  • CSS escapes: linked stylesheet URLs now decode CSS escapes before resolving and re-encoding the file name, so url(icon\ 1.png) rebases to the physical css/icon 1.png instead of icon%5C%201.png. The linked-stylesheet regression (htmlBundler.test.ts, "decodes CSS escapes before rebasing linked stylesheet asset URLs") checks the rebased URL and the inlined bytes. Escaped leading spaces, CSS line continuations and suffix escaping have their own cases.
  • Preview assets: the assertion now expects assets/pic%402x.png and keeps the byte check through the route.

This head also folds in related spelling fixes: a dropped timeline asset writes its src as a URL and Studio reads every authored src back as a URL, a rename matches references already spelled percent-encoded, the source patcher never matches data-id or data-src, and the media and loudness routes find files whose names hold %, # or ?. The earlier red Build came from @codemirror/language 6.13.0, which imported @codemirror/streamparser without declaring it; 6.13.1 fixed that upstream. The edit-accuracy gate was red only because its shards were skipped behind that build.

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve. Both changes the earlier review asked for are in, and I found no blocking defects at dfb749a3.

What I checked

  • CSS escapes: rebaseCssUrls now decodes CSS escapes before resolving, then re-encodes the rebased path.
    • I ran the real bundleToSingleHtml(project, { inlineAssets: false }) on a project whose css/theme.css uses url(icon\ 1.png) next to a real css/icon 1.png. It now produces url(css/icon%201.png).
    • These all rebase to the correct file: hex escapes (\31 , \000020), \27, a line continuation, %, $, non-ASCII, an already percent-encoded name, and ?/# suffixes.
    • data: and https: URLs are left alone.
    • With inlineAssets: true, each one inlines the right bytes.
  • Preview assets: studioServer.previewAssets.test.ts expects assets/pic%402x.png, and it still fetches that URL and checks the bytes.
  • Every path that reads or writes a file reference: timeline drop, the authored-src reader, thumbnails, peaks, the media and loudness routes, preview serving, sub-composition rewriting, producer media resolution, lint and rename.
    • Each path decodes once.
    • Encoded spellings that Studio now writes still resolve in frame extraction, audio mixing and duration probing, because those try decoded variants.
    • Decoded request paths are still checked against the project root.
  • Callers: callers of the removed exports are gone or updated.

Verification

  • Tests:
    • core compiler: 528/528
    • parsers: 1245 passed
    • cli server/static/validate: 257/257
    • studio-server: 1211 passed
    • studio: 7739 passed. The only failures are two browser tests that need Chrome, and this PR doesn't touch them.
    • producer natural-duration: 7/7
  • Six mutants, all caught:
    • dropping CSS escape decoding
    • dropping the rebased-path encoding
    • dropping suffix escaping
    • reverting the shared decoder to a single decodeURIComponent
    • suffix-matching attribute names in the source patcher
    • writing the dropped src raw
  • CI: required checks are green. The timeline viewport gate is not required, and it fails with the same budget miss on the last four commits on main.

Non-blocking

  • files.ts rename and JSON's escaped slash: the guard that skips a match inside a longer existing path no longer understands \/ in a non-script HTML attribute.
    • Example: with logo.png and assets/my logo.png in the project, renaming logo.png to brand.png rewrites data-props='{"img":"assets\/my logo.png"}' to assets\/my brand.png. Main leaves it alone.
    • The old "compares escaped spellings by their text" test was deleted, not adapted.
    • This is rare, since JSON.stringify does not write \/.
  • Two CSS cases that already fail on main:
    • In a linked stylesheet, @import "my\ file.css" is still not decoded for CSS escapes, so it stays un-inlined and resolves against the document root.
    • url("it\'s.png") and url("p\(1\).png") are still not matched by the CSS url pattern, so they are not rebased. The known-limits note mentions only ).

— Rames

@miguel-heygen
miguel-heygen dismissed terencecho’s stale review October 8, 2026 08:34

Requested on an older commit; later commits answer it (see the thread). Current head is dfb749a.

…es-with-a-bare-percent

# Conflicts:
#	packages/studio/src/utils/timelineAssetDrop.ts

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-approving at a3afb8e8. Since my approval at dfb749a3, four commits:

  • 8b0259a3 deletes the one-line design note under docs/contracts/. No code.
  • a8ea52f0, a main merge: the remerge-diff is empty, so it resolved cleanly.
  • f6833e9f, a main merge: the one conflict is the import block of packages/studio/src/utils/timelineAssetDrop.ts. Both lines are kept, and both are live. encodeUrlPath is still used to write the dropped clip's src (line 109). buildTimelineAssetId is main's re-export from @hyperframes/core/timeline-asset-id (#5169). No other conflicts.
  • a3afb8e8: matchesGeneratedId and replacementTimelineAssetId now derive ids from decodedUrlPath(src). Because this PR writes dropped clips with an encoded src (my%20clip.mp4), the generated-id check compared my_clip against my_20clip. A replaced clip was therefore never recognised as auto-named and kept its old id. decodedUrlPath decodes only well-formed escapes and strips any query or fragment, so a bare % can't throw. Core already depends on @hyperframes/parsers, so the import adds no new package edge. The new test covers both call sites: dropping either decode fails it (new_20take, or no rename).

CI is green at this head.

— Rames

@miguel-heygen
miguel-heygen added this pull request to the merge queue Oct 8, 2026
Merged via the queue into main with commit 621ddd4 Oct 8, 2026
170 checks passed
@miguel-heygen
miguel-heygen deleted the fix/studio-serves-files-with-a-bare-percent branch October 8, 2026 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants