fix: Improve compatibility with Windows for uploaded files #2513
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2513 +/- ##
=========================================
Coverage 43.40% 43.40%
Complexity 882 882
=========================================
Files 77 77
Lines 3359 3359
=========================================
Hits 1458 1458
Misses 1901 1901 |
Koc
force-pushed
the
bugfix/improve-windows-compatibility
branch
from
ecdefa9 to
556303e
Compare
lib/Service/FormsService.php
Outdated
| @@ -44,6 +44,19 @@ | |||
| * @psalm-import-type FormsShare from ResponseDefinitions | |||
| */ | |||
| class FormsService { | |||
| private const FILENAME_INVALID_CHARS = [ | |||
There was a problem hiding this comment.
Could you move this to the Constants? And how about using the OCP constants and just combining it with the remaining ones?
There was a problem hiding this comment.
Neither of it just use OCP\Files\IFilenameValidator and I do not think we should replace them but forbid upload of files containing them.
But also do it in the fronend:
There was a problem hiding this comment.
Ok, didn't know about that :) But yes, then I agree: we should already use the validation upon uploading the files in the frontend and the backend, so that we don't have to manipulate the filenames afterwards.
There was a problem hiding this comment.
But one thing remains: The folder structure is based on the question titles. So if you for example put a : in the title of a file question, the corresponding folders can't be sync'ed on Windows devices.
There was a problem hiding this comment.
ok, so I've moved FILENAME_INVALID_CHARS into Constants class. Regarding other hints:
- combining
OCP\Constants::FILENAME_INVALID_CHARS+\OCA\Forms\Constants::FILENAME_INVALID_CHARShas no sense for me because I've defined invalid chars as an array but global invalid characters defined as string. Array is better here because we don't need callmb_str_splitall the time IFilenameValidator- we're already calling$userFolder->getStorage()->verifyPath($path, $uploadedFile['name']);during file upload. Is it not enough? Even more, as already mentioned, uploaded file can have correct name, but due to form/question name path can be invalid
Anyway, feel free to force-push any changes into my branch.
Koc
force-pushed
the
bugfix/improve-windows-compatibility
branch
3 times, most recently
from
513f4fa to
c7fdda8
Compare
Signed-off-by: Kostiantyn Miakshyn <[email protected]>
Koc
force-pushed
the
bugfix/improve-windows-compatibility
branch
from
c7fdda8 to
3859656
Compare
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
Fixes #2409