🐛(backend) sanitize slash in template-created filenames#641
Open
neilcroft wants to merge 1 commit intosuitenumerique:mainfrom
Open
🐛(backend) sanitize slash in template-created filenames#641neilcroft wants to merge 1 commit intosuitenumerique:mainfrom
neilcroft wants to merge 1 commit intosuitenumerique:mainfrom
Conversation
Titles containing '/' (e.g. "30/03/30 - liste à faire") produced a file_key with spurious path separators, crashing WOPI on open. Extract `format_template_filename()` to replace '/' with '-' when building the filename from a template title. Closes suitenumerique#626
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Titles containing '/' (e.g. "30/03/30 - liste à faire") produced a file_key with spurious path separators, crashing WOPI on open.
Closes #626
Proposal
Extract
format_template_filename()to replace '/' with '-' when building the filename from a template title.Alternatives considered
Applying sanitize_filename() to template filenames
This existing function converts to ASCII, replaces spaces with underscores, and strips special characters. It would fix the / issue but also mangle titles like "café résumé" into "cafe_resume", which diverges from what the user typed. Since the frontend displays title (not filename), the name would look fine in Drive but appear mangled in Collabora's editor (BaseFileName).
Aligning all filename paths to only replace /
I explored replacing sanitize_filename() with the minimal
/ -> -replacement in the rename task and update validation as well (not just template creation). This would make filenames consistent across all paths (template, rename, WOPI) and preserve unicode/accents everywhere. I decided against it for now as it changes behavior on existing rename and update paths, and S3 keys would start containing characters they didn't before. The current fix is scoped to the template creation path where the bug occurs.