From 5e070485a8433e0336f9f7e574ab7fb18a95cf2d Mon Sep 17 00:00:00 2001 From: Sebastian Rettig Date: Wed, 18 Aug 2021 15:28:32 +0200 Subject: [PATCH] fix(h5p-server): local dir paths with certain unicode characters work (#1680) --- packages/h5p-server/src/ContentManager.ts | 21 +++++---- packages/h5p-server/src/LibraryManager.ts | 10 ++--- packages/h5p-server/src/PackageValidator.ts | 3 +- .../implementation/fs/FileContentStorage.ts | 45 ++++++++++++------- .../implementation/fs/FileLibraryStorage.ts | 3 +- 5 files changed, 51 insertions(+), 31 deletions(-) diff --git a/packages/h5p-server/src/ContentManager.ts b/packages/h5p-server/src/ContentManager.ts index dd486efdb..e7d400087 100644 --- a/packages/h5p-server/src/ContentManager.ts +++ b/packages/h5p-server/src/ContentManager.ts @@ -70,14 +70,17 @@ export default class ContentManager { ): Promise => this.contentStorage.fileExists(contentId, filename); /** - * 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. + * 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 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 - * @returns The id of the content that was created (the one passed to the method or a new id if there was none). + * @returns The id of the content that was created (the one passed to the + * method or a new id if there was none). */ public async copyContentFromDirectory( packageDirectory: string, @@ -85,6 +88,7 @@ export default class ContentManager { contentId?: ContentId ): Promise<{ id: ContentId; metadata: IContentMetadata; parameters: any }> { log.info(`copying content from directory ${packageDirectory}`); + const packageDirectoryLength = packageDirectory.length + 1; const metadata: IContentMetadata = await fsExtra.readJSON( path.join(packageDirectory, 'h5p.json') ); @@ -97,7 +101,7 @@ export default class ContentManager { ) ).filter( (file: string) => - path.relative(packageDirectory, file) !== 'content.json' + file.substr(packageDirectoryLength) !== 'content.json' ); const newContentId: ContentId = await this.contentStorage.addContent( @@ -106,14 +110,13 @@ export default class ContentManager { user, contentId ); + const contentPath = path.join(packageDirectory, 'content'); + const contentPathLength = contentPath.length + 1; try { await Promise.all( otherContentFiles.map((file: string) => { const readStream: Stream = fsExtra.createReadStream(file); - const localPath: string = path.relative( - path.join(packageDirectory, 'content'), - file - ); + const localPath: string = file.substr(contentPathLength); log.debug(`adding ${file} to ${packageDirectory}`); return this.contentStorage.addFile( newContentId, diff --git a/packages/h5p-server/src/LibraryManager.ts b/packages/h5p-server/src/LibraryManager.ts index 7d11f9301..c9a09cba5 100644 --- a/packages/h5p-server/src/LibraryManager.ts +++ b/packages/h5p-server/src/LibraryManager.ts @@ -600,8 +600,8 @@ export default class LibraryManager { } /** - * Copies all library file s from a directory (excludes library.json) to the storage. - * Throws errors if something went wrong. + * Copies all library file s from a directory (excludes library.json) to the + * storage. Throws errors if something went wrong. * @param fromDirectory The directory to copy from * @param libraryInfo the library object * @returns @@ -611,12 +611,12 @@ export default class LibraryManager { libraryInfo: ILibraryName ): Promise { log.info(`copying library files from ${fromDirectory}`); + const fromDirectoryLength = fromDirectory.length + 1; const files: string[] = await getAllFiles.async.array(fromDirectory); await Promise.all( files.map((fileFullPath: string) => { - const fileLocalPath: string = path.relative( - fromDirectory, - fileFullPath + const fileLocalPath: string = fileFullPath.substr( + fromDirectoryLength ); if (fileLocalPath === 'library.json') { return Promise.resolve(true); diff --git a/packages/h5p-server/src/PackageValidator.ts b/packages/h5p-server/src/PackageValidator.ts index d4146dcc3..da617f103 100644 --- a/packages/h5p-server/src/PackageValidator.ts +++ b/packages/h5p-server/src/PackageValidator.ts @@ -156,8 +156,9 @@ export default class PackageValidator { log.debug(`validating package in directory ${packagePath}`); await this.initializeJsonValidators(); + const packagePathLength = packagePath.length + 1; const files = (await getAllFiles.async.array(packagePath)).map((f) => - upath.toUnix(path.relative(packagePath, f)) + upath.toUnix(f.substr(packagePathLength)) ); const result = await new ValidatorBuilder() diff --git a/packages/h5p-server/src/implementation/fs/FileContentStorage.ts b/packages/h5p-server/src/implementation/fs/FileContentStorage.ts index cef24f37e..a6bb3c22d 100644 --- a/packages/h5p-server/src/implementation/fs/FileContentStorage.ts +++ b/packages/h5p-server/src/implementation/fs/FileContentStorage.ts @@ -55,7 +55,8 @@ export default class FileContentStorage implements IContentStorage { } /** - * @param contentPath The absolute path to the directory where the content should be stored + * @param contentPath The absolute path to the directory where the content + * should be stored */ constructor( protected contentPath: string, @@ -106,8 +107,10 @@ export default class FileContentStorage implements IContentStorage { } /** - * Creates a content object in the repository. Add files to it later with addContentFile(...). - * Throws an error if something went wrong. In this case no traces of the content are left in storage and all changes are reverted. + * Creates a content object in the repository. Add files to it later with + * addContentFile(...). Throws an error if something went wrong. In this + * case no traces of the content are left in storage and all changes are + * reverted. * @param metadata The metadata of the content (= h5p.json) * @param content the content object (= content/content.json) * @param user The user who owns this object. @@ -147,7 +150,8 @@ export default class FileContentStorage implements IContentStorage { } /** - * Adds a content file to an existing content object. The content object has to be created with createContent(...) first. + * Adds a content file to an existing content object. The content object has + * to be created with createContent(...) first. * @param id The id of the content to add the file to * @param filename The filename * @param stream A readable stream that contains the data @@ -262,7 +266,8 @@ export default class FileContentStorage implements IContentStorage { } /** - * Returns information about a content file (e.g. image or video) inside a piece of content. + * 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 @@ -286,12 +291,15 @@ export default class FileContentStorage implements IContentStorage { } /** - * Returns a readable stream of a content file (e.g. image or video) inside a piece of content + * Returns a readable stream of 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 * @param user the user who wants to retrieve the content file - * @param rangeStart (optional) the position in bytes at which the stream should start - * @param rangeEnd (optional) the position in bytes at which the stream should end + * @param rangeStart (optional) the position in bytes at which the stream + * should start + * @param rangeEnd (optional) the position in bytes at which the stream + * should end * @returns */ public async getFileStream( @@ -320,7 +328,8 @@ export default class FileContentStorage implements IContentStorage { /** * Returns the content metadata (=h5p.json) for a content id * @param contentId the content id for which to retrieve the metadata - * @param user (optional) the user who wants to access the metadata. If undefined, access must be granted. + * @param user (optional) the user who wants to access the metadata. If + * undefined, access must be granted. * @returns the metadata */ public async getMetadata( @@ -337,7 +346,8 @@ export default class FileContentStorage implements IContentStorage { /** * Returns the parameters (=content.json) for a content id * @param contentId the content id for which to retrieve the metadata - * @param user (optional) the user who wants to access the metadata. If undefined, access must be granted. + * @param user (optional) the user who wants to access the metadata. If + * undefined, access must be granted. * @returns the parameters */ public async getParameters( @@ -386,7 +396,8 @@ export default class FileContentStorage implements IContentStorage { * Returns an array of permissions that the user has on the piece of content * @param contentId the content id to check * @param user the user who wants to access the piece of content - * @returns the permissions the user has for this content (e.g. download it, delete it etc.) + * @returns the permissions the user has for this content (e.g. download it, + * delete it etc.) */ public async getUserPermissions( contentId: ContentId, @@ -402,7 +413,8 @@ export default class FileContentStorage implements IContentStorage { } /** - * Lists the content objects in the system (if no user is specified) or owned by the user. + * Lists the content objects in the system (if no user is specified) or + * owned by the user. * @param user (optional) the user who owns the content * @returns a list of contentIds */ @@ -425,10 +437,12 @@ export default class FileContentStorage implements IContentStorage { } /** - * Gets the filenames of files added to the content with addContentFile(...) (e.g. images, videos or other files) + * Gets the filenames of files added to the content with addContentFile(...) + * (e.g. images, videos or other files) * @param contentId the piece of content * @param user the user who wants to access the piece of content - * @returns a list of files that are used in the piece of content, e.g. ['image1.png', 'video2.mp4'] + * @returns a list of files that are used in the piece of content, e.g. + * ['image1.png', 'video2.mp4'] */ public async listFiles( contentId: ContentId, @@ -438,6 +452,7 @@ export default class FileContentStorage implements IContentStorage { this.getContentPath(), contentId.toString() ); + const contentDirectoryPathLength = contentDirectoryPath.length + 1; const absolutePaths = await getAllFiles.async.array( path.join(contentDirectoryPath) ); @@ -445,7 +460,7 @@ export default class FileContentStorage implements IContentStorage { const h5pPath = path.join(contentDirectoryPath, 'h5p.json'); return absolutePaths .filter((p) => p !== contentPath && p !== h5pPath) - .map((p) => path.relative(contentDirectoryPath, p)); + .map((p) => p.substr(contentDirectoryPathLength)); } /** diff --git a/packages/h5p-server/src/implementation/fs/FileLibraryStorage.ts b/packages/h5p-server/src/implementation/fs/FileLibraryStorage.ts index 3e68aa8b3..e1ff13273 100644 --- a/packages/h5p-server/src/implementation/fs/FileLibraryStorage.ts +++ b/packages/h5p-server/src/implementation/fs/FileLibraryStorage.ts @@ -430,8 +430,9 @@ export default class FileLibraryStorage implements ILibraryStorage { */ public async listFiles(library: ILibraryName): Promise { const libPath = this.getDirectoryPath(library); + const libPathLength = libPath.length + 1; return (await getAllFiles.async.array(libPath)) - .map((p) => path.relative(libPath, p)) + .map((p) => p.substr(libPathLength)) .filter((p) => !this.isIgnored(p)) .map((p) => upath.toUnix(p)) .sort();