Skip to content

Commit 6e031c2

Browse files
committed
feat: add error handling for decodeURIComponent and tests
- Added try-catch block around decodeURIComponent to handle invalid escape sequences - Falls back to original path if decoding fails - Added comprehensive unit tests for the openFile function - Tests cover invalid URI encoding, valid encoding, and various edge cases
1 parent afdff30 commit 6e031c2

File tree

2 files changed

+236
-2
lines changed

2 files changed

+236
-2
lines changed
Lines changed: 225 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,225 @@
1+
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"
2+
import * as vscode from "vscode"
3+
import * as path from "path"
4+
import * as os from "os"
5+
import { openFile } from "../open-file"
6+
7+
// Mock vscode module
8+
vi.mock("vscode", () => ({
9+
Uri: {
10+
file: vi.fn((path: string) => ({ fsPath: path })),
11+
},
12+
workspace: {
13+
fs: {
14+
stat: vi.fn(),
15+
writeFile: vi.fn(),
16+
},
17+
openTextDocument: vi.fn(),
18+
},
19+
window: {
20+
showTextDocument: vi.fn(),
21+
showErrorMessage: vi.fn(),
22+
tabGroups: {
23+
all: [],
24+
},
25+
activeTextEditor: undefined,
26+
},
27+
commands: {
28+
executeCommand: vi.fn(),
29+
},
30+
FileType: {
31+
Directory: 2,
32+
File: 1,
33+
},
34+
Selection: vi.fn((startLine: number, startChar: number, endLine: number, endChar: number) => ({
35+
start: { line: startLine, character: startChar },
36+
end: { line: endLine, character: endChar },
37+
})),
38+
TabInputText: vi.fn(),
39+
}))
40+
41+
// Mock utils
42+
vi.mock("../../utils/path", () => ({
43+
arePathsEqual: vi.fn((a: string, b: string) => a === b),
44+
getWorkspacePath: vi.fn(() => "/Users/roocode"),
45+
}))
46+
47+
// Mock i18n
48+
vi.mock("../../i18n", () => ({
49+
t: vi.fn((key: string, params?: any) => {
50+
// Return the key without namespace prefix to match actual behavior
51+
if (key.startsWith("common:")) {
52+
return key.replace("common:", "")
53+
}
54+
return key
55+
}),
56+
}))
57+
58+
describe("openFile", () => {
59+
beforeEach(() => {
60+
vi.clearAllMocks()
61+
vi.spyOn(console, "warn").mockImplementation(() => {})
62+
})
63+
64+
afterEach(() => {
65+
vi.restoreAllMocks()
66+
})
67+
68+
describe("decodeURIComponent error handling", () => {
69+
it("should handle invalid URI encoding gracefully", async () => {
70+
const invalidPath = "test%ZZinvalid.txt" // Invalid percent encoding
71+
const mockDocument = { uri: { fsPath: invalidPath } }
72+
73+
vi.mocked(vscode.workspace.fs.stat).mockResolvedValue({
74+
type: vscode.FileType.File,
75+
ctime: 0,
76+
mtime: 0,
77+
size: 0,
78+
})
79+
vi.mocked(vscode.workspace.openTextDocument).mockResolvedValue(mockDocument as any)
80+
vi.mocked(vscode.window.showTextDocument).mockResolvedValue({} as any)
81+
82+
await openFile(invalidPath)
83+
84+
// Should log a warning about decode failure
85+
expect(console.warn).toHaveBeenCalledWith(
86+
"Failed to decode file path: URIError: URI malformed. Using original path.",
87+
)
88+
89+
// Should still attempt to open the file with the original path
90+
expect(vscode.workspace.openTextDocument).toHaveBeenCalled()
91+
expect(vscode.window.showErrorMessage).not.toHaveBeenCalled()
92+
})
93+
94+
it("should successfully decode valid URI-encoded paths", async () => {
95+
const encodedPath = "./src/%5Btest%5D/file.txt" // [test] encoded
96+
const decodedPath = "./src/[test]/file.txt"
97+
const mockDocument = { uri: { fsPath: decodedPath } }
98+
99+
vi.mocked(vscode.workspace.fs.stat).mockResolvedValue({
100+
type: vscode.FileType.File,
101+
ctime: 0,
102+
mtime: 0,
103+
size: 0,
104+
})
105+
vi.mocked(vscode.workspace.openTextDocument).mockResolvedValue(mockDocument as any)
106+
vi.mocked(vscode.window.showTextDocument).mockResolvedValue({} as any)
107+
108+
await openFile(encodedPath)
109+
110+
// Should not log any warnings
111+
expect(console.warn).not.toHaveBeenCalled()
112+
113+
// Should use the decoded path
114+
const expectedPath = path.join("/Users/roocode", "src/[test]/file.txt")
115+
expect(vscode.Uri.file).toHaveBeenCalledWith(expectedPath)
116+
expect(vscode.workspace.openTextDocument).toHaveBeenCalled()
117+
expect(vscode.window.showErrorMessage).not.toHaveBeenCalled()
118+
})
119+
120+
it("should handle paths with special characters that need encoding", async () => {
121+
const pathWithSpecialChars = "./src/[brackets]/file with spaces.txt"
122+
const mockDocument = { uri: { fsPath: pathWithSpecialChars } }
123+
124+
vi.mocked(vscode.workspace.fs.stat).mockResolvedValue({
125+
type: vscode.FileType.File,
126+
ctime: 0,
127+
mtime: 0,
128+
size: 0,
129+
})
130+
vi.mocked(vscode.workspace.openTextDocument).mockResolvedValue(mockDocument as any)
131+
vi.mocked(vscode.window.showTextDocument).mockResolvedValue({} as any)
132+
133+
await openFile(pathWithSpecialChars)
134+
135+
// Should work without errors
136+
expect(console.warn).not.toHaveBeenCalled()
137+
expect(vscode.workspace.openTextDocument).toHaveBeenCalled()
138+
expect(vscode.window.showErrorMessage).not.toHaveBeenCalled()
139+
})
140+
141+
it("should handle already decoded paths without double-decoding", async () => {
142+
const normalPath = "./src/normal/file.txt"
143+
const mockDocument = { uri: { fsPath: normalPath } }
144+
145+
vi.mocked(vscode.workspace.fs.stat).mockResolvedValue({
146+
type: vscode.FileType.File,
147+
ctime: 0,
148+
mtime: 0,
149+
size: 0,
150+
})
151+
vi.mocked(vscode.workspace.openTextDocument).mockResolvedValue(mockDocument as any)
152+
vi.mocked(vscode.window.showTextDocument).mockResolvedValue({} as any)
153+
154+
await openFile(normalPath)
155+
156+
// Should work without errors
157+
expect(console.warn).not.toHaveBeenCalled()
158+
expect(vscode.workspace.openTextDocument).toHaveBeenCalled()
159+
expect(vscode.window.showErrorMessage).not.toHaveBeenCalled()
160+
})
161+
})
162+
163+
describe("error handling", () => {
164+
it("should show error message when file does not exist", async () => {
165+
const nonExistentPath = "./does/not/exist.txt"
166+
167+
vi.mocked(vscode.workspace.fs.stat).mockRejectedValue(new Error("File not found"))
168+
169+
await openFile(nonExistentPath)
170+
171+
expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("errors.could_not_open_file")
172+
})
173+
174+
it("should handle generic errors", async () => {
175+
const testPath = "./test.txt"
176+
177+
vi.mocked(vscode.workspace.fs.stat).mockRejectedValue("Not an Error object")
178+
179+
await openFile(testPath)
180+
181+
expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("errors.could_not_open_file")
182+
})
183+
})
184+
185+
describe("directory handling", () => {
186+
it("should reveal directories in explorer", async () => {
187+
const dirPath = "./src/components"
188+
189+
vi.mocked(vscode.workspace.fs.stat).mockResolvedValue({
190+
type: vscode.FileType.Directory,
191+
ctime: 0,
192+
mtime: 0,
193+
size: 0,
194+
})
195+
196+
await openFile(dirPath)
197+
198+
expect(vscode.commands.executeCommand).toHaveBeenCalledWith(
199+
"revealInExplorer",
200+
expect.objectContaining({ fsPath: expect.stringContaining("components") }),
201+
)
202+
expect(vscode.commands.executeCommand).toHaveBeenCalledWith("list.expand")
203+
expect(vscode.workspace.openTextDocument).not.toHaveBeenCalled()
204+
})
205+
})
206+
207+
describe("file creation", () => {
208+
it("should create new files when create option is true", async () => {
209+
const newFilePath = "./new/file.txt"
210+
const content = "Hello, world!"
211+
212+
vi.mocked(vscode.workspace.fs.stat).mockRejectedValue(new Error("File not found"))
213+
vi.mocked(vscode.workspace.openTextDocument).mockResolvedValue({} as any)
214+
vi.mocked(vscode.window.showTextDocument).mockResolvedValue({} as any)
215+
216+
await openFile(newFilePath, { create: true, content })
217+
218+
expect(vscode.workspace.fs.writeFile).toHaveBeenCalledWith(
219+
expect.objectContaining({ fsPath: expect.stringContaining("new/file.txt") }),
220+
Buffer.from(content, "utf8"),
221+
)
222+
expect(vscode.workspace.openTextDocument).toHaveBeenCalled()
223+
})
224+
})
225+
})

src/integrations/misc/open-file.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,19 @@ interface OpenFileOptions {
1212

1313
export async function openFile(filePath: string, options: OpenFileOptions = {}) {
1414
try {
15-
filePath = decodeURIComponent(filePath)
15+
// Store the original path for error messages before any modifications
16+
const originalFilePathForError = filePath
17+
18+
// Try to decode the URI component, but if it fails, use the original path
19+
try {
20+
filePath = decodeURIComponent(filePath)
21+
} catch (decodeError) {
22+
// If decoding fails (e.g., invalid escape sequences), continue with the original path
23+
console.warn(`Failed to decode file path: ${decodeError}. Using original path.`)
24+
}
25+
1626
const workspaceRoot = getWorkspacePath()
1727
const homeDir = os.homedir()
18-
const originalFilePathForError = filePath // Keep original for error messages
1928

2029
const attemptPaths: string[] = []
2130

0 commit comments

Comments
 (0)