Skip to content

Commit 1c10827

Browse files
tomaioocyfung1031
andauthored
♻️ Path joining logic creates double slashes (#1432)
* refactor(filesystem): path joining logic creates double slashes In packages/filesystem/utils.ts, the joinPath function adds a leading '/' to every path segment that doesn't already start with '/', which creates incorrect paths with double slashes like '//path1//path2' instead of '/path1/path2'. The logic incorrectly prepends '/' to values that already have it stripped. Signed-off-by: tomaioo <203048277+tomaioo@users.noreply.github.com> * refactor(filesystem): add unit tests and change of joinPath implementation * add additional tests for double slashes to providers --------- Signed-off-by: tomaioo <203048277+tomaioo@users.noreply.github.com> Co-authored-by: cyfung1031 <44498510+cyfung1031@users.noreply.github.com>
1 parent ee5b80d commit 1c10827

8 files changed

Lines changed: 185 additions & 10 deletions

File tree

packages/filesystem/baidu/baidu.test.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,4 +32,24 @@ describe("BaiduFileSystem", () => {
3232
);
3333
expect(updateDynamicRulesMock).not.toHaveBeenCalled();
3434
});
35+
36+
it("create should normalize double slashes in paths", async () => {
37+
const fs = new BaiduFileSystem("/apps//ScriptCat", "token");
38+
39+
const writer = await fs.create("dir//file.user.js");
40+
41+
expect((writer as any).path).toBe("/apps/ScriptCat/dir/file.user.js");
42+
});
43+
44+
it("delete should normalize double slashes in filelist payload", async () => {
45+
const fs = new BaiduFileSystem("/apps//ScriptCat", "token");
46+
const request = vi.spyOn(fs, "request").mockResolvedValue({ errno: 0 });
47+
48+
await fs.delete("dir//file.user.js");
49+
50+
const [, config] = request.mock.calls[0];
51+
expect((config as RequestInit).body).toBe(
52+
`async=0&filelist=${encodeURIComponent(JSON.stringify(["/apps/ScriptCat/dir/file.user.js"]))}`
53+
);
54+
});
3555
});

packages/filesystem/dropbox/dropbox.test.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,4 +30,29 @@ describe("DropboxFileSystem", () => {
3030

3131
await expect(fs.exists("/test.txt")).rejects.toThrow("invalid_access_token");
3232
});
33+
34+
it("create should normalize double slashes after the Dropbox app root", async () => {
35+
const fs = new DropboxFileSystem("/ScriptCat//sync", "token");
36+
37+
const writer = await fs.create("dir//file.user.js");
38+
39+
expect((writer as any).path).toBe("/sync/dir/file.user.js");
40+
});
41+
42+
it("delete should normalize double slashes after the Dropbox app root", async () => {
43+
const fs = new DropboxFileSystem("/ScriptCat//sync", "token");
44+
const request = vi.spyOn(fs, "request").mockResolvedValue({});
45+
46+
await fs.delete("dir//file.user.js");
47+
48+
expect(request).toHaveBeenCalledWith(
49+
"https://api.dropboxapi.com/2/files/delete_v2",
50+
expect.objectContaining({
51+
method: "POST",
52+
body: JSON.stringify({
53+
path: "/sync/dir/file.user.js",
54+
}),
55+
})
56+
);
57+
});
3358
});

packages/filesystem/googledrive/googledrive.test.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
22
import { LocalStorageDAO } from "@App/app/repo/localStorage";
33
import { FileSystemError, isAuthError, isConflictError, isNotFoundError, isRateLimitError } from "../error";
4+
import { joinPath } from "../utils";
45
import GoogleDriveFileSystem from "./googledrive";
56

67
function createMockResponse(options: { ok?: boolean; status?: number; text?: string; json?: any }): Response {
@@ -82,6 +83,21 @@ describe("GoogleDriveFileSystem", () => {
8283
expect(requestSpy).toHaveBeenCalledTimes(1);
8384
});
8485

86+
it("create should normalize double slashes in paths", async () => {
87+
const fs = new GoogleDriveFileSystem("/ScriptCat//sync", "token");
88+
89+
const writer = await fs.create("dir//file.user.js");
90+
91+
expect((writer as any).path).toBe("/ScriptCat/sync/dir/file.user.js");
92+
});
93+
94+
it("clearPathCache should accept normalized paths derived from duplicate slashes", () => {
95+
const fs = new GoogleDriveFileSystem("/ScriptCat//sync", "token");
96+
97+
expect(joinPath("/ScriptCat//sync", "dir//file.user.js")).toBe("/ScriptCat/sync/dir/file.user.js");
98+
expect(() => fs.clearPathCache("/ScriptCat//sync/dir")).not.toThrow();
99+
});
100+
85101
it("writer should clear stale path cache and retry once on provider 404", async () => {
86102
const fs = new GoogleDriveFileSystem("/", "token");
87103
const notFoundError = new FileSystemError({

packages/filesystem/onedrive/onedrive.test.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,27 @@ describe("OneDriveFileSystem", () => {
9595
await expect(fs.delete("missing.txt")).resolves.toBeUndefined();
9696
});
9797

98+
it("create should normalize double slashes in paths", async () => {
99+
const fs = new OneDriveFileSystem("/ScriptCat//sync", "token");
100+
101+
const writer = await fs.create("dir//file.user.js");
102+
103+
expect((writer as any).path).toBe("/ScriptCat/sync/dir/file.user.js");
104+
});
105+
106+
it("delete should normalize double slashes in URL paths", async () => {
107+
const fs = new OneDriveFileSystem("/ScriptCat//sync", "token");
108+
const request = vi.spyOn(fs, "request").mockResolvedValue({ status: 204 });
109+
110+
await fs.delete("dir//file.user.js");
111+
112+
expect(request).toHaveBeenCalledWith(
113+
"https://graph.microsoft.com/v1.0/me/drive/special/approot:/ScriptCat/sync/dir/file.user.js",
114+
{ method: "DELETE" },
115+
true
116+
);
117+
});
118+
98119
it("createDir should create nested directories from root", async () => {
99120
const fs = new OneDriveFileSystem("/", "token");
100121
const requestSpy = vi.spyOn(fs, "request").mockResolvedValue({});

packages/filesystem/s3/s3.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,14 @@ describe("S3FileSystem", () => {
202202
})
203203
);
204204
});
205+
206+
it("normalizes double slashes in object keys", async () => {
207+
const subFs = new S3FileSystem("test-bucket", mockClient, "/ScriptCat//sync");
208+
209+
const writer = await subFs.create("dir//file.user.js");
210+
211+
expect((writer as any).key).toBe("ScriptCat/sync/dir/file.user.js");
212+
});
205213
});
206214

207215
// ---- createDir ----
@@ -235,6 +243,15 @@ describe("S3FileSystem", () => {
235243

236244
await expect(fs.delete("test.txt")).rejects.toThrow();
237245
});
246+
247+
it("normalizes double slashes in object keys", async () => {
248+
const subFs = new S3FileSystem("test-bucket", mockClient, "/ScriptCat//sync");
249+
(mockClient.request as ReturnType<typeof vi.fn>).mockResolvedValue(createMockResponse({ ok: true, status: 204 }));
250+
251+
await subFs.delete("dir//file.user.js");
252+
253+
expect(mockClient.request).toHaveBeenCalledWith("DELETE", "test-bucket", "ScriptCat/sync/dir/file.user.js");
254+
});
238255
});
239256

240257
// ---- list ----

packages/filesystem/utils.test.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
import { describe, expect, it } from "vitest";
2+
import { joinPath } from "./utils";
3+
4+
describe("joinPath", () => {
5+
it("joins relative path segments as an absolute normalized path", () => {
6+
expect(joinPath("path1", "path2")).toBe("/path1/path2");
7+
});
8+
9+
it("does not create duplicate slashes when segments already contain slashes", () => {
10+
expect(joinPath("/path1", "/path2")).toBe("/path1/path2");
11+
expect(joinPath("/path1/", "/path2/")).toBe("/path1/path2");
12+
expect(joinPath("path1/", "path2/")).toBe("/path1/path2");
13+
expect(joinPath("/path1/", "path2")).toBe("/path1/path2");
14+
});
15+
16+
it("keeps root-relative behavior when the first segment is empty", () => {
17+
expect(joinPath("", "file.txt")).toBe("/file.txt");
18+
expect(joinPath("", "dir", "file.txt")).toBe("/dir/file.txt");
19+
});
20+
21+
it("handles root path segments", () => {
22+
expect(joinPath("/", "file.txt")).toBe("/file.txt");
23+
expect(joinPath("/", "dir", "file.txt")).toBe("/dir/file.txt");
24+
});
25+
26+
it("skips empty path segments", () => {
27+
expect(joinPath("dir", "", "file.txt")).toBe("/dir/file.txt");
28+
expect(joinPath("", "dir", "", "file.txt", "")).toBe("/dir/file.txt");
29+
});
30+
31+
it("returns empty string when no meaningful path is provided", () => {
32+
expect(joinPath()).toBe("");
33+
expect(joinPath("")).toBe("");
34+
expect(joinPath("", "")).toBe("");
35+
expect(joinPath("/")).toBe("");
36+
expect(joinPath("/", "")).toBe("");
37+
});
38+
39+
it("normalizes multiple adjacent slashes inside segments", () => {
40+
expect(joinPath("//path1//", "//path2//")).toBe("/path1/path2");
41+
expect(joinPath("path1//nested", "path2")).toBe("/path1/nested/path2");
42+
});
43+
});

packages/filesystem/utils.ts

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,25 @@
11
export function joinPath(...paths: string[]): string {
2-
let path = "";
3-
for (let value of paths) {
4-
if (!value) {
2+
let result = "";
3+
4+
for (const path of paths) {
5+
if (!path) {
56
continue;
67
}
7-
if (!value.startsWith("/")) {
8-
value = `/${value}`;
8+
9+
let start = 0;
10+
11+
for (let i = 0; i <= path.length; i++) {
12+
if (i !== path.length && path[i] !== "/") {
13+
continue;
14+
}
15+
16+
if (i > start) {
17+
result += `/${path.slice(start, i)}`;
18+
}
19+
20+
start = i + 1;
921
}
10-
if (value.endsWith("/")) {
11-
value = value.substring(0, value.length - 1);
12-
}
13-
path += value;
1422
}
15-
return path;
23+
24+
return result;
1625
}

packages/filesystem/webdav/webdav.test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,17 @@ describe("WebDAVFileSystem", () => {
186186
expect(mockClient.deleteFile).toHaveBeenCalledWith("/test.txt");
187187
});
188188

189+
it("normalizes double slashes in paths", async () => {
190+
const fs = WebDAVFileSystem.fromSameClient(
191+
{ client: mockClient, url: "https://dav.example.com", basePath: "/ScriptCat//sync" } as any,
192+
"/ScriptCat//sync"
193+
);
194+
195+
await fs.delete("dir//file.user.js");
196+
197+
expect(mockClient.deleteFile).toHaveBeenCalledWith("/ScriptCat/sync/dir/file.user.js");
198+
});
199+
189200
it("应当在 404 时静默成功(幂等删除)", async () => {
190201
(mockClient.deleteFile as ReturnType<typeof vi.fn>).mockRejectedValue({
191202
response: { status: 404 },
@@ -197,6 +208,19 @@ describe("WebDAVFileSystem", () => {
197208
});
198209
});
199210

211+
describe("create", () => {
212+
it("normalizes double slashes in paths", async () => {
213+
const fs = WebDAVFileSystem.fromSameClient(
214+
{ client: mockClient, url: "https://dav.example.com", basePath: "/ScriptCat//sync" } as any,
215+
"/ScriptCat//sync"
216+
);
217+
218+
const writer = await fs.create("dir//file.user.js");
219+
220+
expect((writer as any).path).toBe("/ScriptCat/sync/dir/file.user.js");
221+
});
222+
});
223+
200224
describe("list", () => {
201225
it("应当列出文件并过滤目录", async () => {
202226
(mockClient.getDirectoryContents as ReturnType<typeof vi.fn>).mockResolvedValue([

0 commit comments

Comments
 (0)