Skip to content

feat(FilenameValidator): allow to sanitize filenames - #52688

Merged
come-nc merged 1 commit into
masterfrom
feat/ocp-sanitize-filenames
May 13, 2025
Merged

feat(FilenameValidator): allow to sanitize filenames#52688
come-nc merged 1 commit into
masterfrom
feat/ocp-sanitize-filenames

Conversation

@susnux

@susnuxsusnux commented May 8, 2025

Copy link
Copy Markdown
Contributor

Summary

Share the filename sanitizing with the OCP filename validator.

Checklist

@susnuxsusnux added this to the Nextcloud 32 milestone May 8, 2025
@susnux
susnux requested a review from marcelklehrMay 8, 2025 11:29
@susnux
susnux requested a review from a team as a code ownerMay 8, 2025 11:29
@susnux
susnux requested review from icewind1991, provokateurin and yemkareems and removed request for a teamMay 8, 2025 11:29
@susnuxsusnux added the 3. to review Waiting for reviews label May 8, 2025
@susnux
susnux requested a review from come-ncMay 8, 2025 11:30
@susnux
susnuxforce-pushed the feat/ocp-sanitize-filenames branch from 811ea36 to d293c7aCompareMay 12, 2025 13:08
@github-project-automationgithub-project-automationBot moved this to 🏗️ In progress in 📁 Files teamMay 12, 2025
@susnuxsusnux self-assigned this May 12, 2025
Comment threadlib/private/Files/FilenameValidator.php Outdated
Share the filename sanitizing with the OCP filename validator.
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux
susnuxforce-pushed the feat/ocp-sanitize-filenames branch from d293c7a to 6cf1870CompareMay 13, 2025 12:14
@susnux
susnux requested a review from come-ncMay 13, 2025 12:14
@Rello

Copy link
Copy Markdown
Contributor

@susnux during a test it was figured out, that the wrong default replacement is being used. As per "standard", the "" should be the default and not the " ".
We can proceed the testing by setting the "
", but for the final users, we want the changed default. can you please correct this?

@AndyScherzinger as discussed...

@susnux

Copy link
Copy Markdown
ContributorAuthor

Not sure I understand this comment, which standard?
Which character should be the default replacement?

@AndyScherzinger

Copy link
Copy Markdown
Member

@Rello

Copy link
Copy Markdown
Contributor

Not sure I understand this comment, which standard? Which character should be the default replacement?

Hi,

this means, if you run the occ without any parameters, it will use " " as the replacement - but it needs to be "_" unless the user chooses differently

@susnux

Copy link
Copy Markdown
ContributorAuthor

Ah ok, makes sense. The _ was missing in the comment above as it makes the text italic on Markdown ;)

@Rello

Copy link
Copy Markdown
Contributor

@susnux
there is one more topic that is showing up in clients - its Case Clashes.

Do you have a clever idea if this could also be covered in a cleanup run?

@susnux

Copy link
Copy Markdown
ContributorAuthor

there is one more topic that is showing up in clients - its Case Clashes.

We excluded case issues when implementing as Windows (at least since Windows 10).
I expected that our Windows client automatically enables case-sensitivity on the synced folders?

Otherwise we need to also implement this which is much deeper in the server code than filename validation, probably somewhere on storage level like we do for some SAMBA storages where case sensitivity was not enabled.

@Rello

Copy link
Copy Markdown
Contributor

I agree - the coding effort will be higher.
we would need to check every file against all files within the same folder.
It might be easier to leave it to the user to clean it up via the client. the client is giving a warning and a resolve-ui.

Cost-Benefit might not be positive here

Screenshot 2025-06-20 at 13 29 14

@skjnldsvskjnldsv mentioned this pull request Aug 19, 2025
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: ☑️ Done

Development

Successfully merging this pull request may close these issues.

6 participants

@susnux@Rello@AndyScherzinger@icewind1991@come-nc@skjnldsv