From d82f66973fa0eeea0a09d19ce1ec08bb69a96449 Mon Sep 17 00:00:00 2001 From: Tommaso Sciortino Date: Thu, 12 Feb 2026 18:27:56 -0800 Subject: [PATCH] Fix drag and drop escaping (#18965) --- .../ui/components/shared/text-buffer.test.ts | 32 +- .../src/ui/components/shared/text-buffer.ts | 10 +- .../src/ui/hooks/atCommandProcessor.test.ts | 41 +++ .../cli/src/ui/hooks/atCommandProcessor.ts | 13 +- .../cli/src/ui/utils/clipboardUtils.test.ts | 327 ++++++++++-------- packages/cli/src/ui/utils/clipboardUtils.ts | 127 ++++--- packages/core/src/utils/paths.test.ts | 273 +++++++-------- packages/core/src/utils/paths.ts | 68 ++-- 8 files changed, 492 insertions(+), 399 deletions(-) diff --git a/packages/cli/src/ui/components/shared/text-buffer.test.ts b/packages/cli/src/ui/components/shared/text-buffer.test.ts index 50a7fe795b..85754f9f62 100644 --- a/packages/cli/src/ui/components/shared/text-buffer.test.ts +++ b/packages/cli/src/ui/components/shared/text-buffer.test.ts @@ -41,6 +41,7 @@ import { getTransformedImagePath, } from './text-buffer.js'; import { cpLen } from '../../utils/textUtils.js'; +import { escapePath } from '@google/gemini-cli-core'; const defaultVisualLayout: VisualLayout = { visualLines: [''], @@ -1077,14 +1078,16 @@ describe('useTextBuffer', () => { useTextBuffer({ viewport, escapePastedPaths: true }), ); // Construct escaped path string: "/path/to/my\ file.txt /path/to/other.txt" - const escapedFile1 = file1.replace(/ /g, '\\ '); - const filePaths = `${escapedFile1} ${file2}`; + + const filePaths = `${escapePath(file1)} ${file2}`; act(() => result.current.insert(filePaths, { paste: true })); - expect(getBufferState(result).text).toBe(`@${escapedFile1} @${file2} `); + expect(getBufferState(result).text).toBe( + `@${escapePath(file1)} @${file2} `, + ); }); - it('should only prepend @ to valid paths in multi-path paste', () => { + it('should not prepend @ unless all paths are valid', () => { const validFile = path.join(tempDir, 'valid.txt'); const invalidFile = path.join(tempDir, 'invalid.jpg'); fs.writeFileSync(validFile, ''); @@ -1098,7 +1101,7 @@ describe('useTextBuffer', () => { ); const filePaths = `${validFile} ${invalidFile}`; act(() => result.current.insert(filePaths, { paste: true })); - expect(getBufferState(result).text).toBe(`@${validFile} ${invalidFile} `); + expect(getBufferState(result).text).toBe(`${validFile} ${invalidFile}`); }); }); @@ -2869,12 +2872,26 @@ describe('Unicode helper functions', () => { }); }); +const mockPlatform = (platform: string) => { + vi.stubGlobal( + 'process', + Object.create(process, { + platform: { + get: () => platform, + }, + }), + ); +}; + describe('Transformation Utilities', () => { afterEach(() => { vi.restoreAllMocks(); + vi.unstubAllGlobals(); }); describe('getTransformedImagePath', () => { + beforeEach(() => mockPlatform('linux')); + it('should transform a simple image path', () => { expect(getTransformedImagePath('@test.png')).toBe('[Image test.png]'); }); @@ -2905,11 +2922,6 @@ describe('Transformation Utilities', () => { expect(getTransformedImagePath(input)).toBe('[Image image2x.png]'); }); - it('should handle Windows-style backslash paths on any platform', () => { - const input = '@C:\\Users\\foo\\screenshots\\image2x.png'; - expect(getTransformedImagePath(input)).toBe('[Image image2x.png]'); - }); - it('should handle escaped spaces in paths', () => { const input = '@path/to/my\\ file.png'; expect(getTransformedImagePath(input)).toBe('[Image my file.png]'); diff --git a/packages/cli/src/ui/components/shared/text-buffer.ts b/packages/cli/src/ui/components/shared/text-buffer.ts index d0f425129b..e641633e97 100644 --- a/packages/cli/src/ui/components/shared/text-buffer.ts +++ b/packages/cli/src/ui/components/shared/text-buffer.ts @@ -2814,15 +2814,7 @@ export function useTextBuffer({ paste && escapePastedPaths ) { - let potentialPath = ch.trim(); - const quoteMatch = potentialPath.match(/^'(.*)'$/); - if (quoteMatch) { - potentialPath = quoteMatch[1]; - } - - potentialPath = potentialPath.trim(); - - const processed = parsePastedPaths(potentialPath); + const processed = parsePastedPaths(ch.trim()); if (processed) { textToInsert = processed; } diff --git a/packages/cli/src/ui/hooks/atCommandProcessor.test.ts b/packages/cli/src/ui/hooks/atCommandProcessor.test.ts index 7a9601a4c6..76848ea821 100644 --- a/packages/cli/src/ui/hooks/atCommandProcessor.test.ts +++ b/packages/cli/src/ui/hooks/atCommandProcessor.test.ts @@ -145,6 +145,7 @@ describe('handleAtCommand', () => { afterEach(async () => { abortController.abort(); await fsPromises.rm(testRootDir, { recursive: true, force: true }); + vi.unstubAllGlobals(); }); it('should pass through query if no @ command is present', async () => { @@ -319,6 +320,46 @@ describe('handleAtCommand', () => { ); }, 10000); + it('should correctly handle double-quoted paths with spaces', async () => { + // Mock platform to win32 so unescapePath strips quotes + vi.stubGlobal( + 'process', + Object.create(process, { + platform: { + get: () => 'win32', + }, + }), + ); + + const fileContent = 'Content of file with spaces'; + const filePath = await createTestFile( + path.join(testRootDir, 'my folder', 'my file.txt'), + fileContent, + ); + // On Windows, the user might provide: @"path/to/my file.txt" + const query = `@"${filePath}"`; + + const result = await handleAtCommand({ + query, + config: mockConfig, + addItem: mockAddItem, + onDebugMessage: mockOnDebugMessage, + messageId: 126, + signal: abortController.signal, + }); + + const relativePath = getRelativePath(filePath); + expect(result).toEqual({ + processedQuery: [ + { text: `@${relativePath}` }, + { text: '\n--- Content from referenced files ---' }, + { text: `\nContent from @${relativePath}:\n` }, + { text: fileContent }, + { text: '\n--- End of content ---' }, + ], + }); + }); + it('should correctly handle file paths with narrow non-breaking space (NNBSP)', async () => { const nnbsp = '\u202F'; const fileContent = 'NNBSP file content.'; diff --git a/packages/cli/src/ui/hooks/atCommandProcessor.ts b/packages/cli/src/ui/hooks/atCommandProcessor.ts index 18dcf9a0de..e30f9abbc9 100644 --- a/packages/cli/src/ui/hooks/atCommandProcessor.ts +++ b/packages/cli/src/ui/hooks/atCommandProcessor.ts @@ -31,12 +31,13 @@ const REF_CONTENT_FOOTER = `\n${REFERENCE_CONTENT_END}`; * Regex source for the path/command part of an @ reference. * It uses strict ASCII whitespace delimiters to allow Unicode characters like NNBSP in filenames. * - * 1. \\. matches any escaped character (e.g., \ ). - * 2. [^ \t\n\r,;!?()\[\]{}.] matches any character that is NOT a delimiter and NOT a period. - * 3. \.(?!$|[ \t\n\r]) matches a period ONLY if it is NOT followed by whitespace or end-of-string. + * 1. "(?:[^"]*)" matches a double-quoted string (for Windows paths with spaces). + * 2. \\. matches any escaped character (e.g., \ ). + * 3. [^ \t\n\r,;!?()\[\]{}.] matches any character that is NOT a delimiter and NOT a period. + * 4. \.(?!$|[ \t\n\r]) matches a period ONLY if it is NOT followed by whitespace or end-of-string. */ export const AT_COMMAND_PATH_REGEX_SOURCE = - '(?:\\\\.|[^ \\t\\n\\r,;!?()\\[\\]{}.]|\\.(?!$|[ \\t\\n\\r]))+'; + '(?:(?:"(?:[^"]*)")|(?:\\\\.|[^ \\t\\n\\r,;!?()\\[\\]{}.]|\\.(?!$|[ \\t\\n\\r])))+'; interface HandleAtCommandParams { query: string; @@ -85,8 +86,8 @@ function parseAllAtCommands(query: string): AtCommandPart[] { }); } - // unescapePath expects the @ symbol to be present, and will handle it. - const atPath = unescapePath(fullMatch); + // We strip the @ before unescaping so that unescapePath can handle quoted paths correctly on Windows. + const atPath = '@' + unescapePath(fullMatch.substring(1)); parts.push({ type: 'atPath', content: atPath }); lastIndex = matchIndex + fullMatch.length; diff --git a/packages/cli/src/ui/utils/clipboardUtils.test.ts b/packages/cli/src/ui/utils/clipboardUtils.test.ts index 5b2df637c3..cfd9f115ba 100644 --- a/packages/cli/src/ui/utils/clipboardUtils.test.ts +++ b/packages/cli/src/ui/utils/clipboardUtils.test.ts @@ -62,15 +62,25 @@ import { spawnAsync } from '@google/gemini-cli-core'; // Keep static imports for stateless functions import { cleanupOldClipboardImages, - splitEscapedPaths, + splitDragAndDropPaths, parsePastedPaths, } from './clipboardUtils.js'; +const mockPlatform = (platform: string) => { + vi.stubGlobal( + 'process', + Object.create(process, { + platform: { + get: () => platform, + }, + }), + ); +}; + // Define the type for the module to use in tests type ClipboardUtilsModule = typeof import('./clipboardUtils.js'); describe('clipboardUtils', () => { - let originalPlatform: string; let originalEnv: NodeJS.ProcessEnv; // Dynamic module instance for stateful functions let clipboardUtils: ClipboardUtilsModule; @@ -83,7 +93,6 @@ describe('clipboardUtils', () => { beforeEach(async () => { vi.resetAllMocks(); - originalPlatform = process.platform; originalEnv = process.env; process.env = { ...originalEnv }; @@ -94,22 +103,13 @@ describe('clipboardUtils', () => { }); afterEach(() => { - Object.defineProperty(process, 'platform', { - value: originalPlatform, - }); - process.env = originalEnv; + vi.unstubAllGlobals(); vi.restoreAllMocks(); }); - const setPlatform = (platform: string) => { - Object.defineProperty(process, 'platform', { - value: platform, - }); - }; - describe('clipboardHasImage (Linux)', () => { it('should return true when wl-paste shows image type (Wayland)', async () => { - setPlatform('linux'); + mockPlatform('linux'); process.env['XDG_SESSION_TYPE'] = 'wayland'; vi.mocked(execSync).mockReturnValue(Buffer.from('')); // command -v succeeds vi.mocked(spawnAsync).mockResolvedValueOnce({ @@ -128,7 +128,7 @@ describe('clipboardUtils', () => { }); it('should return true when xclip shows image type (X11)', async () => { - setPlatform('linux'); + mockPlatform('linux'); process.env['XDG_SESSION_TYPE'] = 'x11'; vi.mocked(execSync).mockReturnValue(Buffer.from('')); // command -v succeeds vi.mocked(spawnAsync).mockResolvedValueOnce({ @@ -153,7 +153,7 @@ describe('clipboardUtils', () => { }); it('should return false if tool fails', async () => { - setPlatform('linux'); + mockPlatform('linux'); process.env['XDG_SESSION_TYPE'] = 'wayland'; vi.mocked(execSync).mockReturnValue(Buffer.from('')); vi.mocked(spawnAsync).mockRejectedValueOnce(new Error('wl-paste failed')); @@ -164,7 +164,7 @@ describe('clipboardUtils', () => { }); it('should return false if no image type is found', async () => { - setPlatform('linux'); + mockPlatform('linux'); process.env['XDG_SESSION_TYPE'] = 'wayland'; vi.mocked(execSync).mockReturnValue(Buffer.from('')); vi.mocked(spawnAsync).mockResolvedValueOnce({ @@ -178,7 +178,7 @@ describe('clipboardUtils', () => { }); it('should return false if tool not found', async () => { - setPlatform('linux'); + mockPlatform('linux'); process.env['XDG_SESSION_TYPE'] = 'wayland'; vi.mocked(execSync).mockImplementation(() => { throw new Error('Command not found'); @@ -195,7 +195,7 @@ describe('clipboardUtils', () => { const mockTempDir = path.join('/tmp/global', 'images'); beforeEach(() => { - setPlatform('linux'); + mockPlatform('linux'); vi.mocked(fs.mkdir).mockResolvedValue(undefined); vi.mocked(fs.unlink).mockResolvedValue(undefined); }); @@ -363,65 +363,86 @@ describe('clipboardUtils', () => { }); }); - describe('splitEscapedPaths', () => { - it('should return single path when no spaces', () => { - expect(splitEscapedPaths('/path/to/image.png')).toEqual([ - '/path/to/image.png', - ]); + describe('splitDragAndDropPaths', () => { + describe('in posix', () => { + beforeEach(() => mockPlatform('linux')); + + it.each([ + ['empty string', '', []], + ['single path no spaces', '/path/to/image.png', ['/path/to/image.png']], + [ + 'simple space-separated paths', + '/img1.png /img2.png', + ['/img1.png', '/img2.png'], + ], + [ + 'three paths', + '/a.png /b.jpg /c.heic', + ['/a.png', '/b.jpg', '/c.heic'], + ], + ['escaped spaces', '/my\\ image.png', ['/my image.png']], + [ + 'multiple paths with escaped spaces', + '/my\\ img1.png /my\\ img2.png', + ['/my img1.png', '/my img2.png'], + ], + [ + 'multiple escaped spaces', + '/path/to/my\\ cool\\ image.png', + ['/path/to/my cool image.png'], + ], + [ + 'consecutive spaces', + '/img1.png /img2.png', + ['/img1.png', '/img2.png'], + ], + [ + 'trailing/leading whitespace', + ' /img1.png /img2.png ', + ['/img1.png', '/img2.png'], + ], + ['whitespace only', ' ', []], + ['quoted path with spaces', '"/my image.png"', ['/my image.png']], + [ + 'mixed quoted and unquoted', + '"/my img1.png" /my\\ img2.png', + ['/my img1.png', '/my img2.png'], + ], + [ + 'quoted with escaped quotes', + "'/derp/my '\\''cool'\\'' image.png'", + ["/derp/my 'cool' image.png"], + ], + ])('should escape %s', (_, input, expected) => { + expect([...splitDragAndDropPaths(input)]).toEqual(expected); + }); }); - it('should split simple space-separated paths', () => { - expect(splitEscapedPaths('/img1.png /img2.png')).toEqual([ - '/img1.png', - '/img2.png', - ]); - }); + describe('in windows', () => { + beforeEach(() => mockPlatform('win32')); - it('should split three paths', () => { - expect(splitEscapedPaths('/a.png /b.jpg /c.heic')).toEqual([ - '/a.png', - '/b.jpg', - '/c.heic', - ]); - }); - - it('should preserve escaped spaces within filenames', () => { - expect(splitEscapedPaths('/my\\ image.png')).toEqual(['/my\\ image.png']); - }); - - it('should handle multiple paths with escaped spaces', () => { - expect(splitEscapedPaths('/my\\ img1.png /my\\ img2.png')).toEqual([ - '/my\\ img1.png', - '/my\\ img2.png', - ]); - }); - - it('should handle path with multiple escaped spaces', () => { - expect(splitEscapedPaths('/path/to/my\\ cool\\ image.png')).toEqual([ - '/path/to/my\\ cool\\ image.png', - ]); - }); - - it('should handle multiple consecutive spaces between paths', () => { - expect(splitEscapedPaths('/img1.png /img2.png')).toEqual([ - '/img1.png', - '/img2.png', - ]); - }); - - it('should handle trailing and leading whitespace', () => { - expect(splitEscapedPaths(' /img1.png /img2.png ')).toEqual([ - '/img1.png', - '/img2.png', - ]); - }); - - it('should return empty array for empty string', () => { - expect(splitEscapedPaths('')).toEqual([]); - }); - - it('should return empty array for whitespace only', () => { - expect(splitEscapedPaths(' ')).toEqual([]); + it.each([ + ['double quoted path', '"C:\\my image.png"', ['C:\\my image.png']], + [ + 'multiple double quoted paths', + '"C:\\img 1.png" "D:\\img 2.png"', + ['C:\\img 1.png', 'D:\\img 2.png'], + ], + ['unquoted path', 'C:\\img.png', ['C:\\img.png']], + [ + 'mixed quoted and unquoted', + '"C:\\img 1.png" D:\\img2.png', + ['C:\\img 1.png', 'D:\\img2.png'], + ], + ['single quoted path', "'C:\\my image.png'", ['C:\\my image.png']], + [ + 'mixed single and double quoted', + '"C:\\img 1.png" \'D:\\img 2.png\'', + ['C:\\img 1.png', 'D:\\img 2.png'], + ], + ])('should split %s', (_, input, expected) => { + expect([...splitDragAndDropPaths(input)]).toEqual(expected); + }); }); }); @@ -455,14 +476,14 @@ describe('clipboardUtils', () => { expect(result).toBe('@/path/to/file1.txt @/path/to/file2.txt '); }); - it('should only add @ prefix to valid paths', () => { + it('should return null if any path is invalid', () => { vi.mocked(existsSync).mockImplementation((p) => (p as string).endsWith('.txt'), ); vi.mocked(statSync).mockReturnValue(MOCK_FILE_STATS); const result = parsePastedPaths('/valid/file.txt /invalid/file.jpg'); - expect(result).toBe('@/valid/file.txt /invalid/file.jpg '); + expect(result).toBe(null); }); it('should return null if no paths are valid', () => { @@ -471,76 +492,110 @@ describe('clipboardUtils', () => { expect(result).toBe(null); }); - it('should handle paths with escaped spaces', () => { - const validPaths = new Set(['/path/to/my file.txt', '/other/path.txt']); - vi.mocked(existsSync).mockImplementation((p) => - validPaths.has(p as string), - ); - vi.mocked(statSync).mockReturnValue(MOCK_FILE_STATS); - - const result = parsePastedPaths('/path/to/my\\ file.txt /other/path.txt'); - expect(result).toBe('@/path/to/my\\ file.txt @/other/path.txt '); - }); - - it('should unescape paths before validation', () => { - const validPaths = new Set(['/my file.txt', '/other.txt']); - const validatedPaths: string[] = []; - vi.mocked(existsSync).mockImplementation((p) => { - validatedPaths.push(p as string); - return validPaths.has(p as string); + describe('in posix', () => { + beforeEach(() => { + mockPlatform('linux'); }); - vi.mocked(statSync).mockReturnValue(MOCK_FILE_STATS); - parsePastedPaths('/my\\ file.txt /other.txt'); - // First checks entire string, then individual unescaped segments - expect(validatedPaths).toEqual([ - '/my\\ file.txt /other.txt', - '/my file.txt', - '/other.txt', - ]); + it('should handle paths with escaped spaces', () => { + const validPaths = new Set(['/path/to/my file.txt', '/other/path.txt']); + vi.mocked(existsSync).mockImplementation((p) => + validPaths.has(p as string), + ); + vi.mocked(statSync).mockReturnValue(MOCK_FILE_STATS); + + const result = parsePastedPaths( + '/path/to/my\\ file.txt /other/path.txt', + ); + expect(result).toBe('@/path/to/my\\ file.txt @/other/path.txt '); + }); + + it('should unescape paths before validation', () => { + const validPaths = new Set(['/my file.txt', '/other.txt']); + const validatedPaths: string[] = []; + vi.mocked(existsSync).mockImplementation((p) => { + validatedPaths.push(p as string); + return validPaths.has(p as string); + }); + vi.mocked(statSync).mockReturnValue(MOCK_FILE_STATS); + + parsePastedPaths('/my\\ file.txt /other.txt'); + // First checks entire string, then individual unescaped segments + expect(validatedPaths).toEqual([ + '/my\\ file.txt /other.txt', + '/my file.txt', + '/other.txt', + ]); + }); + + it('should handle single path with unescaped spaces from copy-paste', () => { + vi.mocked(existsSync).mockReturnValue(true); + vi.mocked(statSync).mockReturnValue(MOCK_FILE_STATS); + + const result = parsePastedPaths('/path/to/my file.txt'); + expect(result).toBe('@/path/to/my\\ file.txt '); + }); + + it('should handle single-quoted with escaped quote', () => { + const validPaths = new Set([ + "/usr/test/my file with 'single quotes'.txt", + ]); + const validatedPaths: string[] = []; + vi.mocked(existsSync).mockImplementation((p) => { + validatedPaths.push(p as string); + return validPaths.has(p as string); + }); + vi.mocked(statSync).mockReturnValue(MOCK_FILE_STATS); + + const result = parsePastedPaths( + "'/usr/test/my file with '\\''single quotes'\\''.txt'", + ); + expect(result).toBe( + "@/usr/test/my\\ file\\ with\\ \\'single\\ quotes\\'.txt ", + ); + + expect(validatedPaths).toEqual([ + "/usr/test/my file with 'single quotes'.txt", + ]); + }); }); - it('should handle single path with unescaped spaces from copy-paste', () => { - vi.mocked(existsSync).mockReturnValue(true); - vi.mocked(statSync).mockReturnValue(MOCK_FILE_STATS); + describe('in windows', () => { + beforeEach(() => mockPlatform('win32')); - const result = parsePastedPaths('/path/to/my file.txt'); - expect(result).toBe('@/path/to/my\\ file.txt '); - }); + it('should handle Windows path', () => { + vi.mocked(existsSync).mockReturnValue(true); + vi.mocked(statSync).mockReturnValue(MOCK_FILE_STATS); - it('should handle Windows path', () => { - vi.mocked(existsSync).mockReturnValue(true); - vi.mocked(statSync).mockReturnValue(MOCK_FILE_STATS); + const result = parsePastedPaths('C:\\Users\\file.txt'); + expect(result).toBe('@C:\\Users\\file.txt '); + }); - const result = parsePastedPaths('C:\\Users\\file.txt'); - expect(result).toBe('@C:\\Users\\file.txt '); - }); + it('should handle Windows path with unescaped spaces', () => { + vi.mocked(existsSync).mockReturnValue(true); + vi.mocked(statSync).mockReturnValue(MOCK_FILE_STATS); - it('should handle Windows path with unescaped spaces', () => { - vi.mocked(existsSync).mockReturnValue(true); - vi.mocked(statSync).mockReturnValue(MOCK_FILE_STATS); + const result = parsePastedPaths('C:\\My Documents\\file.txt'); + expect(result).toBe('@"C:\\My Documents\\file.txt" '); + }); + it('should handle multiple Windows paths', () => { + const validPaths = new Set(['C:\\file1.txt', 'D:\\file2.txt']); + vi.mocked(existsSync).mockImplementation((p) => + validPaths.has(p as string), + ); + vi.mocked(statSync).mockReturnValue(MOCK_FILE_STATS); - const result = parsePastedPaths('C:\\My Documents\\file.txt'); - expect(result).toBe('@C:\\My\\ Documents\\file.txt '); - }); + const result = parsePastedPaths('C:\\file1.txt D:\\file2.txt'); + expect(result).toBe('@C:\\file1.txt @D:\\file2.txt '); + }); - it('should handle multiple Windows paths', () => { - const validPaths = new Set(['C:\\file1.txt', 'D:\\file2.txt']); - vi.mocked(existsSync).mockImplementation((p) => - validPaths.has(p as string), - ); - vi.mocked(statSync).mockReturnValue(MOCK_FILE_STATS); + it('should handle Windows UNC path', () => { + vi.mocked(existsSync).mockReturnValue(true); + vi.mocked(statSync).mockReturnValue(MOCK_FILE_STATS); - const result = parsePastedPaths('C:\\file1.txt D:\\file2.txt'); - expect(result).toBe('@C:\\file1.txt @D:\\file2.txt '); - }); - - it('should handle Windows UNC path', () => { - vi.mocked(existsSync).mockReturnValue(true); - vi.mocked(statSync).mockReturnValue(MOCK_FILE_STATS); - - const result = parsePastedPaths('\\\\server\\share\\file.txt'); - expect(result).toBe('@\\\\server\\share\\file.txt '); + const result = parsePastedPaths('\\\\server\\share\\file.txt'); + expect(result).toBe('@\\\\server\\share\\file.txt '); + }); }); }); }); diff --git a/packages/cli/src/ui/utils/clipboardUtils.ts b/packages/cli/src/ui/utils/clipboardUtils.ts index a6a7b485cd..fd46a2c749 100644 --- a/packages/cli/src/ui/utils/clipboardUtils.ts +++ b/packages/cli/src/ui/utils/clipboardUtils.ts @@ -11,7 +11,6 @@ import * as path from 'node:path'; import { debugLogger, spawnAsync, - unescapePath, escapePath, Storage, } from '@google/gemini-cli-core'; @@ -418,48 +417,77 @@ export async function cleanupOldClipboardImages( debugLogger.debug('Failed to clean up old clipboard images:', e); } } - /** - * Splits text into individual path segments, respecting escaped spaces. - * Unescaped spaces act as separators between paths, while "\ " is preserved - * as part of a filename. + * Splits a pasted text block up into escaped path segements if it's a legal + * drag-and-drop string. * - * Example: "/img1.png /path/my\ image.png" → ["/img1.png", "/path/my\ image.png"] + * There are multiple ways drag-and-drop paths might be escaped: + * - Bare (only if there are no special chars): /path/to/myfile.png + * - Wrapped in double quotes (Windows only): "/path/to/my file~!.png" + * - Escaped with backslashes (POSIX only): /path/to/my\ file~!.png + * - Wrapped in single quotes: '/path/to/my file~!.png' * - * @param text The text to split - * @returns Array of path segments (still escaped) + * When wrapped in single quotes, actual single quotes in the filename are + * escaped with "'\''". For example: '/path/to/my '\''fancy file'\''.png' + * + * When wrapped in double quotes, actual double quotes are not an issue becuase + * windows doesn't allow them in filenames. + * + * On all systems, a single drag-and-drop may include both wrapped and bare + * paths, so we need to handle both simultaneously. + * + * @param text + * @returns An iterable of escaped paths */ -export function splitEscapedPaths(text: string): string[] { - const paths: string[] = []; +export function* splitDragAndDropPaths(text: string): Generator { let current = ''; - let i = 0; + let mode: 'NORMAL' | 'DOUBLE' | 'SINGLE' = 'NORMAL'; + const isWindows = process.platform === 'win32'; + let i = 0; while (i < text.length) { const char = text[i]; - if (char === '\\' && i + 1 < text.length && text[i + 1] === ' ') { - // Escaped space - part of filename, preserve the escape sequence - current += '\\ '; - i += 2; - } else if (char === ' ') { - // Unescaped space - path separator - if (current.trim()) { - paths.push(current.trim()); + if (mode === 'NORMAL') { + if (char === ' ') { + if (current.length > 0) { + yield current; + current = ''; + } + } else if (char === '"') { + mode = 'DOUBLE'; + } else if (char === "'") { + mode = 'SINGLE'; + } else if (char === '\\' && !isWindows) { + // POSIX escape in normal mode + if (i + 1 < text.length) { + const next = text[i + 1]; + current += next; + i++; + } + } else { + current += char; + } + } else if (mode === 'DOUBLE') { + if (char === '"') { + mode = 'NORMAL'; + } else { + current += char; + } + } else if (mode === 'SINGLE') { + if (char === "'") { + mode = 'NORMAL'; + } else { + current += char; } - current = ''; - i++; - } else { - current += char; - i++; } + + i++; } - // Don't forget the last segment - if (current.trim()) { - paths.push(current.trim()); + if (current.length > 0) { + yield current; } - - return paths; } /** @@ -467,44 +495,35 @@ export function splitEscapedPaths(text: string): string[] { */ function isValidFilePath(p: string): boolean { try { - return existsSync(p) && statSync(p).isFile(); + return PATH_PREFIX_PATTERN.test(p) && existsSync(p) && statSync(p).isFile(); } catch { return false; } } /** - * Processes pasted text containing file paths, adding @ prefix to valid paths. - * Handles both single and multiple space-separated paths. + * Processes pasted text containing file paths (like those from drag and drop), + * adding @ prefix to valid paths and escaping them in a standard way. * - * @param text The pasted text (potentially space-separated paths) - * @returns Processed string with @ prefixes on valid paths, or null if no valid paths + * @param text The pasted text + * @returns Processed string with @ prefixes or null if any paths are invalid */ export function parsePastedPaths(text: string): string | null { // First, check if the entire text is a single valid path - if (PATH_PREFIX_PATTERN.test(text) && isValidFilePath(text)) { + if (isValidFilePath(text)) { return `@${escapePath(text)} `; } - // Otherwise, try splitting on unescaped spaces - const segments = splitEscapedPaths(text); - if (segments.length === 0) { + const validPaths = []; + for (const segment of splitDragAndDropPaths(text)) { + if (isValidFilePath(segment)) { + validPaths.push(`@${escapePath(segment)}`); + } else { + return null; // If any segment is invalid, return null for the whole string + } + } + if (validPaths.length === 0) { return null; } - - let anyValidPath = false; - const processedPaths = segments.map((segment) => { - // Quick rejection: skip segments that can't be paths - if (!PATH_PREFIX_PATTERN.test(segment)) { - return segment; - } - const unescaped = unescapePath(segment); - if (isValidFilePath(unescaped)) { - anyValidPath = true; - return `@${segment}`; - } - return segment; - }); - - return anyValidPath ? processedPaths.join(' ') + ' ' : null; + return validPaths.join(' ') + ' '; } diff --git a/packages/core/src/utils/paths.test.ts b/packages/core/src/utils/paths.test.ts index 64e4e94ddc..bfca3763e2 100644 --- a/packages/core/src/utils/paths.test.ts +++ b/packages/core/src/utils/paths.test.ts @@ -4,7 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import * as fs from 'node:fs'; import path from 'node:path'; import { pathToFileURL } from 'node:url'; @@ -24,131 +24,118 @@ vi.mock('node:fs', async (importOriginal) => { }; }); +const mockPlatform = (platform: string) => { + vi.stubGlobal( + 'process', + Object.create(process, { + platform: { + get: () => platform, + }, + }), + ); +}; + describe('escapePath', () => { - it.each([ - ['spaces', 'my file.txt', 'my\\ file.txt'], - ['tabs', 'file\twith\ttabs.txt', 'file\\\twith\\\ttabs.txt'], - ['parentheses', 'file(1).txt', 'file\\(1\\).txt'], - ['square brackets', 'file[backup].txt', 'file\\[backup\\].txt'], - ['curly braces', 'file{temp}.txt', 'file\\{temp\\}.txt'], - ['semicolons', 'file;name.txt', 'file\\;name.txt'], - ['ampersands', 'file&name.txt', 'file\\&name.txt'], - ['pipes', 'file|name.txt', 'file\\|name.txt'], - ['asterisks', 'file*.txt', 'file\\*.txt'], - ['question marks', 'file?.txt', 'file\\?.txt'], - ['dollar signs', 'file$name.txt', 'file\\$name.txt'], - ['backticks', 'file`name.txt', 'file\\`name.txt'], - ['single quotes', "file'name.txt", "file\\'name.txt"], - ['double quotes', 'file"name.txt', 'file\\"name.txt'], - ['hash symbols', 'file#name.txt', 'file\\#name.txt'], - ['exclamation marks', 'file!name.txt', 'file\\!name.txt'], - [ - 'tildes', - 'file~name.txt', - process.platform === 'win32' ? 'file~name.txt' : 'file\\~name.txt', - ], - [ - 'less than and greater than signs', - 'file.txt', - 'file\\.txt', - ], - ])('should escape %s', (_, input, expected) => { - expect(escapePath(input)).toBe(expected); + afterEach(() => vi.unstubAllGlobals()); + + describe('in posix', () => { + beforeEach(() => mockPlatform('linux')); + + it.each([ + ['spaces', 'my file.txt', 'my\\ file.txt'], + ['tabs', 'file\twith\ttabs.txt', 'file\\\twith\\\ttabs.txt'], + ['parentheses', 'file(1).txt', 'file\\(1\\).txt'], + ['square brackets', 'file[backup].txt', 'file\\[backup\\].txt'], + ['curly braces', 'file{temp}.txt', 'file\\{temp\\}.txt'], + ['semicolons', 'file;name.txt', 'file\\;name.txt'], + ['ampersands', 'file&name.txt', 'file\\&name.txt'], + ['pipes', 'file|name.txt', 'file\\|name.txt'], + ['asterisks', 'file*.txt', 'file\\*.txt'], + ['question marks', 'file?.txt', 'file\\?.txt'], + ['dollar signs', 'file$name.txt', 'file\\$name.txt'], + ['backticks', 'file`name.txt', 'file\\`name.txt'], + ['single quotes', "file'name.txt", "file\\'name.txt"], + ['double quotes', 'file"name.txt', 'file\\"name.txt'], + ['hash symbols', 'file#name.txt', 'file\\#name.txt'], + ['exclamation marks', 'file!name.txt', 'file\\!name.txt'], + ['tildes', 'file~name.txt', 'file\\~name.txt'], + [ + 'less than and greater than signs', + 'file.txt', + 'file\\.txt', + ], + [ + 'multiple special characters', + 'my file (backup) [v1.2].txt', + 'my\\ file\\ \\(backup\\)\\ \\[v1.2\\].txt', + ], + ['normal file', 'normalfile.txt', 'normalfile.txt'], + ['normal path', 'path/to/normalfile.txt', 'path/to/normalfile.txt'], + [ + 'real world example 1', + 'My Documents/Project (2024)/file [backup].txt', + 'My\\ Documents/Project\\ \\(2024\\)/file\\ \\[backup\\].txt', + ], + [ + 'real world example 2', + 'file with $special &chars!.txt', + 'file\\ with\\ \\$special\\ \\&chars\\!.txt', + ], + ['empty string', '', ''], + [ + 'all special chars', + ' ()[]{};&|*?$`\'"#!<>', + '\\ \\(\\)\\[\\]\\{\\}\\;\\&\\|\\*\\?\\$\\`\\\'\\"\\#\\!\\<\\>', + ], + ])('should escape %s', (_, input, expected) => { + expect(escapePath(input)).toBe(expected); + }); }); - it('should handle multiple special characters', () => { - expect(escapePath('my file (backup) [v1.2].txt')).toBe( - 'my\\ file\\ \\(backup\\)\\ \\[v1.2\\].txt', - ); - }); + describe('in windows', () => { + beforeEach(() => mockPlatform('win32')); - it('should not double-escape already escaped characters', () => { - expect(escapePath('my\\ file.txt')).toBe('my\\ file.txt'); - expect(escapePath('file\\(name\\).txt')).toBe('file\\(name\\).txt'); - }); - - it('should handle escaped backslashes correctly', () => { - // Double backslash (escaped backslash) followed by space should escape the space - expect(escapePath('path\\\\ file.txt')).toBe('path\\\\\\ file.txt'); - // Triple backslash (escaped backslash + escaping backslash) followed by space should not double-escape - expect(escapePath('path\\\\\\ file.txt')).toBe('path\\\\\\ file.txt'); - // Quadruple backslash (two escaped backslashes) followed by space should escape the space - expect(escapePath('path\\\\\\\\ file.txt')).toBe('path\\\\\\\\\\ file.txt'); - }); - - it('should handle complex escaped backslash scenarios', () => { - // Escaped backslash before special character that needs escaping - expect(escapePath('file\\\\(test).txt')).toBe('file\\\\\\(test\\).txt'); - // Multiple escaped backslashes - expect(escapePath('path\\\\\\\\with space.txt')).toBe( - 'path\\\\\\\\with\\ space.txt', - ); - }); - - it('should handle paths without special characters', () => { - expect(escapePath('normalfile.txt')).toBe('normalfile.txt'); - expect(escapePath('path/to/normalfile.txt')).toBe('path/to/normalfile.txt'); - }); - - it('should handle complex real-world examples', () => { - expect(escapePath('My Documents/Project (2024)/file [backup].txt')).toBe( - 'My\\ Documents/Project\\ \\(2024\\)/file\\ \\[backup\\].txt', - ); - expect(escapePath('file with $special &chars!.txt')).toBe( - 'file\\ with\\ \\$special\\ \\&chars\\!.txt', - ); - }); - - it('should handle empty strings', () => { - expect(escapePath('')).toBe(''); - }); - - it('should handle paths with multiple special characters', () => { - expect(escapePath(' ()[]{};&|*?$`\'"#!<>')).toBe( - '\\ \\(\\)\\[\\]\\{\\}\\;\\&\\|\\*\\?\\$\\`\\\'\\"\\#\\!\\<\\>', - ); - }); - - it('should handle tildes based on platform', () => { - const expected = process.platform === 'win32' ? '~' : '\\~'; - expect(escapePath('~')).toBe(expected); + it.each([ + [ + 'spaces', + 'C:\\path with spaces\\file.txt', + '"C:\\path with spaces\\file.txt"', + ], + ['parentheses', 'file(1).txt', '"file(1).txt"'], + ['special chars', 'file&name.txt', '"file&name.txt"'], + ['caret', 'file^name.txt', '"file^name.txt"'], + ['normal path', 'C:\\path\\to\\file.txt', 'C:\\path\\to\\file.txt'], + ])('should escape %s', (_, input, expected) => { + expect(escapePath(input)).toBe(expected); + }); }); }); describe('unescapePath', () => { - it.each([ - ['spaces', 'my\\ file.txt', 'my file.txt'], - ['tabs', 'file\\\twith\\\ttabs.txt', 'file\twith\ttabs.txt'], - ['parentheses', 'file\\(1\\).txt', 'file(1).txt'], - ['square brackets', 'file\\[backup\\].txt', 'file[backup].txt'], - ['curly braces', 'file\\{temp\\}.txt', 'file{temp}.txt'], - ])('should unescape %s', (_, input, expected) => { - expect(unescapePath(input)).toBe(expected); - }); + afterEach(() => vi.unstubAllGlobals()); - it('should unescape multiple special characters', () => { - expect(unescapePath('my\\ file\\ \\(backup\\)\\ \\[v1.2\\].txt')).toBe( - 'my file (backup) [v1.2].txt', - ); - }); + describe('in posix', () => { + beforeEach(() => mockPlatform('linux')); - it('should handle paths without escaped characters', () => { - expect(unescapePath('normalfile.txt')).toBe('normalfile.txt'); - expect(unescapePath('path/to/normalfile.txt')).toBe( - 'path/to/normalfile.txt', - ); - }); + it.each([ + ['spaces', 'my\\ file.txt', 'my file.txt'], + ['tabs', 'file\\\twith\\\ttabs.txt', 'file\twith\ttabs.txt'], + ['parentheses', 'file\\(1\\).txt', 'file(1).txt'], + ['square brackets', 'file\\[backup\\].txt', 'file[backup].txt'], + ['curly braces', 'file\\{temp\\}.txt', 'file{temp}.txt'], + [ + 'multiple special characters', + 'my\\ file\\ \\(backup\\)\\ \\[v1.2\\].txt', + 'my file (backup) [v1.2].txt', + ], + ['normal file', 'normalfile.txt', 'normalfile.txt'], + ['normal path', 'path/to/normalfile.txt', 'path/to/normalfile.txt'], + ['empty string', '', ''], + ])('should unescape %s', (_, input, expected) => { + expect(unescapePath(input)).toBe(expected); + }); - it('should handle all special characters but tilda', () => { - expect( - unescapePath( - '\\ \\(\\)\\[\\]\\{\\}\\;\\&\\|\\*\\?\\$\\`\\\'\\"\\#\\!\\<\\>', - ), - ).toBe(' ()[]{};&|*?$`\'"#!<>'); - }); - - it('should be the inverse of escapePath', () => { - const testCases = [ + it.each([ 'my file.txt', 'file(1).txt', 'file[backup].txt', @@ -156,29 +143,35 @@ describe('unescapePath', () => { 'file with $special &chars!.txt', ' ()[]{};&|*?$`\'"#!~<>', 'file\twith\ttabs.txt', - ]; - - testCases.forEach((testCase) => { - expect(unescapePath(escapePath(testCase))).toBe(testCase); + ])('should unescape escaped %s', (input) => { + expect(unescapePath(escapePath(input))).toBe(input); }); }); - it('should handle empty strings', () => { - expect(unescapePath('')).toBe(''); - }); + describe('in windows', () => { + beforeEach(() => mockPlatform('win32')); - it('should not affect backslashes not followed by special characters', () => { - expect(unescapePath('file\\name.txt')).toBe('file\\name.txt'); - expect(unescapePath('path\\to\\file.txt')).toBe('path\\to\\file.txt'); - }); + it.each([ + [ + 'quoted path', + '"C:\\path with spaces\\file.txt"', + 'C:\\path with spaces\\file.txt', + ], + ['unquoted path', 'C:\\path\\to\\file.txt', 'C:\\path\\to\\file.txt'], + ['partially quoted', '"C:\\path', '"C:\\path'], + ['empty string', '', ''], + ])('should unescape %s', (_, input, expected) => { + expect(unescapePath(input)).toBe(expected); + }); - it('should handle escaped backslashes in unescaping', () => { - // Should correctly unescape when there are escaped backslashes - expect(unescapePath('path\\\\\\ file.txt')).toBe('path\\\\ file.txt'); - expect(unescapePath('path\\\\\\\\\\ file.txt')).toBe( - 'path\\\\\\\\ file.txt', - ); - expect(unescapePath('file\\\\\\(test\\).txt')).toBe('file\\\\(test).txt'); + it.each([ + 'C:\\path\\to\\file.txt', + 'C:\\path with spaces\\file.txt', + 'file(1).txt', + 'file&name.txt', + ])('should unescape escaped %s', (input) => { + expect(unescapePath(escapePath(input))).toBe(input); + }); }); }); @@ -222,19 +215,9 @@ describe('isSubpath', () => { }); describe('isSubpath on Windows', () => { - const originalPlatform = process.platform; + afterEach(() => vi.unstubAllGlobals()); - beforeAll(() => { - Object.defineProperty(process, 'platform', { - value: 'win32', - }); - }); - - afterAll(() => { - Object.defineProperty(process, 'platform', { - value: originalPlatform, - }); - }); + beforeEach(() => mockPlatform('win32')); it('should return true for a direct subpath on Windows', () => { expect(isSubpath('C:\\Users\\Test', 'C:\\Users\\Test\\file.txt')).toBe( diff --git a/packages/core/src/utils/paths.ts b/packages/core/src/utils/paths.ts index e2b6a72b64..6c3236606d 100644 --- a/packages/core/src/utils/paths.ts +++ b/packages/core/src/utils/paths.ts @@ -6,7 +6,6 @@ import path from 'node:path'; import os from 'node:os'; -import process from 'node:process'; import * as crypto from 'node:crypto'; import * as fs from 'node:fs'; import { fileURLToPath } from 'node:url'; @@ -14,15 +13,6 @@ import { fileURLToPath } from 'node:url'; export const GEMINI_DIR = '.gemini'; export const GOOGLE_ACCOUNTS_FILENAME = 'google_accounts.json'; -/** - * Special characters that need to be escaped in file paths for shell compatibility. - * Note that windows doesn't escape tilda. - */ -export const SHELL_SPECIAL_CHARS = - process.platform === 'win32' - ? /[ \t()[\]{};|*?$`'"#&<>!]/ - : /[ \t()[\]{};|*?$`'"#&<>!~]/; - /** * Returns the home directory. * If GEMINI_CLI_HOME environment variable is set, it returns its value. @@ -280,43 +270,43 @@ export function makeRelative( } /** - * Escapes special characters in a file path like macOS terminal does. - * Escapes: spaces, parentheses, brackets, braces, semicolons, ampersands, pipes, - * asterisks, question marks, dollar signs, backticks, quotes, hash, and other shell metacharacters. + * Escape paths for at-commands. + * + * - Windows: double quoted if they contain special chars, otherwise bare + * - POSIX: backslash-escaped */ export function escapePath(filePath: string): string { - let result = ''; - for (let i = 0; i < filePath.length; i++) { - const char = filePath[i]; - - // Count consecutive backslashes before this character - let backslashCount = 0; - for (let j = i - 1; j >= 0 && filePath[j] === '\\'; j--) { - backslashCount++; - } - - // Character is already escaped if there's an odd number of backslashes before it - const isAlreadyEscaped = backslashCount % 2 === 1; - - // Only escape if not already escaped - if (!isAlreadyEscaped && SHELL_SPECIAL_CHARS.test(char)) { - result += '\\' + char; - } else { - result += char; + if (process.platform === 'win32') { + // Windows: Double quote if it contains space or special chars + if (/[\s()[\]{};|&^$!@%`'~]/.test(filePath)) { + return `"${filePath}"`; } + return filePath; + } else { + // POSIX: Backslash escape + return filePath.replace(/([ \t()[\]{};|*?$`'"#&<>!~\\])/g, '\\$1'); } - return result; } /** - * Unescapes special characters in a file path. - * Removes backslash escaping from shell metacharacters. + * Unescapes paths for at-commands. + * + * - Windows: double quoted if they contain special chars, otherwise bare + * - POSIX: backslash-escaped */ export function unescapePath(filePath: string): string { - return filePath.replace( - new RegExp(`\\\\([${SHELL_SPECIAL_CHARS.source.slice(1, -1)}])`, 'g'), - '$1', - ); + if (process.platform === 'win32') { + if ( + filePath.length >= 2 && + filePath.startsWith('"') && + filePath.endsWith('"') + ) { + return filePath.slice(1, -1); + } + return filePath; + } else { + return filePath.replace(/\\(.)/g, '$1'); + } } /** @@ -345,7 +335,7 @@ export function normalizePath(p: string): string { * @returns True if childPath is a subpath of parentPath, false otherwise. */ export function isSubpath(parentPath: string, childPath: string): boolean { - const isWindows = os.platform() === 'win32'; + const isWindows = process.platform === 'win32'; const pathModule = isWindows ? path.win32 : path; // On Windows, path.relative is case-insensitive. On POSIX, it's case-sensitive.