From 2c7df00c2318e4932109ab33f190fbfbf2aaf2c0 Mon Sep 17 00:00:00 2001 From: Abdullah Atta Date: Mon, 14 Oct 2024 21:37:25 +0500 Subject: [PATCH 1/5] test: formatting --- __tests__/fdir.test.ts | 39 ++++++++++++++++++--------------------- 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/__tests__/fdir.test.ts b/__tests__/fdir.test.ts index f22a8fe..aa933aa 100644 --- a/__tests__/fdir.test.ts +++ b/__tests__/fdir.test.ts @@ -296,7 +296,7 @@ for (const type of apiTypes) { }, "/some/dir/dir2/dir3": { file: "some file", - } + }, }); const api = new fdir({ excludeFiles: true, excludeSymlinks: true }) @@ -306,12 +306,8 @@ for (const type of apiTypes) { const paths = await api[type](); t.expect(paths.length).toBe(5); - t.expect( - paths.filter((p) => p === ".").length - ).toBe(1); - t.expect( - paths.filter((p) => p === "").length - ).toBe(0); + t.expect(paths.filter((p) => p === ".").length).toBe(1); + t.expect(paths.filter((p) => p === "").length).toBe(0); mock.restore(); }); @@ -325,7 +321,7 @@ for (const type of apiTypes) { }, "/some/dir/dir2/dir3": { file: "some file", - } + }, }); const api = new fdir({ excludeFiles: true, excludeSymlinks: true }) @@ -336,15 +332,9 @@ for (const type of apiTypes) { const paths = await api[type](); t.expect(paths.length).toBe(4); - t.expect( - paths.includes(path.join("dir", "dir1/")) - ).toBe(false); - t.expect( - paths.filter((p) => p === ".").length - ).toBe(1); - t.expect( - paths.filter((p) => p === "").length - ).toBe(0); + t.expect(paths.includes(path.join("dir", "dir1/"))).toBe(false); + t.expect(paths.filter((p) => p === ".").length).toBe(1); + t.expect(paths.filter((p) => p === "").length).toBe(0); mock.restore(); }); @@ -435,10 +425,10 @@ for (const type of apiTypes) { .withRelativePaths() .crawl("./some/dir"); const files = await api[type](); - t.expect(files).toStrictEqual([ + t.expect(files.sort()).toStrictEqual([ + path.join("..", "..", "..", "..", "other", "dir", "file-2"), path.join("..", "..", "..", "..", "sym", "linked", "file-1"), path.join("..", "..", "..", "..", "sym", "linked", "file-excluded-1"), - path.join("..", "..", "..", "..", "other", "dir", "file-2"), ]); mock.restore(); }); @@ -624,7 +614,10 @@ test(`paths should never start with ./`, async (t) => { test(`default to . if root is not provided`, async (t) => { const files = await new fdir().crawl().withPromise(); - const files2 = await new fdir().crawl(".").withPromise().then(f => f.sort()); + const files2 = await new fdir() + .crawl(".") + .withPromise() + .then((f) => f.sort()); t.expect(files.sort().every((r, i) => r === files2[i])).toBe(true); }); @@ -652,7 +645,11 @@ test(`there should be no empty directory when using withDirs`, async (t) => { }); test(`there should be no empty directory when using withDirs and filters`, async (t) => { - const files = await new fdir().withDirs().filter(p => p !== "node_modules").crawl("./").withPromise(); + const files = await new fdir() + .withDirs() + .filter((p) => p !== "node_modules") + .crawl("./") + .withPromise(); t.expect(files.every((r) => r.length > 0)).toBe(true); }); From 189b0ecac4fbd57ca4637f9d7d2407a0fd823181 Mon Sep 17 00:00:00 2001 From: Abdullah Atta Date: Mon, 14 Oct 2024 21:38:42 +0500 Subject: [PATCH 2/5] fix: handle recursive symlinks --- src/api/functions/resolve-symlink.ts | 122 ++++++++++++++++----------- src/api/functions/walk-directory.ts | 28 +++--- src/api/walker.ts | 6 +- src/types.ts | 22 +++-- 4 files changed, 101 insertions(+), 77 deletions(-) diff --git a/src/api/functions/resolve-symlink.ts b/src/api/functions/resolve-symlink.ts index 185291f..1ba838d 100644 --- a/src/api/functions/resolve-symlink.ts +++ b/src/api/functions/resolve-symlink.ts @@ -1,5 +1,7 @@ import fs from "fs"; import { WalkerState, Options } from "../../types"; +import { dirname } from "path"; +import { readlink } from "fs/promises"; export type ResolveSymlinkFunction = ( path: string, @@ -14,70 +16,50 @@ const resolveSymlinksAsync: ResolveSymlinkFunction = function ( ) { const { queue, - options: { suppressErrors }, + options: { suppressErrors, useRealPaths }, } = state; queue.enqueue(); - fs.stat(path, (error, stat) => { - if (error) { - queue.dequeue(suppressErrors ? null : error, state); - return; - } + fs.realpath(path, (error, resolvedPath) => { + if (error) return queue.dequeue(suppressErrors ? null : error, state); + + fs.stat(resolvedPath, (error, stat) => { + if (error) return queue.dequeue(suppressErrors ? null : error, state); - callback(stat, path); - queue.dequeue(null, state); + if (stat.isDirectory()) { + isRecursiveAsync(path, resolvedPath, state).then((isRecursive) => { + if (isRecursive) return queue.dequeue(null, state); + callback(stat, useRealPaths ? resolvedPath : path); + queue.dequeue(null, state); + }); + } else { + callback(stat, useRealPaths ? resolvedPath : path); + queue.dequeue(null, state); + } + }); }); }; -const resolveSymlinksWithRealPathsAsync: ResolveSymlinkFunction = function ( +const resolveSymlinks: ResolveSymlinkFunction = function ( path, state, callback ) { const { queue, - options: { suppressErrors }, + options: { suppressErrors, useRealPaths }, } = state; queue.enqueue(); - fs.realpath(path, (error, resolvedPath) => { - if (error) { - queue.dequeue(suppressErrors ? null : error, state); - return; - } - - fs.lstat(resolvedPath, (_error, stat) => { - callback(stat, resolvedPath); - - queue.dequeue(null, state); - }); - }); -}; - -const resolveSymlinksSync: ResolveSymlinkFunction = function ( - path, - state, - callback -) { - try { - const stat = fs.statSync(path); - callback(stat, path); - } catch (e) { - if (!state.options.suppressErrors) throw e; - } -}; - -const resolveSymlinksWithRealPathsSync: ResolveSymlinkFunction = function ( - path, - state, - callback -) { try { const resolvedPath = fs.realpathSync(path); - const stat = fs.lstatSync(resolvedPath); - callback(stat, resolvedPath); + const stat = fs.statSync(resolvedPath); + + if (stat.isDirectory() && isRecursive(path, resolvedPath, state)) return; + + callback(stat, useRealPaths ? resolvedPath : path); } catch (e) { - if (!state.options.suppressErrors) throw e; + if (!suppressErrors) throw e; } }; @@ -87,9 +69,49 @@ export function build( ): ResolveSymlinkFunction | null { if (!options.resolveSymlinks || options.excludeSymlinks) return null; - if (options.useRealPaths) - return isSynchronous - ? resolveSymlinksWithRealPathsSync - : resolveSymlinksWithRealPathsAsync; - return isSynchronous ? resolveSymlinksSync : resolveSymlinksAsync; + return isSynchronous ? resolveSymlinks : resolveSymlinksAsync; +} + +async function isRecursiveAsync( + path: string, + resolved: string, + state: WalkerState +) { + if (state.options.useRealPaths) + return isRecursiveUsingRealPaths(resolved, state); + + let parent = dirname(path); + if (parent + state.options.pathSeparator === state.root || parent === path) + return false; + try { + const resolvedParent = + state.symlinks.get(parent) || (await readlink(parent)); + if (resolvedParent !== resolved) return false; + state.symlinks.set(path, resolved); + return true; + } catch (e) { + return isRecursiveAsync(parent, resolved, state); + } +} + +function isRecursiveUsingRealPaths(resolved: string, state: WalkerState) { + return state.visited.includes(resolved + state.options.pathSeparator); +} + +function isRecursive(path: string, resolved: string, state: WalkerState) { + if (state.options.useRealPaths) + return isRecursiveUsingRealPaths(resolved, state); + + let parent = dirname(path); + if (parent + state.options.pathSeparator === state.root || parent === path) + return false; + try { + const resolvedParent = + state.symlinks.get(parent) || fs.readlinkSync(parent); + if (resolvedParent !== resolved) return false; + state.symlinks.set(path, resolved); + return true; + } catch (e) { + return isRecursive(parent, resolved, state); + } } diff --git a/src/api/functions/walk-directory.ts b/src/api/functions/walk-directory.ts index af9aa47..9e09638 100644 --- a/src/api/functions/walk-directory.ts +++ b/src/api/functions/walk-directory.ts @@ -16,26 +16,19 @@ const walkAsync: WalkDirectoryFunction = ( currentDepth, callback ) => { - state.queue.enqueue(); - - if (currentDepth < 0) { - state.queue.dequeue(null, state); - return; - } + if (currentDepth < 0) return; + state.visited.push(directoryPath); state.counts.directories++; + state.queue.enqueue(); // Perf: Node >= 10 introduced withFileTypes that helps us // skip an extra fs.stat call. - fs.readdir( - directoryPath || ".", - readdirOpts, - function process(error, entries = []) { - callback(entries, directoryPath, currentDepth); - - state.queue.dequeue(state.options.suppressErrors ? null : error, state); - } - ); + fs.readdir(directoryPath || ".", readdirOpts, (error, entries = []) => { + callback(entries, directoryPath, currentDepth); + + state.queue.dequeue(state.options.suppressErrors ? null : error, state); + }); }; const walkSync: WalkDirectoryFunction = ( @@ -44,9 +37,8 @@ const walkSync: WalkDirectoryFunction = ( currentDepth, callback ) => { - if (currentDepth < 0) { - return; - } + if (currentDepth < 0) return; + state.visited.push(directoryPath); state.counts.directories++; let entries: fs.Dirent[] = []; diff --git a/src/api/walker.ts b/src/api/walker.ts index db078e9..ddb5501 100644 --- a/src/api/walker.ts +++ b/src/api/walker.ts @@ -35,7 +35,9 @@ export class Walker { this.isSynchronous = !callback; this.callbackInvoker = invokeCallback.build(options, this.isSynchronous); + this.root = normalizePath(root, options); this.state = { + root: this.root, // Perf: we explicitly tell the compiler to optimize for String arrays paths: [""].slice(0, 0), groups: [], @@ -44,10 +46,10 @@ export class Walker { queue: new Queue((error, state) => this.callbackInvoker(state, error, callback) ), + symlinks: new Map(), + visited: [""].slice(0, 0), }; - this.root = normalizePath(root, this.state.options); - /* * Perf: We conditionally change functions according to options. This gives a slight * performance boost. Since these functions are so small, they are automatically inlined diff --git a/src/types.ts b/src/types.ts index c290a3a..1dd5f46 100644 --- a/src/types.ts +++ b/src/types.ts @@ -26,11 +26,15 @@ export type PathsOutput = string[]; export type Output = OnlyCountsOutput | PathsOutput | GroupOutput; export type WalkerState = { + root: string; paths: string[]; groups: Group[]; counts: Counts; options: Options; queue: Queue; + + symlinks: Map; + visited: string[]; }; export type ResultCallback = ( @@ -60,13 +64,17 @@ export type Options = { relativePaths?: boolean; pathSeparator: PathSeparator; signal?: AbortSignal; - globFunction?: TGlobFunction + globFunction?: TGlobFunction; }; export type GlobMatcher = (test: string) => boolean; -export type GlobFunction = - ((glob: string | string[], ...params: unknown[]) => GlobMatcher); -export type GlobParams = - T extends (globs: string|string[], ...params: infer TParams extends unknown[]) => GlobMatcher - ? TParams - : []; +export type GlobFunction = ( + glob: string | string[], + ...params: unknown[] +) => GlobMatcher; +export type GlobParams = T extends ( + globs: string | string[], + ...params: infer TParams extends unknown[] +) => GlobMatcher + ? TParams + : []; From 7112b6d20cf6836fea0b68fd200c8146b4467fec Mon Sep 17 00:00:00 2001 From: Abdullah Atta Date: Mon, 14 Oct 2024 22:32:33 +0500 Subject: [PATCH 3/5] test: refactor symlink tests to separate file --- __tests__/fdir.test.ts | 216 +------------------------------------ __tests__/symlinks.test.ts | 142 ++++++++++++++++++++++++ __tests__/utils.ts | 24 +++++ 3 files changed, 167 insertions(+), 215 deletions(-) create mode 100644 __tests__/symlinks.test.ts create mode 100644 __tests__/utils.ts diff --git a/__tests__/fdir.test.ts b/__tests__/fdir.test.ts index aa933aa..6d2b693 100644 --- a/__tests__/fdir.test.ts +++ b/__tests__/fdir.test.ts @@ -5,52 +5,12 @@ import { test, beforeEach, TestContext, vi } from "vitest"; import path, { sep } from "path"; import { convertSlashes } from "../src/utils"; import picomatch from "picomatch"; +import { apiTypes, APITypes, cwd, restricted, root } from "./utils"; beforeEach(() => { mock.restore(); }); -const mockFsWithSymlinks = { - "/sym/linked": { - "file-1": "file contents", - "file-excluded-1": "file contents", - }, - "/other/dir": { - "file-2": "file contents2", - }, - "/some/dir": { - fileSymlink: mock.symlink({ - path: "/other/dir/file-2", - }), - fileSymlink2: mock.symlink({ - path: "/other/dir/file-3", - }), - dirSymlink: mock.symlink({ - path: "/sym/linked", - }), - }, -}; - -function root() { - return process.platform === "win32" ? process.cwd().split(path.sep)[0] : "/"; -} - -function cwd() { - return `.${path.sep}`; -} - -function restricted() { - return process.platform === "win32" - ? path.join(root(), "Windows", "System32") - : "/etc"; -} - -function resolveSymlinkRoot(p: string) { - return process.platform === "win32" - ? path.join(root(), path.normalize(p)) - : p; -} - test(`crawl single depth directory with callback`, (t) => { const api = new fdir().crawl("__tests__"); @@ -65,9 +25,6 @@ test(`crawl single depth directory with callback`, (t) => { }); }); -type APITypes = "withPromise" | "sync"; -const apiTypes = ["withPromise", "sync"] as const; - async function crawl(type: APITypes, path: string, t: TestContext) { const api = new fdir().crawl(path); const files = await api[type](); @@ -346,130 +303,6 @@ for (const type of apiTypes) { ).toBeTruthy(); }); - test(`[${type}] crawl all files and include resolved symlinks`, async (t) => { - mock(mockFsWithSymlinks); - - const api = new fdir().withSymlinks().crawl("/some/dir"); - const files = await api[type](); - t.expect(files).toHaveLength(3); - t.expect( - files.indexOf(resolveSymlinkRoot("/sym/linked/file-1")) > -1 - ).toBeTruthy(); - t.expect( - files.indexOf(resolveSymlinkRoot("/other/dir/file-2")) > -1 - ).toBeTruthy(); - mock.restore(); - }); - - test(`[${type}] crawl all files and include resolved symlinks without real paths`, async (t) => { - mock(mockFsWithSymlinks); - - const api = new fdir() - .withSymlinks({ resolvePaths: false }) - .crawl("/some/dir"); - const files = await api[type](); - t.expect(files).toHaveLength(3); - t.expect( - files.indexOf(resolveSymlinkRoot("/some/dir/dirSymlink/file-1")) > -1 - ).toBeTruthy(); - t.expect( - files.indexOf( - resolveSymlinkRoot("/some/dir/dirSymlink/file-excluded-1") - ) > -1 - ).toBeTruthy(); - mock.restore(); - }); - - test(`[${type}] crawl all files and include resolved symlinks without real paths with relative paths on`, async (t) => { - mock(mockFsWithSymlinks); - - const api = new fdir() - .withSymlinks({ resolvePaths: false }) - .withRelativePaths() - .crawl("/some/dir"); - const files = await api[type](); - t.expect(files).toHaveLength(3); - t.expect( - files.indexOf(path.join("dirSymlink", "file-1")) > -1 - ).toBeTruthy(); - t.expect( - files.indexOf(path.join("dirSymlink", "file-excluded-1")) > -1 - ).toBeTruthy(); - t.expect(files.indexOf("fileSymlink") > -1).toBeTruthy(); - mock.restore(); - }); - - test(`[${type}] crawl all files and include resolved symlinks with real paths with relative paths on`, async (t) => { - mock({ - "../../sym/linked": { - "file-1": "file contents", - "file-excluded-1": "file contents", - }, - "../../other/dir": { - "file-2": "file contents2", - }, - "some/dir": { - fileSymlink: mock.symlink({ - path: "../../../../other/dir/file-2", - }), - fileSymlink2: mock.symlink({ - path: "../../../../other/dir/file-3", - }), - dirSymlink: mock.symlink({ - path: "../../../../sym/linked", - }), - }, - }); - const api = new fdir() - .withSymlinks() - .withRelativePaths() - .crawl("./some/dir"); - const files = await api[type](); - t.expect(files.sort()).toStrictEqual([ - path.join("..", "..", "..", "..", "other", "dir", "file-2"), - path.join("..", "..", "..", "..", "sym", "linked", "file-1"), - path.join("..", "..", "..", "..", "sym", "linked", "file-excluded-1"), - ]); - mock.restore(); - }); - - test("crawl all files and include resolved symlinks with exclusions", async (t) => { - mock(mockFsWithSymlinks); - const api = new fdir() - .withSymlinks() - .exclude((_name, path) => path === resolveSymlinkRoot("/sym/linked/")) - .crawl("/some/dir"); - const files = await api[type](); - t.expect(files).toHaveLength(1); - t.expect( - files.indexOf(resolveSymlinkRoot("/other/dir/file-2")) > -1 - ).toBeTruthy(); - mock.restore(); - }); - - test(`[${type}] crawl all files and include unresolved symlinks`, async (t) => { - mock(mockFsWithSymlinks); - - const api = new fdir().withDirs().crawl("/some/dir"); - const files = await api[type](); - t.expect(files).toHaveLength(4); - - t.expect(files.indexOf(path.normalize("/some/dir/")) > -1).toBeTruthy(); - t.expect(files.indexOf("fileSymlink") > -1).toBeTruthy(); - t.expect(files.indexOf("fileSymlink2") > -1).toBeTruthy(); - t.expect(files.indexOf("dirSymlink") > -1).toBeTruthy(); - mock.restore(); - }); - - test(`[${type}] crawl all files and exclude symlinks`, async (t) => { - mock(mockFsWithSymlinks); - - const api = new fdir({ excludeSymlinks: true }).crawl("/some/dir"); - const files = await api[type](); - t.expect(files).toHaveLength(0); - mock.restore(); - }); - test(`[${type}] crawl all files and invert path separator`, async (t) => { const api = new fdir() .withPathSeparator(sep === "/" ? "\\" : "/") @@ -479,53 +312,6 @@ for (const type of apiTypes) { t.expect(files.every((f) => !f.includes(sep))).toBeTruthy(); }); - test(`[${type}] crawl all files (including symlinks)`, async (t) => { - mock({ - "/other/dir": { - "file-3": "somefile", - }, - "/some/dir": { - fileSymlink: mock.symlink({ - path: "/other/dir/file-3", - }), - }, - }); - - const api = new fdir().withErrors().withSymlinks().crawl("/some/dir"); - const files = await api[type](); - t.expect( - files.indexOf(resolveSymlinkRoot("/other/dir/file-3")) > -1 - ).toBeTruthy(); - mock.restore(); - }); - - test(`[${type}] crawl all files (including symlinks without real paths)`, async (t) => { - mock({ - "/other/dir": { - "file-3": "somefile", - }, - "/some/dir": { - fileSymlink: mock.symlink({ - path: "/other/dir/file-3", - }), - }, - }); - - const api = new fdir() - .withErrors() - .withSymlinks({ resolvePaths: false }) - .crawl("/some/dir"); - - await api[type](); - - const files = await api[type](); - t.expect( - files.indexOf(resolveSymlinkRoot("/some/dir/fileSymlink")) > -1 - ).toBeTruthy(); - - mock.restore(); - }); - test(`[${type}] crawl files that match using a custom glob`, async (t) => { const globFunction = vi.fn((glob: string | string[]) => { return (test: string): boolean => test.endsWith(".js"); diff --git a/__tests__/symlinks.test.ts b/__tests__/symlinks.test.ts new file mode 100644 index 0000000..62cba05 --- /dev/null +++ b/__tests__/symlinks.test.ts @@ -0,0 +1,142 @@ +import { afterAll, beforeAll, beforeEach, describe, test } from "vitest"; +import { apiTypes, normalize, root } from "./utils"; +import mock from "mock-fs"; +import { fdir, Options } from "../src"; +import path from "path"; + +const mockFsWithSymlinks = { + "../../sym-relative/linked": { + "file-1": "file contents", + "file-excluded-1": "file contents", + }, + "../../other-relative/dir": { + "file-2": "file contents2", + }, + "relative/dir": { + fileSymlink: mock.symlink({ + path: "../../../../other-relative/dir/file-2", + }), + fileSymlink2: mock.symlink({ + path: "../../../../other-relative/dir/file-3", + }), + dirSymlink: mock.symlink({ + path: "../../../../sym-relative/linked", + }), + }, + + "/sym/linked": { + "file-1": "file contents", + "file-excluded-1": "file contents", + }, + "/other/dir": { + "file-2": "file contents2", + }, + "/some/dir": { + fileSymlink: mock.symlink({ + path: "/other/dir/file-2", + }), + fileSymlink2: mock.symlink({ + path: "/other/dir/file-3", + }), + dirSymlink: mock.symlink({ + path: "/sym/linked", + }), + }, +}; + +for (const type of apiTypes) { + describe.concurrent(type, () => { + beforeAll(() => { + mock(mockFsWithSymlinks); + }); + + afterAll(() => { + mock.restore(); + }); + + test(`resolve symlinks`, async (t) => { + const api = new fdir().withSymlinks().crawl("/some/dir"); + const files = await api[type](); + t.expect(files.sort()).toStrictEqual( + normalize([ + "/other/dir/file-2", + "/sym/linked/file-1", + "/sym/linked/file-excluded-1", + ]) + ); + }); + + test(`resolve symlinks (real paths: false)`, async (t) => { + const api = new fdir() + .withSymlinks({ resolvePaths: false }) + .crawl("/some/dir"); + const files = await api[type](); + t.expect(files.sort()).toStrictEqual( + normalize([ + "/some/dir/dirSymlink/file-1", + "/some/dir/dirSymlink/file-excluded-1", + "/some/dir/fileSymlink", + ]) + ); + }); + + test(`resolve symlinks (real paths: false, relative paths: true)`, async (t) => { + const api = new fdir() + .withSymlinks({ resolvePaths: false }) + .withRelativePaths() + .crawl("/some/dir"); + const files = await api[type](); + t.expect(files.sort()).toStrictEqual( + normalize([ + "dirSymlink/file-1", + "dirSymlink/file-excluded-1", + "fileSymlink", + ]) + ); + }); + + test(`crawl all files and include resolved symlinks with real paths with relative paths on`, async (t) => { + const api = new fdir() + .withSymlinks() + .withRelativePaths() + .crawl("./relative/dir"); + const files = await api[type](); + t.expect(files.sort()).toStrictEqual( + normalize([ + "../../../../other-relative/dir/file-2", + "../../../../sym-relative/linked/file-1", + "../../../../sym-relative/linked/file-excluded-1", + ]) + ); + }); + + test("resolve symlinks (exclude /sym/linked/)", async (t) => { + const api = new fdir() + .withSymlinks() + .exclude((_name, path) => path === resolveSymlinkRoot("/sym/linked/")) + .crawl("/some/dir"); + const files = await api[type](); + t.expect(files.sort()).toStrictEqual(normalize(["/other/dir/file-2"])); + }); + + test(`do not resolve symlinks`, async (t) => { + const api = new fdir().crawl("/some/dir"); + const files = await api[type](); + t.expect(files.sort()).toStrictEqual( + normalize(["dirSymlink", "fileSymlink", "fileSymlink2"]) + ); + }); + + test(`exclude symlinks`, async (t) => { + const api = new fdir({ excludeSymlinks: true }).crawl("/some/dir"); + const files = await api[type](); + t.expect(files).toHaveLength(0); + }); + }); +} + +function resolveSymlinkRoot(p: string) { + return process.platform === "win32" + ? path.join(root(), path.normalize(p)) + : p; +} diff --git a/__tests__/utils.ts b/__tests__/utils.ts new file mode 100644 index 0000000..f3c7cb0 --- /dev/null +++ b/__tests__/utils.ts @@ -0,0 +1,24 @@ +import path from "path"; + +export type APITypes = (typeof apiTypes)[number]; +export const apiTypes = ["withPromise", "sync"] as const; + +export function root() { + return process.platform === "win32" ? process.cwd().split(path.sep)[0] : "/"; +} + +export function cwd() { + return `.${path.sep}`; +} + +export function restricted() { + return process.platform === "win32" + ? path.join(root(), "Windows", "System32") + : "/etc"; +} + +export function normalize(paths: string[]) { + return paths.map((p) => + path.isAbsolute(p) ? path.resolve(p) : path.normalize(p) + ); +} From 903f4cd98ba4fde8345b01e17cf4e23dda4a32db Mon Sep 17 00:00:00 2001 From: Abdullah Atta Date: Tue, 15 Oct 2024 02:58:52 +0500 Subject: [PATCH 4/5] fix: simplify recursion detection algorithm --- src/api/functions/resolve-symlink.ts | 62 ++++++++-------------------- src/api/walker.ts | 2 +- 2 files changed, 19 insertions(+), 45 deletions(-) diff --git a/src/api/functions/resolve-symlink.ts b/src/api/functions/resolve-symlink.ts index 1ba838d..37a9aeb 100644 --- a/src/api/functions/resolve-symlink.ts +++ b/src/api/functions/resolve-symlink.ts @@ -1,7 +1,6 @@ import fs from "fs"; import { WalkerState, Options } from "../../types"; import { dirname } from "path"; -import { readlink } from "fs/promises"; export type ResolveSymlinkFunction = ( path: string, @@ -26,16 +25,11 @@ const resolveSymlinksAsync: ResolveSymlinkFunction = function ( fs.stat(resolvedPath, (error, stat) => { if (error) return queue.dequeue(suppressErrors ? null : error, state); - if (stat.isDirectory()) { - isRecursiveAsync(path, resolvedPath, state).then((isRecursive) => { - if (isRecursive) return queue.dequeue(null, state); - callback(stat, useRealPaths ? resolvedPath : path); - queue.dequeue(null, state); - }); - } else { - callback(stat, useRealPaths ? resolvedPath : path); - queue.dequeue(null, state); - } + if (stat.isDirectory() && isRecursive(path, resolvedPath, state)) + return queue.dequeue(null, state); + + callback(stat, useRealPaths ? resolvedPath : path); + queue.dequeue(null, state); }); }); }; @@ -72,46 +66,26 @@ export function build( return isSynchronous ? resolveSymlinks : resolveSymlinksAsync; } -async function isRecursiveAsync( - path: string, - resolved: string, - state: WalkerState -) { +function isRecursive(path: string, resolved: string, state: WalkerState) { if (state.options.useRealPaths) return isRecursiveUsingRealPaths(resolved, state); let parent = dirname(path); - if (parent + state.options.pathSeparator === state.root || parent === path) - return false; - try { - const resolvedParent = - state.symlinks.get(parent) || (await readlink(parent)); - if (resolvedParent !== resolved) return false; - state.symlinks.set(path, resolved); - return true; - } catch (e) { - return isRecursiveAsync(parent, resolved, state); + let depth = 1; + while (parent !== state.root && depth < 2) { + const resolvedPath = state.symlinks.get(parent); + const isSameRoot = + !!resolvedPath && + (resolvedPath === resolved || + resolvedPath.startsWith(resolved) || + resolved.startsWith(resolvedPath)); + if (isSameRoot) depth++; + else parent = dirname(parent); } + state.symlinks.set(path, resolved); + return depth > 1; } function isRecursiveUsingRealPaths(resolved: string, state: WalkerState) { return state.visited.includes(resolved + state.options.pathSeparator); } - -function isRecursive(path: string, resolved: string, state: WalkerState) { - if (state.options.useRealPaths) - return isRecursiveUsingRealPaths(resolved, state); - - let parent = dirname(path); - if (parent + state.options.pathSeparator === state.root || parent === path) - return false; - try { - const resolvedParent = - state.symlinks.get(parent) || fs.readlinkSync(parent); - if (resolvedParent !== resolved) return false; - state.symlinks.set(path, resolved); - return true; - } catch (e) { - return isRecursive(parent, resolved, state); - } -} diff --git a/src/api/walker.ts b/src/api/walker.ts index ddb5501..9461fbe 100644 --- a/src/api/walker.ts +++ b/src/api/walker.ts @@ -37,7 +37,7 @@ export class Walker { this.root = normalizePath(root, options); this.state = { - root: this.root, + root: this.root.slice(0, -1), // Perf: we explicitly tell the compiler to optimize for String arrays paths: [""].slice(0, 0), groups: [], From 100cf2f0810a70209cf58301fcc78ce6addb4a86 Mon Sep 17 00:00:00 2001 From: Abdullah Atta Date: Tue, 15 Oct 2024 02:59:10 +0500 Subject: [PATCH 5/5] test: add tests for resolving recursive symlinks --- __tests__/symlinks.test.ts | 114 ++++++++++++++++++++++++++++++++++++- 1 file changed, 112 insertions(+), 2 deletions(-) diff --git a/__tests__/symlinks.test.ts b/__tests__/symlinks.test.ts index 62cba05..c5f6213 100644 --- a/__tests__/symlinks.test.ts +++ b/__tests__/symlinks.test.ts @@ -4,7 +4,7 @@ import mock from "mock-fs"; import { fdir, Options } from "../src"; import path from "path"; -const mockFsWithSymlinks = { +const fsWithRelativeSymlinks = { "../../sym-relative/linked": { "file-1": "file contents", "file-excluded-1": "file contents", @@ -23,6 +23,52 @@ const mockFsWithSymlinks = { path: "../../../../sym-relative/linked", }), }, +}; + +const fsWithRecursiveSymlinks = { + "/double/recursive": { + "another-file": "hello", + "recursive-4": mock.symlink({ + path: resolveSymlinkRoot("/recursive"), + }), + }, + "/just/some": { + "another-file": "hello", + "another-file2": "hello", + "symlink-to-earth": mock.symlink({ + path: resolveSymlinkRoot("/random/other"), + }), + }, + "/random/other": { + "another-file": "hello", + "another-file2": "hello", + }, + "/recursive": { + "random-file": "somecontent", + }, + "/recursive/dir": { + "some-file": "somecontent2", + "recursive-1": mock.symlink({ + path: resolveSymlinkRoot("/recursive/dir"), + }), + "recursive-2": mock.symlink({ + path: resolveSymlinkRoot("/recursive/dir/recursive-1"), + }), + "recursive-3": mock.symlink({ + path: resolveSymlinkRoot("/recursive"), + }), + "recursive-5": mock.symlink({ + path: resolveSymlinkRoot("/double/recursive"), + }), + "not-recursive": mock.symlink({ + path: resolveSymlinkRoot("/just/some"), + }), + }, +}; + +const mockFs = { + ...fsWithRelativeSymlinks, + ...fsWithRecursiveSymlinks, "/sym/linked": { "file-1": "file contents", @@ -47,7 +93,7 @@ const mockFsWithSymlinks = { for (const type of apiTypes) { describe.concurrent(type, () => { beforeAll(() => { - mock(mockFsWithSymlinks); + mock(mockFs); }); afterAll(() => { @@ -66,6 +112,70 @@ for (const type of apiTypes) { ); }); + test(`resolve recursive symlinks`, async (t) => { + const api = new fdir().withSymlinks().crawl("/recursive"); + const files = await api[type](); + t.expect(files.sort()).toStrictEqual( + normalize([ + "/double/recursive/another-file", + "/just/some/another-file", + "/just/some/another-file2", + "/random/other/another-file", + "/random/other/another-file2", + "/recursive/dir/some-file", + "/recursive/random-file", + ]) + ); + }); + + test(`resolve recursive symlinks (real paths: false)`, async (t) => { + const api = new fdir() + .withSymlinks({ resolvePaths: false }) + .crawl("/recursive"); + const files = await api[type](); + t.expect(files.sort()).toStrictEqual( + normalize([ + "/recursive/dir/not-recursive/another-file", + "/recursive/dir/not-recursive/another-file2", + "/recursive/dir/not-recursive/symlink-to-earth/another-file", + "/recursive/dir/not-recursive/symlink-to-earth/another-file2", + + "/recursive/dir/recursive-1/not-recursive/another-file", + "/recursive/dir/recursive-1/not-recursive/another-file2", + "/recursive/dir/recursive-1/not-recursive/symlink-to-earth/another-file", + "/recursive/dir/recursive-1/not-recursive/symlink-to-earth/another-file2", + "/recursive/dir/recursive-1/recursive-5/another-file", + "/recursive/dir/recursive-1/some-file", + + "/recursive/dir/recursive-2/not-recursive/another-file", + "/recursive/dir/recursive-2/not-recursive/another-file2", + "/recursive/dir/recursive-2/not-recursive/symlink-to-earth/another-file", + "/recursive/dir/recursive-2/not-recursive/symlink-to-earth/another-file2", + "/recursive/dir/recursive-2/recursive-5/another-file", + "/recursive/dir/recursive-2/some-file", + + "/recursive/dir/recursive-3/dir/not-recursive/another-file", + "/recursive/dir/recursive-3/dir/not-recursive/another-file2", + "/recursive/dir/recursive-3/dir/not-recursive/symlink-to-earth/another-file", + "/recursive/dir/recursive-3/dir/not-recursive/symlink-to-earth/another-file2", + "/recursive/dir/recursive-3/dir/recursive-5/another-file", + "/recursive/dir/recursive-3/dir/some-file", + "/recursive/dir/recursive-3/random-file", + + "/recursive/dir/recursive-5/another-file", + "/recursive/dir/recursive-5/recursive-4/dir/not-recursive/another-file", + "/recursive/dir/recursive-5/recursive-4/dir/not-recursive/another-file2", + "/recursive/dir/recursive-5/recursive-4/dir/not-recursive/symlink-to-earth/another-file", + "/recursive/dir/recursive-5/recursive-4/dir/not-recursive/symlink-to-earth/another-file2", + "/recursive/dir/recursive-5/recursive-4/dir/some-file", + "/recursive/dir/recursive-5/recursive-4/random-file", + + "/recursive/dir/some-file", + "/recursive/random-file", + ]) + ); + }); + test(`resolve symlinks (real paths: false)`, async (t) => { const api = new fdir() .withSymlinks({ resolvePaths: false })