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

@@ -225,9 +225,9 @@ export default abstract class ImportTask extends BaseTask<Props> {
 
           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);
             });
         });
       });

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);
           }
         }
       );

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");
+  });
+});

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<T>(dirName: string): [T, string][] {
   return getFilenamesInDirectory(dirName).map((fileName) => {
     const filePath = path.join(dirName, fileName);