Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(files): validate input when creating file/directory #46689

Merged
merged 2 commits into from
Jul 24, 2024

Conversation

Antreesy
Copy link
Contributor

Summary

Before After
image image
image image
image image

TODO

  • Review
  • Compile
  • Backport back to stable28

Checklist

@Antreesy Antreesy added this to the Nextcloud 30 milestone Jul 22, 2024
@Antreesy Antreesy requested review from susnux and szaimen July 22, 2024 16:07
@Antreesy Antreesy self-assigned this Jul 22, 2024
@Antreesy Antreesy requested a review from skjnldsv as a code owner July 22, 2024 16:07
Copy link
Contributor

@susnux susnux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is common filename validation now with new nextcloud-files, this function will be removed until beta1 of Nextcloud 30 ;)

@Antreesy
Copy link
Contributor Author

There is common filename validation now with new nextcloud-files, this function will be removed until beta1 of Nextcloud 30 ;)

I saw some PRs on this, but they're not backported, so - not coming to stable branches?
In that case I can rebase my PR directly on stable29 and continue from there

@susnux
Copy link
Contributor

susnux commented Jul 22, 2024

I saw some PRs on this, but they're not backported, so - not coming to stable branches?

Yes and no, the plan is to use the feature introduced here:
nextcloud-libraries/nextcloud-files#1013

This also works for Nextcloud 28 and 29. So we can use validateFilename in all supported versions and return user feedback based on the exception that is thrown. As it is too much work to maintain all three versions if they diverge too much :)

@Antreesy
Copy link
Contributor Author

This also works for Nextcloud 28 and 29.

Got you there 😉

@Antreesy Antreesy removed request for skjnldsv and szaimen July 23, 2024 07:56
@Antreesy Antreesy marked this pull request as draft July 23, 2024 07:56
@susnux
Copy link
Contributor

susnux commented Jul 24, 2024

Lets go with this first, as there is a lot of good stuff!
I will then add the library stuff ontop of your changes.

Thank you!

@susnux susnux marked this pull request as ready for review July 24, 2024 11:12
@susnux susnux requested review from artonge and nfebe July 24, 2024 11:12
Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
@susnux susnux force-pushed the fix/46688/validate-new-file-name branch from 6a54469 to 5709e54 Compare July 24, 2024 11:14
@susnux
Copy link
Contributor

susnux commented Jul 24, 2024

/compile

@susnux
Copy link
Contributor

susnux commented Jul 24, 2024

/backport to stable29

@susnux
Copy link
Contributor

susnux commented Jul 24, 2024

/backport to stable28

Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
@susnux susnux merged commit b839483 into master Jul 24, 2024
109 checks passed
@susnux susnux deleted the fix/46688/validate-new-file-name branch July 24, 2024 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: 'forbidden_chars' not respected when creating File / Directory
4 participants