Skip to content

Commit

Permalink
fix(h5p-server): local dir paths with certain unicode characters work (
Browse files Browse the repository at this point in the history
  • Loading branch information
sr258 authored Aug 18, 2021
1 parent cafa5d2 commit 5e07048
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 31 deletions.
21 changes: 12 additions & 9 deletions packages/h5p-server/src/ContentManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,21 +70,25 @@ export default class ContentManager {
): Promise<boolean> => 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,
user: IUser,
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')
);
Expand All @@ -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(
Expand All @@ -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,
Expand Down
10 changes: 5 additions & 5 deletions packages/h5p-server/src/LibraryManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -611,12 +611,12 @@ export default class LibraryManager {
libraryInfo: ILibraryName
): Promise<void> {
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);
Expand Down
3 changes: 2 additions & 1 deletion packages/h5p-server/src/PackageValidator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
45 changes: 30 additions & 15 deletions packages/h5p-server/src/implementation/fs/FileContentStorage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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(
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand Down Expand Up @@ -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,
Expand All @@ -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
*/
Expand All @@ -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,
Expand All @@ -438,14 +452,15 @@ export default class FileContentStorage implements IContentStorage {
this.getContentPath(),
contentId.toString()
);
const contentDirectoryPathLength = contentDirectoryPath.length + 1;
const absolutePaths = await getAllFiles.async.array(
path.join(contentDirectoryPath)
);
const contentPath = path.join(contentDirectoryPath, 'content.json');
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));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -430,8 +430,9 @@ export default class FileLibraryStorage implements ILibraryStorage {
*/
public async listFiles(library: ILibraryName): Promise<string[]> {
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();
Expand Down

0 comments on commit 5e07048

Please sign in to comment.