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

@susnux susnux commented May 8, 2025

Copy link
Copy Markdown
Contributor

Summary

Share the filename sanitizing with the OCP filename validator.

Checklist

@susnux susnux added this to the Nextcloud 32 milestone May 8, 2025
@susnux susnux requested a review from marcelklehr May 8, 2025 11:29
@susnux susnux requested a review from a team as a code owner May 8, 2025 11:29
@susnux susnux requested review from icewind1991, provokateurin and yemkareems and removed request for a team May 8, 2025 11:29
@susnux susnux added the 3. to review Waiting for reviews label May 8, 2025
@susnux susnux requested a review from come-nc May 8, 2025 11:30
@susnux susnux force-pushed the feat/ocp-sanitize-filenames branch from 811ea36 to d293c7a Compare May 12, 2025 13:08
@github-project-automation github-project-automation Bot moved this to 🏗️ In progress in 📁 Files team May 12, 2025
@susnux susnux self-assigned this May 12, 2025
Comment thread lib/private/Files/FilenameValidator.php Outdated
Share the filename sanitizing with the OCP filename validator.

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux susnux force-pushed the feat/ocp-sanitize-filenames branch from d293c7a to 6cf1870 Compare May 13, 2025 12:14
@susnux susnux requested a review from come-nc May 13, 2025 12:14
@come-nc come-nc merged commit fb615ef into master May 13, 2025
192 checks passed
@come-nc come-nc deleted the feat/ocp-sanitize-filenames branch May 13, 2025 21:35
@susnux susnux moved this from 🏗️ In progress to ☑️ Done in 📁 Files team May 15, 2025
@Rello

Rello commented Jun 16, 2025

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

susnux commented Jun 16, 2025

Copy link
Copy Markdown
Contributor Author

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

@AndyScherzinger

Copy link
Copy Markdown
Member

@Rello

Rello commented Jun 16, 2025

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

susnux commented Jun 16, 2025

Copy link
Copy Markdown
Contributor Author

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

@Rello

Rello commented Jun 20, 2025

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

susnux commented Jun 20, 2025

Copy link
Copy Markdown
Contributor Author

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

Rello commented Jun 20, 2025

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

@skjnldsv skjnldsv mentioned this pull request Aug 19, 2025
@skjnldsv skjnldsv modified the milestones: Nextcloud 32, Nextcloud 33 Sep 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: ☑️ Done

Development

Successfully merging this pull request may close these issues.

6 participants