From 36a145faff50ba5d2637969523659862cd8d9750 Mon Sep 17 00:00:00 2001 From: Paulo Reis Date: Sun, 5 Apr 2026 08:15:11 -0300 Subject: [PATCH] refactor: decouple dragDropPaths from imagePaste and harden image checks - Check image extension against the cleaned path (post quote/escape stripping) so quoted or backslash-escaped image drops are reliably routed to the image paste handler. - Inline the image extension regex and drop the imagePaste/fsOperations imports so the module (and its tests) no longer pull in `bun:bundle` and the heavier fs wrapper chain. Use plain `fs.existsSync` for the on-disk check. - Add tests covering quoted image paths, uppercase extensions, backslash-escaped image paths, escaped real files with spaces, mixed segments containing an image, quoted-nonexistent paths, and leading or trailing whitespace. --- src/utils/dragDropPaths.test.ts | 68 ++++++++++++++++++++++++++++++++- src/utils/dragDropPaths.ts | 22 +++++++---- 2 files changed, 82 insertions(+), 8 deletions(-) diff --git a/src/utils/dragDropPaths.test.ts b/src/utils/dragDropPaths.test.ts index 7f41fbec..0c3aad4a 100644 --- a/src/utils/dragDropPaths.test.ts +++ b/src/utils/dragDropPaths.test.ts @@ -1,4 +1,7 @@ -import { expect, test, describe } from 'bun:test' +import { afterAll, beforeAll, describe, expect, test } from 'bun:test' +import { mkdtempSync, rmSync, writeFileSync } from 'fs' +import { tmpdir } from 'os' +import { join } from 'path' import { extractDraggedFilePaths } from './dragDropPaths.js' describe('extractDraggedFilePaths', () => { @@ -6,6 +9,19 @@ describe('extractDraggedFilePaths', () => { const thisFile = import.meta.path const packageJson = `${process.cwd()}/package.json` + // Temp dir with a file whose name contains a space, for Finder-drag + // backslash-escape tests. + let tmpDir: string + let spacedFile: string + beforeAll(() => { + tmpDir = mkdtempSync(join(tmpdir(), 'dragdrop-test-')) + spacedFile = join(tmpDir, 'my file.txt') + writeFileSync(spacedFile, 'test') + }) + afterAll(() => { + rmSync(tmpDir, { recursive: true, force: true }) + }) + test('detects a single absolute file path', () => { const result = extractDraggedFilePaths(thisFile) expect(result).toEqual([thisFile]) @@ -62,4 +78,54 @@ describe('extractDraggedFilePaths', () => { const result = extractDraggedFilePaths(`'${thisFile}'`) expect(result).toEqual([thisFile]) }) + + test('returns empty for a double-quoted image path', () => { + // Regression guard: image detection must see through outer quotes so + // quoted image drops still route to the image paste handler. + expect(extractDraggedFilePaths('"/Users/foo/shot.png"')).toEqual([]) + }) + + test('returns empty for a single-quoted image path', () => { + expect(extractDraggedFilePaths("'/Users/foo/shot.jpg'")).toEqual([]) + }) + + test('returns empty for an uppercase image extension', () => { + expect(extractDraggedFilePaths('/Users/foo/SHOT.PNG')).toEqual([]) + }) + + if (process.platform !== 'win32') { + test('returns empty for a backslash-escaped image path', () => { + // Finder drags escape spaces with backslashes; the image check must + // apply after escape stripping. + expect( + extractDraggedFilePaths('/Users/foo/my\\ shot.png'), + ).toEqual([]) + }) + + test('resolves a backslash-escaped path to a real file on disk', () => { + // `spacedFile` is an existing file with a space in its name; the + // raw form matches what a terminal delivers on Finder drag. + const escaped = spacedFile.replace(/ /g, '\\ ') + expect(extractDraggedFilePaths(escaped)).toEqual([spacedFile]) + }) + } + + test('returns empty when mixed segments include an image file', () => { + // All-or-nothing: one image in the group disqualifies the whole paste + // so it can be handled by the image paste handler instead. + expect( + extractDraggedFilePaths(`${thisFile}\n/Users/foo/shot.png`), + ).toEqual([]) + }) + + test('returns empty for a single-quoted nonexistent path', () => { + // Quoted but nonexistent — exists check still runs after unquoting. + expect(extractDraggedFilePaths("'/definitely/nonexistent.ts'")).toEqual( + [], + ) + }) + + test('trims surrounding whitespace from the whole paste', () => { + expect(extractDraggedFilePaths(` ${thisFile} `)).toEqual([thisFile]) + }) }) diff --git a/src/utils/dragDropPaths.ts b/src/utils/dragDropPaths.ts index 19d3b519..a940efb7 100644 --- a/src/utils/dragDropPaths.ts +++ b/src/utils/dragDropPaths.ts @@ -1,6 +1,10 @@ +import { existsSync } from 'fs' import { isAbsolute } from 'path' -import { getFsImplementation } from './fsOperations.js' -import { isImageFilePath } from './imagePaste.js' + +// Inlined to avoid pulling the full `imagePaste.ts` module (which imports +// `bun:bundle`) into this file's dependency graph. Must stay in sync with +// `IMAGE_EXTENSION_REGEX` in `./imagePaste.ts`. +const IMAGE_EXTENSION_REGEX = /\.(png|jpe?g|gif|webp)$/i /** * Detect absolute file paths in pasted text (typically from drag-and-drop). @@ -19,7 +23,6 @@ export function extractDraggedFilePaths(text: string): string[] { if (segments.length === 0) return [] - const fs = getFsImplementation() const cleaned: string[] = [] for (const raw of segments) { @@ -35,11 +38,16 @@ export function extractDraggedFilePaths(text: string): string[] { p = p.replace(/\\(.)/g, '$1') } + // Image files are handled by the upstream image paste handler. + // Check against the cleaned path so quoted/escaped image paths like + // `"/foo/shot.png"` or `/foo/my\ shot.png` are reliably excluded. + if (IMAGE_EXTENSION_REGEX.test(p)) return [] if (!isAbsolute(p)) return [] - // Image files are handled by the upstream image paste handler - if (isImageFilePath(raw)) return [] - // Verify the path actually exists on disk - if (!fs.existsSync(p)) return [] + // Verify the path actually exists on disk. Plain `fs.existsSync` is + // used intentionally here instead of the wrapped `getFsImplementation` + // to keep this module free of the heavy `fsOperations` dependency + // chain — this is a pure existence check with no permission semantics. + if (!existsSync(p)) return [] cleaned.push(p) }