[Bug 42992] New: FTP file transport list_files() corrupts filenames
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42992 Bug ID: 42992 Summary: FTP file transport list_files() corrupts filenames Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: major Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: martin.renvoize@openfifth.co.uk QA Contact: testopia@bugs.koha-community.org Target Milestone: --- Koha::File::Transport::FTP->_list_files parses the LIST / "ls -l" output returned by the FTP server. The date field in that output is three whitespace-separated tokens (Mon DD HH:MM, or Mon DD YYYY for older files), but the parse used a non-greedy capture that only consumed the first token (the month). The remaining two date tokens were left glued to the front of the filename, so every filename returned for an FTP transport was prefixed with the listing date, e.g. -rw-r--r-- 1 user group 1234 Jan 01 12:00 QUOTES_413514.CEQ becomes filename "01 12:00 QUOTES_413514.CEQ" Callers (Koha::Edifact::Transport and misc/cronjobs/marc_ordering_process.pl) then ask the server for a file that does not exist, so no files are downloaded, renamed or deleted over FTP. SFTP and Local transports are not affected. This regressed when EDI transport moved onto the file_transports system (bug 38489): the old Koha::Edifact::Transport::ftp_download listed files with Net::FTP->ls (NLST, bare names); the refactor switched to Net::FTP->dir (LIST) to return the richer hashref shape used by the SFTP and Local transports, but the parse was faulty. The fix consumes the date field explicitly as three tokens so the filename is captured cleanly (spaces preserved, directories still skipped). The LIST-based approach and the filename/longname/size/perms return shape are kept so the structure stays consistent across transport classes. Test plan: 1. prove t/db_dependent/Koha/File/Transport/FTP.t 2. Revert Koha/File/Transport/FTP.pm only and re-run: the list_files() subtest fails on the filename/size/perms assertions. 3. prove t/db_dependent/Koha/Edifact/Transport.t -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42992 --- Comment #1 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 201498 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201498&action=edit Bug 42992: FTP file transport: fix list_files() corrupting filenames Koha::File::Transport::FTP::_list_files parsed the LIST / "ls -l" output with a non-greedy date capture that only consumed the first of the three date tokens (the month), leaving the day and time glued to the front of the filename. Every filename returned for an FTP transport was therefore prefixed with the listing date, e.g. -rw-r--r-- 1 user group 1234 Jan 01 12:00 QUOTES_413514.CEQ => filename "01 12:00 QUOTES_413514.CEQ" Callers (Koha::Edifact::Transport, marc_ordering_process.pl) then asked the server for a file that does not exist, so no files were downloaded, renamed or deleted over FTP. SFTP and Local transports were unaffected. This regressed when the EDI transport was moved onto the file_transports system (bug 38489). The previous Koha::Edifact::Transport::ftp_download listed files with Net::FTP->ls (NLST, bare names); the refactor switched to Net::FTP->dir (LIST) so it could return the richer hashref shape used by the SFTP and Local transports, but the parse was faulty. The date field in "ls -l" output is a fixed run of three whitespace separated tokens (Mon DD HH:MM or Mon DD YYYY). Consume it explicitly so the filename is captured cleanly, preserving spaces and still skipping directories. The LIST-based approach and the filename/longname/size/perms return shape are kept so the structure stays consistent across transport classes (relied on by consumers, including out-of-tree ones). Test plan: 1) prove t/db_dependent/Koha/File/Transport/FTP.t 2) git checkout Koha/File/Transport/FTP.pm and re-run: the list_files() subtest fails on the filename/size/perms assertions 3) prove t/db_dependent/Koha/Edifact/Transport.t -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42992 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |martin.renvoize@openfifth.c |ity.org |o.uk QA Contact|testopia@bugs.koha-communit |kyle@bywatersolutions.com |y.org | Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42992 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42992 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #201498|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42992 --- Comment #2 from David Nind <david@davidnind.com> --- Created attachment 201501 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201501&action=edit Bug 42992: FTP file transport: fix list_files() corrupting filenames Koha::File::Transport::FTP::_list_files parsed the LIST / "ls -l" output with a non-greedy date capture that only consumed the first of the three date tokens (the month), leaving the day and time glued to the front of the filename. Every filename returned for an FTP transport was therefore prefixed with the listing date, e.g. -rw-r--r-- 1 user group 1234 Jan 01 12:00 QUOTES_413514.CEQ => filename "01 12:00 QUOTES_413514.CEQ" Callers (Koha::Edifact::Transport, marc_ordering_process.pl) then asked the server for a file that does not exist, so no files were downloaded, renamed or deleted over FTP. SFTP and Local transports were unaffected. This regressed when the EDI transport was moved onto the file_transports system (bug 38489). The previous Koha::Edifact::Transport::ftp_download listed files with Net::FTP->ls (NLST, bare names); the refactor switched to Net::FTP->dir (LIST) so it could return the richer hashref shape used by the SFTP and Local transports, but the parse was faulty. The date field in "ls -l" output is a fixed run of three whitespace separated tokens (Mon DD HH:MM or Mon DD YYYY). Consume it explicitly so the filename is captured cleanly, preserving spaces and still skipping directories. The LIST-based approach and the filename/longname/size/perms return shape are kept so the structure stays consistent across transport classes (relied on by consumers, including out-of-tree ones). Test plan: 1) prove t/db_dependent/Koha/File/Transport/FTP.t 2) git checkout Koha/File/Transport/FTP.pm and re-run: the list_files() subtest fails on the filename/size/perms assertions 3) prove t/db_dependent/Koha/Edifact/Transport.t Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42992 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #3 from David Nind <david@davidnind.com> --- Wasn't sure of the exact sequence, so did this: 1. Ran the tests before the patch - they pass: - prove t/db_dependent/Koha/File/Transport/FTP.t t/db_dependent/Koha/Edifact/Transport.t 2. Applied the patch 3. Ran the tests again - they still pass. 4. git checkout b0fa3d7e6a -- Koha/File/Transport/FTP.pm [1] 5. Run prove t/db_dependent/Koha/File/Transport/FTP.t - tests fail as per the test plan 6. git checkout 4a435ec077 -- Koha/File/Transport/FTP.pm [2] 7. Sign off [1] The commit hash for the patch before this patch may be different, use git log --oneline [2] The commit hash for the current patch may be different, use git log --oneline -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42992 --- Comment #4 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Thanks for testing David.. this one is indeed a difficult one to get to grips with.. it hurt my brain ;). I sort of discovered the easiest way to 'see' the difference before and after the patches is to use https://github.com/openfifth/koha-plugin-transport-browser (I only though about that after submitted my test plan in the bug). But even then, you'll need a FTP server somewhere to test it with) What you would find is that the file listing before the patch would look something like: `00 01 MAY filename.md` Then after the patch it'll be: `filename.md` And the other columns will contain the properly split out date instead of it being squashed into the filename string. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42992 --- Comment #5 from David Nind <david@davidnind.com> --- Thanks Martin! I did have some instructions on how to start KTD up with an SFTP server, but these are now outdated with changes made to KTD: https://gitlab.com/-/snippets/1893788#running-an-sftp-server I've added an issue for KTD https://gitlab.com/koha-community/koha-testing-docker/-/work_items/556 David -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42992 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |43078 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43078 [Bug 43078] File transport SFTP backend returns inconsistent list() structure compared to FTP/Local backends -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42992 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |39190 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39190 [Bug 39190] Rework new (S)FTP classes to be polymorphic classes -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org