mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 12:32:43 +00:00
fix(backup): create a directory one level at a time
Node's recursive mkdir retries ENOENT/EEXIST forever below /proc on Linux, inside one native call, so neither a timer nor a test timeout runs again. A DB_BACKUP_DIR or GEOLITE2_DIR pointing there froze the process; the database-backup integration file, which uses such a path for its unwritable case, never finished on a Linux runner. Walk from the nearest existing parent with plain mkdir instead.
This commit is contained in:
1 parent
65414be936
commit
9501d540d0
5 files changed
+146
-3
No files matched your search
@@ -274,6 +274,10 @@
|
||||
- **The hint for reading scans on the device points to the right switch**,
|
||||
Settings, Modules, Labs, in every language.
|
||||
- **Help links open the right page** instead of going through redirects.
|
||||
- **A directory setting below `/proc` no longer freezes the server.** Node's
|
||||
way of creating a folder with its parents loops forever there on Linux,
|
||||
so a `DB_BACKUP_DIR` or `GEOLITE2_DIR` pointing below it stopped the
|
||||
process. Such a directory now reads as not writable.
|
||||
|
||||
### Security
|
||||
|
||||
|
||||
@@ -9,6 +9,7 @@
|
||||
* of the rule: the restore script cannot import TypeScript, so these are the
|
||||
* tests that keep its plain-JS reading of both from drifting.
|
||||
*/
|
||||
import { execFileSync } from "node:child_process";
|
||||
import {
|
||||
chmodSync,
|
||||
createReadStream,
|
||||
@@ -24,6 +25,7 @@ import {
|
||||
import { readdir } from "node:fs/promises";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import { Readable } from "node:stream";
|
||||
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
@@ -53,7 +55,9 @@ import {
|
||||
selectForDeletion,
|
||||
verifyArchive,
|
||||
instanceIdFromEnv,
|
||||
isWritableDirectory,
|
||||
listBucketDumps,
|
||||
makeDirectory,
|
||||
type DirectoryProbe,
|
||||
type DumpSet,
|
||||
type Env,
|
||||
@@ -1025,3 +1029,46 @@ describe("whether the dump directory survives the container", () => {
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("making the backup directory", () => {
|
||||
const root = mkdtempSync(join(tmpdir(), "hl-mkdir-"));
|
||||
afterEach(() => {
|
||||
for (const name of readdirSync(root)) {
|
||||
rmSync(join(root, name), { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("creates every missing level and leaves an existing one alone", () => {
|
||||
const dir = join(root, "a", "b", "c");
|
||||
makeDirectory(dir);
|
||||
expect(statSync(dir).isDirectory()).toBe(true);
|
||||
makeDirectory(dir);
|
||||
expect(isWritableDirectory(dir)).toBe(true);
|
||||
});
|
||||
|
||||
it("answers false below a file", () => {
|
||||
writeFileSync(join(root, "file"), "");
|
||||
expect(isWritableDirectory(join(root, "file", "below"))).toBe(false);
|
||||
});
|
||||
|
||||
// Node's recursive mkdir loops forever below /proc, inside one native call,
|
||||
// so this runs in a child with a deadline: a regression fails here instead
|
||||
// of freezing the test worker.
|
||||
it.runIf(process.platform === "linux")(
|
||||
"answers false below /proc instead of hanging",
|
||||
() => {
|
||||
const core = fileURLToPath(new URL("../dump-core.mjs", import.meta.url));
|
||||
const out = execFileSync(
|
||||
process.execPath,
|
||||
[
|
||||
"--input-type=module",
|
||||
"-e",
|
||||
`const { isWritableDirectory } = await import(${JSON.stringify(core)});
|
||||
console.log(isWritableDirectory("/proc/healthlog-cannot-write-here/nested"));`,
|
||||
],
|
||||
{ encoding: "utf8", timeout: 10_000 },
|
||||
);
|
||||
expect(out.trim()).toBe("false");
|
||||
},
|
||||
);
|
||||
});
|
||||
@@ -43,7 +43,7 @@ import {
|
||||
statSync,
|
||||
} from "node:fs";
|
||||
import { readFile, rename, rm, stat, writeFile } from "node:fs/promises";
|
||||
import { basename, join } from "node:path";
|
||||
import { basename, dirname, join, resolve } from "node:path";
|
||||
import { pipeline } from "node:stream/promises";
|
||||
import { createGunzip } from "node:zlib";
|
||||
|
||||
@@ -208,6 +208,41 @@ export function resolveBackupSchedule(env = process.env) {
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Create `dir` and its missing parents, one level at a time from the nearest
|
||||
* one that exists. Throws what `mkdir` throws.
|
||||
*
|
||||
* Not `mkdirSync(dir, { recursive: true })`: on Linux, below a pseudo file
|
||||
* system such as `/proc`, the child answers ENOENT while its parent answers
|
||||
* EEXIST, and Node's recursive walk retries that pair forever inside one
|
||||
* native call, so no timer, signal handler or test timeout ever runs again.
|
||||
* A `DB_BACKUP_DIR` set to such a path froze the worker. Here every level is
|
||||
* one plain `mkdir`, and the walk is as long as the path.
|
||||
*
|
||||
* @param {string} dir
|
||||
*/
|
||||
export function makeDirectory(dir) {
|
||||
/** @type {string[]} */
|
||||
const missing = [];
|
||||
let current = resolve(dir);
|
||||
while (!existsSync(current)) {
|
||||
missing.push(current);
|
||||
const parent = dirname(current);
|
||||
if (parent === current) break;
|
||||
current = parent;
|
||||
}
|
||||
for (const path of missing.reverse()) {
|
||||
try {
|
||||
mkdirSync(path);
|
||||
} catch (err) {
|
||||
// Created in between by someone else: that is what was wanted.
|
||||
if (/** @type {NodeJS.ErrnoException} */ (err).code !== "EEXIST") {
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether `dir` exists (or can be created) and this process may write in it.
|
||||
* Never throws.
|
||||
@@ -216,7 +251,7 @@ export function resolveBackupSchedule(env = process.env) {
|
||||
*/
|
||||
export function isWritableDirectory(dir) {
|
||||
try {
|
||||
mkdirSync(dir, { recursive: true });
|
||||
makeDirectory(dir);
|
||||
if (!statSync(dir).isDirectory()) return false;
|
||||
accessSync(dir, fsConstants.W_OK | fsConstants.X_OK);
|
||||
return true;
|
||||
|
||||
@@ -28,6 +28,7 @@ vi.mock("@/lib/geo", async (importOriginal) => {
|
||||
import {
|
||||
buildMaxmindDownloadUrl,
|
||||
fetchGeoLite2Databases,
|
||||
makeDirectory,
|
||||
MAXMIND_DOWNLOAD_BASE,
|
||||
} from "@/lib/geo/geolite2-fetch";
|
||||
|
||||
@@ -282,3 +283,28 @@ describe("fetchGeoLite2Databases — gating", () => {
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("makeDirectory", () => {
|
||||
it("creates every missing level and accepts an existing one", async () => {
|
||||
const root = fs.mkdtempSync(path.join(os.tmpdir(), "hl-geo-mkdir-"));
|
||||
try {
|
||||
const dir = path.join(root, "a", "b");
|
||||
await makeDirectory(dir);
|
||||
await makeDirectory(dir);
|
||||
expect(fs.statSync(dir).isDirectory()).toBe(true);
|
||||
} finally {
|
||||
fs.rmSync(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
// The recursive form never settles below /proc; this one rejects.
|
||||
it.runIf(process.platform === "linux")(
|
||||
"rejects below /proc instead of never settling",
|
||||
async () => {
|
||||
await expect(
|
||||
makeDirectory("/proc/healthlog-cannot-write-here/nested"),
|
||||
).rejects.toMatchObject({ code: "ENOENT" });
|
||||
},
|
||||
5_000,
|
||||
);
|
||||
});
|
||||
@@ -227,6 +227,37 @@ async function downloadAndExtractEdition(
|
||||
return tmpDest;
|
||||
}
|
||||
|
||||
/**
|
||||
* Create `dir` and its missing parents one level at a time. Not
|
||||
* `fsp.mkdir(dir, { recursive: true })`: below `/proc` on Linux Node's
|
||||
* recursive walk retries ENOENT/EEXIST forever and the promise never settles.
|
||||
* Same walk as `makeDirectory` in `src/lib/database-backup/dump-core.mjs`,
|
||||
* which this module cannot import (its `child_process` import would undo the
|
||||
* bounded trace described above).
|
||||
*/
|
||||
export async function makeDirectory(dir: string): Promise<void> {
|
||||
const missing: string[] = [];
|
||||
let current = path.resolve(dir);
|
||||
for (;;) {
|
||||
try {
|
||||
await fsp.stat(current);
|
||||
break;
|
||||
} catch {
|
||||
missing.push(current);
|
||||
const parent = path.dirname(current);
|
||||
if (parent === current) break;
|
||||
current = parent;
|
||||
}
|
||||
}
|
||||
for (const p of missing.reverse()) {
|
||||
try {
|
||||
await fsp.mkdir(p);
|
||||
} catch (err) {
|
||||
if ((err as NodeJS.ErrnoException).code !== "EEXIST") throw err;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Fetch both GeoLite2 databases and place them in `geoLiteDir()`.
|
||||
*
|
||||
@@ -255,7 +286,7 @@ export async function fetchGeoLite2Databases(
|
||||
|
||||
const dir = geoLiteDir();
|
||||
try {
|
||||
await fsp.mkdir(dir, { recursive: true });
|
||||
await makeDirectory(dir);
|
||||
} catch (err) {
|
||||
return fail(`geolite2 directory not writable (${dir})`, err);
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user