Fix drag and drop escaping (#18965)

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 ');
+      });
     });
   });
 });

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<string> {
   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(' ') + ' ';
 }