Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix recursive symlinks causing infinite loop #126

Merged
merged 5 commits into from
Oct 14, 2024
Merged

Fix recursive symlinks causing infinite loop #126

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2c7df00
test: formatting
thecodrr Oct 14, 2024
189b0ec
fix: handle recursive symlinks
thecodrr Oct 14, 2024
7112b6d
test: refactor symlink tests to separate file
thecodrr Oct 14, 2024
903f4cd
fix: simplify recursion detection algorithm
thecodrr Oct 14, 2024
100cf2f
test: add tests for resolving recursive symlinks
thecodrr Oct 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
251 changes: 17 additions & 234 deletions __tests__/fdir.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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__");

Expand All @@ -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]();
Expand Down Expand Up @@ -296,7 +253,7 @@ for (const type of apiTypes) {
},
"/some/dir/dir2/dir3": {
file: "some file",
}
},
});

const api = new fdir({ excludeFiles: true, excludeSymlinks: true })
Expand All @@ -306,12 +263,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();
});

Expand All @@ -325,7 +278,7 @@ for (const type of apiTypes) {
},
"/some/dir/dir2/dir3": {
file: "some file",
}
},
});

const api = new fdir({ excludeFiles: true, excludeSymlinks: true })
Expand All @@ -336,15 +289,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();
});

Expand All @@ -356,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).toStrictEqual([
path.join("..", "..", "..", "..", "sym", "linked", "file-1"),
path.join("..", "..", "..", "..", "sym", "linked", "file-excluded-1"),
path.join("..", "..", "..", "..", "other", "dir", "file-2"),
]);
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 === "/" ? "\\" : "/")
Expand All @@ -489,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");
Expand Down Expand Up @@ -624,7 +400,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);
});
Expand Down Expand Up @@ -652,7 +431,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);
});

Expand Down
Loading
Loading