From 32db641636f281a9a04d036476d1edf51d0b3b0e Mon Sep 17 00:00:00 2001 From: dormouse-bot <287024035+dormouse-bot@users.noreply.github.com> Date: Thu, 1 Oct 2026 13:20:35 +0000 Subject: [PATCH] Answer an error, not a 500, when the folder viewer opens a path OSC 367 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. --- docs/specs/dor-tools-builtin.md | 4 ++-- docs/specs/dor-tools-lib.md | 2 +- dor-tools-builtin/src/folder-viewer-page.ts | 2 +- dor-tools-builtin/src/folder-viewer.ts | 20 ++++++++++++------- dor-tools-builtin/test/folder-viewer.test.mjs | 16 ++++++++++++++- scripts/spec-word-budgets.json | 2 +- 6 files changed, 33 insertions(+), 13 deletions(-) diff --git a/docs/specs/dor-tools-builtin.md b/docs/specs/dor-tools-builtin.md index faeb0c4d9..1c3d222e1 100644 --- a/docs/specs/dor-tools-builtin.md +++ b/docs/specs/dor-tools-builtin.md @@ -58,10 +58,10 @@ Source of truth: `readEditableFile` / `saveEditableFile` in `dor-tools-builtin/s - **Must list dotfiles.** Git-ignored entries are dimmed and shown by default, with a show/hide toggle. - **Must compact a directory containing exactly one child directory and nothing else into a slash-separated row**, continuing up to 32 levels per listing; hidden/ignored entries still count as siblings. Never compact through a symlink child. Refresh and Collapse all preserve the ordinary select/activate contract. - **Must cut a listing to its first 5,000 entries in display order**, directories first, from at most 100,000 names read. -- **Must route the page's select and activate through its own process**: a same-origin POST to its capability listener, which writes it to the Tool's terminal as an OSC 367 `open` (`docs/specs/dor-tool.md` → OSC 367), `preview` for a select, in arrival order. The page learns only that it was sent. +- **Must route the page's select and activate through its own process**: a same-origin POST to its capability listener, which writes it to the Tool's terminal as an OSC 367 `open` (`docs/specs/dor-tool.md` → OSC 367), `preview` for a select, in arrival order. The page learns only that it was sent, or, for a path `validToolOpenPath` refuses, an error instead of a write. - **Must hold an activate until every select in flight settles**, keeping selects concurrent. (rationale) -Source of truth: `startFolderViewer` / `runFolderViewer` in `dor-tools-builtin/src/folder-viewer.ts`; `folderViewerPage` in `dor-tools-builtin/src/folder-viewer-page.ts`. Tests: `dor-tools-builtin/test/folder-viewer.test.mjs`, `the folder entry selects and activates with OSC 367 open, in the order the page sends them` in `dor/test/builtin-viewers.test.mjs`. +Source of truth: `startFolderViewer` / `runFolderViewer` / `oscOpen` in `dor-tools-builtin/src/folder-viewer.ts`; `folderViewerPage` in `dor-tools-builtin/src/folder-viewer-page.ts`. Tests: `dor-tools-builtin/test/folder-viewer.test.mjs`, `the folder entry selects and activates with OSC 367 open, in the order the page sends them` in `dor/test/builtin-viewers.test.mjs`. ## Error viewer diff --git a/docs/specs/dor-tools-lib.md b/docs/specs/dor-tools-lib.md index f8c734b3f..566f5b838 100644 --- a/docs/specs/dor-tools-lib.md +++ b/docs/specs/dor-tools-lib.md @@ -5,7 +5,7 @@ ## Files -- `dor-tools-lib/src/osc.ts` — OSC 367: the Tool's `serve` / `state` encoders and the host's parsers. +- `dor-tools-lib/src/osc.ts` — OSC 367: the Tool's `serve` / `state` / `open` encoders and the host's parsers. - `dor-tools-lib/src/protocol.ts` — the iframe save channel's messages in both directions, versioned by `dorTool`. - `dor-tools-lib/src/frame.ts` — the Tool side of the save channel for a framed page. - `dor-tools-lib/src/sanitize.ts` — the package's own guards for untrusted input. diff --git a/dor-tools-builtin/src/folder-viewer-page.ts b/dor-tools-builtin/src/folder-viewer-page.ts index 14fd0f8c1..e8997589d 100644 --- a/dor-tools-builtin/src/folder-viewer-page.ts +++ b/dor-tools-builtin/src/folder-viewer-page.ts @@ -178,7 +178,7 @@ const SCRIPT = `(function () { } // The latest request alone owns the status line; a superseded preview is not an error. - // Each POST is its own control connection, so an activate waits for every + // Each POST is its own HTTP connection, so an activate waits for every // select in flight: sent at once, it could overtake a double-click's select and // open beside the slot instead of pinning it. Selects stay concurrent, so the // newest supersedes. diff --git a/dor-tools-builtin/src/folder-viewer.ts b/dor-tools-builtin/src/folder-viewer.ts index f5074b5d9..650cbbdda 100644 --- a/dor-tools-builtin/src/folder-viewer.ts +++ b/dor-tools-builtin/src/folder-viewer.ts @@ -3,7 +3,7 @@ import { opendir, realpath, stat } from 'node:fs/promises'; import type { IncomingMessage } from 'node:http'; import { join } from 'node:path'; import { resolveBinaryPath, spawnAndCapture } from 'dor-lib-common'; -import { openSequence } from 'dor-tools-lib/osc'; +import { openSequence, validToolOpenPath } from 'dor-tools-lib/osc'; import { folderViewerPage } from './folder-viewer-page.js'; import { announceViewer, HttpError, isInsideRoot, pathSegments, readJsonBody, reply, startCapabilityViewer } from './viewer-server.js'; @@ -160,11 +160,17 @@ export async function startFolderViewer(input: string, { open }: { open: FolderO * order the page sends them; the host answers nothing, showing a failure in * the preview slot. */ export async function runFolderViewer(dir: string): Promise { - const viewer = await startFolderViewer(dir, { - open: async (path, preview) => { - process.stdout.write(openSequence({ path, preview })); - return { ok: true, status: 'sent' }; - }, - }); + const viewer = await startFolderViewer(dir, { open: oscOpen(text => process.stdout.write(text)) }); return announceViewer(viewer, viewer.root); } + +/** Writes each open as an OSC 367 `open`. A path the host would refuse (a + * control character, a UNC root, over the length limit) answers an error the + * page shows, rather than throwing into a 500. */ +export function oscOpen(write: (text: string) => void): FolderOpen { + return async (path, preview) => { + if (!validToolOpenPath(path)) return { ok: false, error: `Cannot open ${JSON.stringify(path)} from a Tool` }; + write(openSequence({ path, preview })); + return { ok: true, status: 'sent' }; + }; +} diff --git a/dor-tools-builtin/test/folder-viewer.test.mjs b/dor-tools-builtin/test/folder-viewer.test.mjs index 9ef3aa369..5b54fb420 100644 --- a/dor-tools-builtin/test/folder-viewer.test.mjs +++ b/dor-tools-builtin/test/folder-viewer.test.mjs @@ -7,7 +7,7 @@ import { request } from 'node:http'; import { spawnSync } from 'node:child_process'; import { runInNewContext } from 'node:vm'; import { afterEach, beforeEach, test } from 'node:test'; -import { startFolderViewer } from '../dist/folder-viewer.js'; +import { oscOpen, startFolderViewer } from '../dist/folder-viewer.js'; import { folderViewerPage } from '../dist/folder-viewer-page.js'; const posixOnly = { skip: process.platform === 'win32' ? 'POSIX file names and symlinks' : false }; @@ -330,3 +330,17 @@ test('the page\'s Enter waits for a select in flight as a double-click does', as await settle(); assert.deepEqual(page.sent(), ['select a.txt', 'activate a.txt']); }); + +test('writes each open as OSC 367, and answers an error for a path the host would refuse', async () => { + const written = []; + const open = oscOpen(text => written.push(text)); + assert.deepEqual(await open('/x/a.txt', true), { ok: true, status: 'sent' }); + assert.equal(written.length, 1); + assert.match(written[0], /^\u001b]367;open;/); + for (const path of ['/x/line\nbreak.txt', '\\\\server\\share\\a.txt', '/' + 'a'.repeat(2048)]) { + const result = await open(path, false); + assert.equal(result.ok, false, path); + assert.match(result.error, /^Cannot open /); + } + assert.equal(written.length, 1); +}); diff --git a/scripts/spec-word-budgets.json b/scripts/spec-word-budgets.json index b3e6ae343..c8e440f2f 100644 --- a/scripts/spec-word-budgets.json +++ b/scripts/spec-word-budgets.json @@ -9,7 +9,7 @@ "docs/specs/dor-browser.md": 9350, "docs/specs/dor-cli.md": 6450, "docs/specs/dor-tool.md": 6250, - "docs/specs/dor-tools-builtin.md": 1050, + "docs/specs/dor-tools-builtin.md": 1100, "docs/specs/dor-tools-lib.md": 400, "docs/specs/glossary.md": 3000, "docs/specs/hosted.md": 1900,