fix(editor): files with very long filenames now shortened

docs/mongo-s3-content-storage.md

@@ -83,6 +83,10 @@ const storage = new dbImplementations.MongoS3ContentStorage(
     you can set any custom configuration values you want.
 -   To achieve greater configurability, you can decide not to use `initS3` or
     `initMongo` and instantiate the required clients yourself.
+-   While Amazon S3 supports keys with up to 1024 characters, some other S3
+    systems such as Minio might only support less in certain situations. To
+    cater for these system you can set the option `maxKeyLength` to the value
+    you need. It defaults to 1024.
 
 ## Using MongoS3ContentStorage in the example
 

docs/s3-temporary-file-storage.md

@@ -68,6 +68,10 @@ const temporaryStorage = new dbImplementations.S3TemporaryFileStorage(
     you can set any custom configuration values you want.
 -   To achieve greater configurability, you can decide not to use `initS3` or
     and instantiate the required client yourself.
+-   While Amazon S3 supports keys with up to 1024 characters, some other S3
+    systems such as Minio might only support less in certain situations. To
+    cater for these system you can set the option `maxKeyLength` to the value
+    you need. It defaults to 1024.
 
 ## Using S3TemporaryFileStorage in the example
 

examples/createH5PEditor.ts

@@ -45,15 +45,31 @@ export default async function createH5PEditor(
                   (await dbImplementations.initMongo()).collection(
                       process.env.CONTENT_MONGO_COLLECTION
                   ),
-                  { s3Bucket: process.env.CONTENT_AWS_S3_BUCKET }
+                  {
+                      s3Bucket: process.env.CONTENT_AWS_S3_BUCKET,
+                      maxKeyLength: process.env.AWS_S3_MAX_FILE_LENGTH
+                          ? Number.parseInt(
+                                process.env.AWS_S3_MAX_FILE_LENGTH,
+                                10
+                            )
+                          : undefined
+                  }
               ),
         process.env.TEMPORARYSTORAGE === 's3'
             ? new dbImplementations.S3TemporaryFileStorage(
                   dbImplementations.initS3({
                       s3ForcePathStyle: true,
                       signatureVersion: 'v4'
                   }),
-                  { s3Bucket: process.env.TEMPORARY_AWS_S3_BUCKET }
+                  {
+                      s3Bucket: process.env.TEMPORARY_AWS_S3_BUCKET,
+                      maxKeyLength: process.env.AWS_S3_MAX_FILE_LENGTH
+                          ? Number.parseInt(
+                                process.env.AWS_S3_MAX_FILE_LENGTH,
+                                10
+                            )
+                          : undefined
+                  }
               )
             : new H5P.fsImplementations.DirectoryTemporaryFileStorage(
                   localTemporaryPath

package.json

@@ -8,27 +8,29 @@
         "url": "https://github.com/Lumieducation/H5P-Nodejs-library"
     },
     "scripts": {
-        "start": "./node_modules/.bin/ts-node examples/express.ts",
-        "prepare": "npm run download:content-type-cache && npm run download:content && npm run download:core && npm run build",
-        "build": "npx tsc -p ./tsconfig.build.json && cp -r src/schemas build/src/schemas",
         "build:watch": "npx tsc -w -p ./tsconfig.build.json",
-        "uninstall": "rm -rf node_modules && rm -rf test/data/hub-content && rm test/data/content-type-cache/real-content-types.json && rm -rf h5p && rm -rf build",
-        "download:content": "node scripts/download-examples.js test/data/content-type-cache/real-content-types.json test/data/hub-content",
-        "download:core": "sh scripts/install.sh",
+        "build": "npx tsc -p ./tsconfig.build.json && cp -r src/schemas build/src/schemas",
+        "ci": "npm run build && npm run lint && npm run format:check && npm run test && npm run test:integration && npm run test:e2e",
         "clear": "rm -rf test/data/hub-content && rm -rf h5p/temporary-storage && rm test/data/content-type-cache/real-content-types.json",
         "download:content-type-cache": "ts-node scripts/update-real-content-type-cache.ts",
-        "ci": "npm run build && npm run lint && npm run format:check && npm run test && npm run test:integration && npm run test:e2e",
+        "download:content": "node scripts/download-examples.js test/data/content-type-cache/real-content-types.json test/data/hub-content",
+        "download:core": "sh scripts/install.sh",
+        "format:check": "npx prettier --check \"{src,test,examples}/**/*.ts\"",
+        "format": "npx prettier --write \"{src,test,examples}/**/*.ts\"",
         "lint": "./node_modules/.bin/tslint --project tsconfig.json --config tslint.json",
-        "test": "jest --testTimeout=120000 --logHeapUsage --maxWorkers=2",
-        "test:watch": "jest --watch",
-        "test:e2e": "npm run test:e2e:player",
-        "test:e2e:player": "./node_modules/.bin/ts-node test/e2e/H5PPlayer.DisplayContent.test.ts",
+        "prepare": "npm run download:content-type-cache && npm run download:content && npm run download:core && npm run build",
+        "semantic-release": "semantic-release",
+        "start": "./node_modules/.bin/ts-node examples/express.ts",
+        "start:dbs": "docker-compose -f test/implementation/db/mongo-s3-docker-compose.yml up -d",
+        "stop:dbs": "docker-compose -f test/implementation/db/mongo-s3-docker-compose.yml down",
         "test:coverage": "npx jest --config jest.coverage.config.js --collect-coverage --testTimeout=120000",
-        "test:integration": "npx jest --config jest.integration.config.js --maxWorkers=2 --logHeapUsage",
         "test:db": "npx jest --config jest.db.config.js --maxWorkers=2 --logHeapUsage",
-        "format:check": "npx prettier --check \"{src,test,examples}/**/*.ts\"",
-        "format": "npx prettier --write \"{src,test,examples}/**/*.ts\"",
-        "semantic-release": "semantic-release"
+        "test:e2e:player": "./node_modules/.bin/ts-node test/e2e/H5PPlayer.DisplayContent.test.ts",
+        "test:e2e": "npm run test:e2e:player",
+        "test:integration": "npx jest --config jest.integration.config.js --maxWorkers=2 --logHeapUsage",
+        "test:watch": "jest --watch",
+        "test": "jest --testTimeout=120000 --logHeapUsage --maxWorkers=2",
+        "uninstall": "rm -rf node_modules && rm -rf test/data/hub-content && rm test/data/content-type-cache/real-content-types.json && rm -rf h5p && rm -rf build"
     },
     "release": {
         "branch": "master"

src/ContentManager.ts

@@ -74,6 +74,8 @@ export default class ContentManager {
     /**
      * Adds content from a H5P package (in a temporary directory) to the installation.
      * It does not check whether the user has permissions to save content.
+     * @deprecated The method should not be used as it anymore, as there might
+     * be issues with invalid filenames!
      * @param packageDirectory The absolute path containing the package (the directory containing h5p.json)
      * @param user The user who is adding the package.
      * @param contentId (optional) The content id to use for the package
@@ -258,4 +260,11 @@ export default class ContentManager {
         log.info(`loading content files for ${contentId}`);
         return this.contentStorage.listFiles(contentId, user);
     }
+
+    public sanitizeFilename(filename: string): string {
+        if (this.contentStorage.sanitizeFilename) {
+            return this.contentStorage.sanitizeFilename(filename);
+        }
+        return filename;
+    }
 }

src/ContentStorer.ts

@@ -395,6 +395,7 @@ export default class ContentStorer {
         let attempts = 0;
         let filenameAttempt = '';
         let exists = false;
+        actualFilename = this.contentManager.sanitizeFilename(actualFilename);
         const dirname = path.dirname(actualFilename);
         do {
             filenameAttempt = `${dirname ? `${dirname}/` : ''}${path.basename(

src/PackageImporter.ts

@@ -111,6 +111,8 @@ export default class PackageImporter {
      * This is __NOT__ what you want if the user is just uploading a package in the editor client!
      *
      * Throws errors if something goes wrong.
+     * @deprecated The method should not be used as it anymore, as there might
+     * be issues with invalid filenames!
      * @param packagePath The full path to the H5P package file on the local disk.
      * @param user The user who wants to upload the package.
      * @param contentId (optional) the content id to use for the package

src/TemporaryFileManager.ts

@@ -125,14 +125,17 @@ export default class TemporaryFileManager {
         let attempts = 0;
         let filenameAttempt = '';
         let exists = false;
-        const dirname = path.dirname(filename);
+        const cleanedFilename = this.storage.sanitizeFilename
+            ? this.storage.sanitizeFilename(filename)
+            : filename;
+        const dirname = path.dirname(cleanedFilename);
         do {
             filenameAttempt = `${
                 dirname && dirname !== '.' ? `${dirname}/` : ''
             }${path.basename(
-                filename,
-                path.extname(filename)
-            )}-${shortid()}${path.extname(filename)}`;
+                cleanedFilename,
+                path.extname(cleanedFilename)
+            )}-${shortid()}${path.extname(cleanedFilename)}`;
             exists = await this.storage.fileExists(filenameAttempt, user);
             attempts += 1;
         } while (attempts < 5 && exists); // only try 5 times

src/implementation/db/MongoS3ContentStorage.ts

@@ -14,7 +14,7 @@ import {
 } from '../../types';
 import Logger from '../../helpers/Logger';
 import H5pError from '../../helpers/H5pError';
-import { validateFilename } from './S3Utils';
+import { validateFilename, sanitizeFilename } from './S3Utils';
 
 const log = new Logger('MongoS3ContentStorage');
 
@@ -45,6 +45,12 @@ export default class MongoS3ContentStorage implements IContentStorage {
                 contentId: ContentId,
                 user: IUser
             ) => Promise<Permission[]>;
+            /**
+             * Indicates how long keys in S3 can be. Defaults to 1024. (S3
+             * supports 1024 characters, other systems such as Minio might only
+             * support 255 on Windows).
+             */
+            maxKeyLength?: number;
             /**
              * The ACL to use for uploaded content files. Defaults to private.
              */
@@ -56,8 +62,20 @@ export default class MongoS3ContentStorage implements IContentStorage {
         }
     ) {
         log.info('initialize');
+        this.maxKeyLength =
+            options?.maxKeyLength !== undefined
+                ? options.maxKeyLength - 22
+                : 1002;
+        // By default we shorten to 1002 as S3 supports a maximum of 1024
+        // characters and we need to account for contentIds (12), unique ids
+        // appended to the name (8) and separators (2).
     }
 
+    /**
+     * Indicates how long keys can be.
+     */
+    private maxKeyLength: number;
+
     /**
      * Generates the S3 key for a file in a content object
      * @param contentId
@@ -402,6 +420,14 @@ export default class MongoS3ContentStorage implements IContentStorage {
         return true;
     }
 
+    /**
+     * Returns information about a content file (e.g. image or video) inside a
+     * piece of content.
+     * @param id the id of the content object that the file is attached to
+     * @param filename the filename of the file to get information about
+     * @param user the user who wants to retrieve the content file
+     * @returns
+     */
     getFileStats(
         contentId: string,
         file: string,
@@ -646,4 +672,16 @@ export default class MongoS3ContentStorage implements IContentStorage {
         log.debug(`Found ${files.length} file(s) in S3.`);
         return files;
     }
+
+    /**
+     * Removes invalid characters from filenames and enforces other filename
+     * rules required by the storage implementation (e.g. filename length
+     * restrictions).
+     * @param filename the filename to sanitize; this can be a relative path
+     * (e.g. "images/image1.png")
+     * @returns the clean filename
+     */
+    public sanitizeFilename(filename: string): string {
+        return sanitizeFilename(filename, this.maxKeyLength);
+    }
 }

src/implementation/db/S3TemporaryFileStorage.ts

@@ -8,7 +8,7 @@ import {
     IH5PConfig
 } from '../../types';
 import { ReadStream } from 'fs';
-import { validateFilename } from './S3Utils';
+import { validateFilename, sanitizeFilename } from './S3Utils';
 import Logger from '../../helpers/Logger';
 import H5pError from '../../helpers/H5pError';
 
@@ -43,6 +43,12 @@ export default class S3TemporaryFileStorage implements ITemporaryFileStorage {
                 userId: string,
                 filename?: string
             ) => Promise<Permission[]>;
+            /**
+             * Indicates how long keys in S3 can be. Defaults to 1024. (S3
+             * supports 1024 characters, other systems such as Minio might only
+             * support 255 on Windows).
+             */
+            maxKeyLength?: number;
             /**
              * The ACL to use for uploaded content files. Defaults to private.
              * See the S3 documentation for possible values.
@@ -53,7 +59,22 @@ export default class S3TemporaryFileStorage implements ITemporaryFileStorage {
              */
             s3Bucket: string;
         }
-    ) {}
+    ) {
+        log.info('initialize');
+
+        this.maxKeyLength =
+            options?.maxKeyLength !== undefined
+                ? options.maxKeyLength - 22
+                : 1002;
+        // By default we shorten to 1002 as S3 supports a maximum of 1024
+        // characters and we need to account for contentIds (12), unique ids
+        // appended to the name (8) and separators (2).
+    }
+
+    /**
+     * Indicates how long keys can be.
+     */
+    private maxKeyLength: number;
 
     /**
      * Deletes the file from temporary storage.
@@ -224,6 +245,18 @@ export default class S3TemporaryFileStorage implements ITemporaryFileStorage {
         return [];
     }
 
+    /**
+     * Removes invalid characters from filenames and enforces other filename
+     * rules required by the storage implementation (e.g. filename length
+     * restrictions).
+     * @param filename the filename to sanitize; this can be a relative path
+     * (e.g. "images/image1.png")
+     * @returns the clean filename
+     */
+    public sanitizeFilename(filename: string): string {
+        return sanitizeFilename(filename, this.maxKeyLength);
+    }
+
     /**
      * DSaves a file to temporary storage.
      * @param filename

src/implementation/db/S3Utils.ts

@@ -1,5 +1,6 @@
 import Logger from '../../helpers/Logger';
 import H5pError from '../../helpers/H5pError';
+import { generalizedSanitizeFilename } from '../utils';
 
 const log = new Logger('S3Utils');
 
@@ -33,3 +34,14 @@ export function validateFilename(filename: string): void {
         throw new H5pError('illegal-filename', { filename }, 400);
     }
 }
+
+export function sanitizeFilename(
+    filename: string,
+    maxFileLength: number
+): string {
+    return generalizedSanitizeFilename(
+        filename,
+        /[&\$@=;:\+\s,\?\\\{\^\}%`\]'">\[~<#|]/g,
+        maxFileLength
+    );
+}

src/implementation/fs/DirectoryTemporaryFileStorage.ts

@@ -10,7 +10,7 @@ import {
     ITemporaryFileStorage,
     IUser
 } from '../../../src';
-import checkFilename from './filenameCheck';
+import { checkFilename, sanitizeFilename } from './filenameUtils';
 
 /**
  * Stores temporary files in directories on the disk.
@@ -23,11 +23,28 @@ export default class DirectoryTemporaryFileStorage
     /**
      * @param directory the directory in which the temporary files are stored.
      * Must be read- and write accessible
+     * @param maxPathLength how long paths can be in the filesystem (Differs
+     * between Windows, Linux and MacOS, so check out the limitation of your
+     * system!)
      */
-    constructor(private directory: string) {
+    constructor(
+        private directory: string,
+        options?: { maxPathLength?: number }
+    ) {
         fsExtra.ensureDirSync(directory);
+        this.maxFileLength =
+            (options?.maxPathLength ?? 255) - (directory.length + 1) - 40;
+        // we subtract 40 for the contentId (12), the unique id attached to the
+        // file (8), the .metadata suffix (9), userIds (8) and separators (3).
+        if (this.maxFileLength < 20) {
+            throw new Error(
+                'The path of the temporary files directory is too long to add files to it. Put the directory into a different location.'
+            );
+        }
     }
 
+    private maxFileLength: number;
+
     public async deleteFile(filename: string, userId: string): Promise<void> {
         checkFilename(filename);
         checkFilename(userId);
@@ -92,6 +109,18 @@ export default class DirectoryTemporaryFileStorage
         ).reduce((prev, curr) => prev.concat(curr), []);
     }
 
+    /**
+     * Removes invalid characters from filenames and enforces other filename
+     * rules required by the storage implementation (e.g. filename length
+     * restrictions).
+     * @param filename the filename to sanitize; this can be a relative path
+     * (e.g. "images/image1.png")
+     * @returns the clean filename
+     */
+    public sanitizeFilename(filename: string): string {
+        return sanitizeFilename(filename, this.maxFileLength);
+    }
+
     public async saveFile(
         filename: string,
         dataStream: ReadStream,