From d3c8224839f09fbff601e4a3e7fd8ff1a6b2ba08 Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Mon, 7 Oct 2024 08:36:18 -0400 Subject: [PATCH] fix: Error during import with long filenames (#7738) * fix: Stream error during import causes worker restart * refactor * fix: Ensure we never write filenames longer than the system can handle --- server/queues/tasks/ImportTask.ts | 6 ++-- server/utils/ZipHelper.ts | 58 ++++++++++++++++++++----------- server/utils/fs.test.ts | 25 ++++++++++++- server/utils/fs.ts | 50 ++++++++++++++++++++++++++ 4 files changed, 114 insertions(+), 25 deletions(-) diff --git a/server/queues/tasks/ImportTask.ts b/server/queues/tasks/ImportTask.ts index 2a8c5f1a5b..5a4af8d164 100644 --- a/server/queues/tasks/ImportTask.ts +++ b/server/queues/tasks/ImportTask.ts @@ -225,9 +225,9 @@ export default abstract class ImportTask extends BaseTask { void ZipHelper.extract(filePath, tmpDir) .then(() => resolve(tmpDir)) - .catch((err) => { - Logger.error("Could not extract zip file", err); - reject(err); + .catch((zErr) => { + Logger.error("Could not extract zip file", zErr); + reject(zErr); }); }); }); diff --git a/server/utils/ZipHelper.ts b/server/utils/ZipHelper.ts index 1fdad20117..c355ed2277 100644 --- a/server/utils/ZipHelper.ts +++ b/server/utils/ZipHelper.ts @@ -6,6 +6,10 @@ import yauzl, { Entry, validateFileName } from "yauzl"; import { bytesToHumanReadable } from "@shared/utils/files"; import Logger from "@server/logging/Logger"; import { trace } from "@server/logging/tracing"; +import { trimFileAndExt } from "./fs"; + +const MAX_FILE_NAME_LENGTH = 255; +const MAX_PATH_LENGTH = 4096; @trace() export default class ZipHelper { @@ -81,9 +85,9 @@ export default class ZipHelper { } } ) - .on("error", (err) => { + .on("error", (rErr) => { dest.end(); - reject(err); + reject(rErr); }) .pipe(dest); } @@ -129,35 +133,47 @@ export default class ZipHelper { // directory file names end with '/' fs.mkdirp( path.join(outputDir, fileName), - function (err: Error) { - if (err) { - throw err; + function (mErr: Error) { + if (mErr) { + return reject(mErr); } zipfile.readEntry(); } ); } else { // file entry - zipfile.openReadStream(entry, function (err, readStream) { - if (err) { - throw err; + zipfile.openReadStream(entry, function (rErr, readStream) { + if (rErr) { + return reject(rErr); } // ensure parent directory exists fs.mkdirp( path.join(outputDir, path.dirname(fileName)), - function (err) { - if (err) { - throw err; + function (mkErr) { + if (mkErr) { + return reject(mkErr); } - readStream.pipe( - fs.createWriteStream(path.join(outputDir, fileName)) + + const location = trimFileAndExt( + path.join( + outputDir, + trimFileAndExt(fileName, MAX_FILE_NAME_LENGTH) + ), + MAX_PATH_LENGTH ); - readStream.on("end", function () { - zipfile.readEntry(); - }); - readStream.on("error", (err) => { - throw err; - }); + const dest = fs + .createWriteStream(location) + .on("error", reject); + + readStream + .on("error", (rsErr) => { + dest.end(); + reject(rsErr); + }) + .on("end", function () { + zipfile.readEntry(); + }) + .pipe(dest); } ); }); @@ -165,8 +181,8 @@ export default class ZipHelper { }); zipfile.on("close", resolve); zipfile.on("error", reject); - } catch (err) { - reject(err); + } catch (zErr) { + reject(zErr); } } ); diff --git a/server/utils/fs.test.ts b/server/utils/fs.test.ts index c36cd1e840..b4365b4e48 100644 --- a/server/utils/fs.test.ts +++ b/server/utils/fs.test.ts @@ -1,4 +1,9 @@ -import { serializeFilename, deserializeFilename } from "./fs"; +import { + serializeFilename, + deserializeFilename, + trimFileAndExt, + stringByteLength, +} from "./fs"; describe("serializeFilename", () => { it("should serialize forward slashes", () => { @@ -31,3 +36,21 @@ describe("deserializeFilename", () => { ); }); }); + +describe("stringByteLength", () => { + it("should return byte length of string", () => { + expect(stringByteLength("")).toBe(0); + expect(stringByteLength("a")).toBe(1); + expect(stringByteLength("🦄")).toBe(4); + expect(stringByteLength("你好")).toBe(6); + }); +}); + +describe("trimFileAndExt", () => { + it("should trim filename", () => { + expect(trimFileAndExt("file.txt", 6)).toBe("fi.txt"); + expect(trimFileAndExt("file.txt", 8)).toBe("file.txt"); + expect(trimFileAndExt("file.md", 9)).toBe("file.md"); + expect(trimFileAndExt("你好.md", 2)).toBe("你.md"); + }); +}); diff --git a/server/utils/fs.ts b/server/utils/fs.ts index 079d1945db..6ddbd15a5c 100644 --- a/server/utils/fs.ts +++ b/server/utils/fs.ts @@ -1,14 +1,58 @@ import path from "path"; import fs from "fs-extra"; +/** + * Serialize a file name for inclusion in a ZIP. + * + * @param text The file name to serialize. + * @returns The serialized file name. + */ export function serializeFilename(text: string): string { return text.replace(/\//g, "%2F").replace(/\\/g, "%5C"); } +/** + * Deserialize a file name serialized with `serializeFilename`. + * + * @param text The file name to deserialize. + * @returns The deserialized file name. + */ export function deserializeFilename(text: string): string { return text.replace(/%2F/g, "/").replace(/%5C/g, "\\"); } +/** + * Get the UTF8 byte length of a string. + * + * @param str The string to measure. + * @returns The byte length of the string. + */ +export function stringByteLength(str: string): number { + return Buffer.byteLength(str, "utf8"); +} + +/** + * Trim a file name to a maximum length, retaining the extension. + * + * @param text The file name to trim. + * @param length The maximum length of the file name. + * @returns The trimmed file name. + */ +export function trimFileAndExt(text: string, length: number): string { + if (stringByteLength(text) > length) { + const ext = path.extname(text); + const name = path.basename(text, ext); + return name.slice(0, length - stringByteLength(ext)) + ext; + } + return text; +} + +/** + * Get a list of file names in a directory. + * + * @param dirName The directory to search. + * @returns A list of file names in the directory. + */ export function getFilenamesInDirectory(dirName: string): string[] { return fs .readdirSync(dirName) @@ -21,6 +65,12 @@ export function getFilenamesInDirectory(dirName: string): string[] { ); } +/** + * Require all files in a directory and return them as an array of tuples. + * + * @param dirName The directory to search. + * @returns An array of tuples containing the required files and their names. + */ export function requireDirectory(dirName: string): [T, string][] { return getFilenamesInDirectory(dirName).map((fileName) => { const filePath = path.join(dirName, fileName);