fix(files_external): skip BSD ftpd total N listing headers - #63197
fix(files_external): skip BSD ftpd total N listing headers#63197xusnitdinov wants to merge 1 commit into
Conversation
|
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.) |
joshtrichards
added
bug
3. to review
| $filtered = array_values(array_filter($rawList, static function (string $item): bool { | ||
| $item = trim($item); | ||
| // BSD ftpd (and some Unix servers) prepend listings with a "total N" header | ||
| return $item !== '' && !preg_match('/^total\b/i', $item); |
There was a problem hiding this comment.
| return $item !== '' && !preg_match('/^total\b/i', $item); | |
| return !str_starts_with($item, 'total '); |
This is a lot cheaper.
There was a problem hiding this comment.
Agreed, good call. Though the $item !== '' check still needed too since array_filter() being used with callback.
There was a problem hiding this comment.
I don't understand what you mean. Empty lines or lines with only whitespace weren't filtered out before, so why should they now.
There was a problem hiding this comment.
I meant that str_starts_with() only replaces the regex part of the condition. If we still intend to exclude empty/whitespace-only lines as described in the PR, $item !== '' remains necessary because array_filter() has a callback and therefore won't filter out empty values automatically. Without that check, str_starts_with() returns false for an empty haystack, so the negated condition returns true; the empty line then reaches the parser and causes the same "not enough parts" exception.
There was a problem hiding this comment.
Sure, but I don't see any explanation why filtering out empty lines is necessary.
|
/backport to stable35 |
|
/backport to stable34 |
|
/backport to stable33 |
|
/backport to stable32 |
BSD ftpd prepends directory listings with a `total N` line. The raw LIST parser treated that as a file entry and threw, so external FTP storage against FreeBSD could not be browsed. Skip empty lines and `total` headers the same way flysystem does, while still parsing `.` / `..` as cdir/pdir. Fixes: nextcloud#37045 Signed-off-by: Xusnitdinov Azizbek <252487890+xusnitdinov@users.noreply.github.com>
bdc1f4e to
758a226
Compare
3 participants
Summary
LISToutput with atotal Nline. The raw-list fallback inFtpConnectiontried to parse that as a file entry and threwMetadata can't be parsed from item 'total 17'.totalheaders the same way flysystem does../..are still parsed ascdir/pdir.Fixes #37045
Checklist
Made with Cursor