Skip to content

Show an error instead of a 500 when the folder viewer opens a path OSC 367 refuses - #877

Merged
nedtwigg merged 1 commit into
mainfrom
fix/folder-viewer-unopenable-path
Oct 1, 2026
Merged

nedtwigg merged 1 commit into
mainfrom
fix/folder-viewer-unopenable-path

Conversation

@dormouse-bot

Copy link
Copy Markdown
Collaborator

The builtin:folder viewer turned a click on some files into a 500 and a misleading "Folder viewer unavailable" status. Since #856, its select and activate call openSequence({ path, preview }), which throws RangeError for any path validToolOpenPath rejects. The throw escaped the route, so startCapabilityViewer answered 500 with its generic unavailable text. This PR checks the path first and answers { ok: false, error }, which the page already shows as its status line. Nothing is written to the terminal for such a path.

Paths that reach this:

  • a POSIX file name containing a control character (a newline is legal in a file name);
  • a path longer than 2048 characters;
  • a Windows UNC path (\\server\share\…). validToolOpenPath accepts only / or X:\ prefixes, and the viewer lists realpath(dir), so a folder opened on a network share fails on every click. Before Add the OSC 367 open verb and show failed opens in the preview slot #856 these went through client.toolSurface({ file }) with no such filter.

Whether the host should accept UNC paths in OSC 367 open is a separate question for a maintainer: it widens what a Tool can ask the host to open, so this PR leaves validToolOpenPath unchanged and only makes the refusal readable.

The new test in dor-tools-builtin/test/folder-viewer.test.mjs drives the extracted oscOpen with one valid path and the three refused shapes. Before this change, the refused shapes threw instead of answering an error. I couldn't run it locally because this sandbox can't reach the npm registry, so CI is the first run.

The same commit fixes two stale descriptions left by #856: the folder page's comment still said each POST is "its own control connection" (it is an HTTP request now), and the osc.ts entry in docs/specs/dor-tools-lib.md omitted the open encoder. The dor-tools-builtin.md budget is ratcheted from 1050 to 1100 for the one added clause.

…67 refuses

The folder viewer's select and activate wrote openSequence() unguarded, which
throws for a path validToolOpenPath rejects (a control character, a UNC root,
over 2048 characters); the throw became a 500 and the page showed only
"Folder viewer unavailable". Check the path first and answer { ok: false }.

Also fix the stale "control connection" comment in the folder page and the
dor-tools-lib osc.ts map entry, which omitted the open encoder.
@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying mouseterm with  Cloudflare Pages  Cloudflare Pages

Latest commit: 32db641
Status: ✅  Deploy successful!
Preview URL: https://55b23b64.mouseterm.pages.dev
Branch Preview URL: https://fix-folder-viewer-unopenable.mouseterm.pages.dev

View logs

@nedtwigg nedtwigg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed the existing path guard, OSC encoder, HTTP response, and status-line handling. Refused paths now produce a readable error without emitting OSC; the existing trust policy stays intact. CI and bot self-review passed.

@nedtwigg
nedtwigg merged commit 193533a into main Oct 1, 2026
11 checks passed

This branch is waiting to be deployed

1 waiting deployment
hosted-preview — 32db6416 Waiting Oct 1, 2026 by nedtwigg via cleanup #644
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.

2 participants