From 5ca496f1b7881ca20648186dd472ef08a9702b59 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Tue, 21 Jul 2026 11:49:23 +0100 Subject: [PATCH] Bug 43078: Exclude '.' and '..' entries from SFTP file listings The FTP and Local transports both explicitly filter out '.' and '..' pseudo-entries when listing a directory, but the SFTP transport never did - it relied on the assumption that Net::SFTP::Foreign's ls() already excludes them. It doesn't: ls() returns exactly whatever entries the remote server includes in its SSH_FXP_READDIR response, with no default filtering. Whether '.'/'..' show up therefore depends entirely on the remote SFTP server implementation, producing inconsistent results between otherwise identically configured SFTP accounts. Test plan: 1. prove t/db_dependent/Koha/File/Transport/SFTP.t => fails before this patch (listing includes '.' and '..') => passes after this patch 2. prove -r t/db_dependent/Koha/File/Transport/ t/db_dependent/Koha/File/Transports.t => all still pass Signed-off-by: Andrew Fuerste Henry Signed-off-by: Pedro Amorim --- Koha/File/Transport/SFTP.pm | 7 ++++++- t/db_dependent/Koha/File/Transport/SFTP.t | 21 ++++++++++++++++++++- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/Koha/File/Transport/SFTP.pm b/Koha/File/Transport/SFTP.pm index 2ebe9a678e..31d7f3ab45 100644 --- a/Koha/File/Transport/SFTP.pm +++ b/Koha/File/Transport/SFTP.pm @@ -179,6 +179,11 @@ Net::SFTP::Foreign's ls() natively returns the size/perms/mtime nested inside a Net::SFTP::Foreign::Attributes object (the 'a' key). Those fields are flattened out here so the returned shape matches the FTP and Local transports. +Net::SFTP::Foreign::ls() does not filter '.' and '..' out of its +SSH_FXP_READDIR response itself - whether those pseudo-entries appear is +purely down to the remote server implementation. They are skipped here so +behaviour is consistent with the FTP and Local transports. + =cut sub _list_files { @@ -201,7 +206,7 @@ sub _list_files { : S_ISREG($mode) ? 'file' : 'other', } - } @{$file_list}; + } grep { $_->{filename} !~ /^\.\.?$/ } @{$file_list}; $self->add_message( { diff --git a/t/db_dependent/Koha/File/Transport/SFTP.t b/t/db_dependent/Koha/File/Transport/SFTP.t index 001dd70e71..3fdb781a35 100755 --- a/t/db_dependent/Koha/File/Transport/SFTP.t +++ b/t/db_dependent/Koha/File/Transport/SFTP.t @@ -250,7 +250,7 @@ subtest 'change_directory() tests' => sub { }; subtest 'list_files() tests' => sub { - plan tests => 11; + plan tests => 12; $schema->storage->txn_begin; @@ -275,7 +275,21 @@ subtest 'list_files() tests' => sub { my $dir_attrs = Net::SFTP::Foreign::Attributes->new; $dir_attrs->set_perm( oct('040755') ); + # Some SFTP servers include '.' and '..' pseudo-entries in their + # SSH_FXP_READDIR response - Net::SFTP::Foreign::ls() does not filter + # them out by default, so Koha must do it itself, matching the FTP and + # Local transports. my @sftp_ls_output = ( + { + filename => '.', + longname => 'drwxr-xr-x 2 kohauser kohauser 4096 Nov 14 22:15 .', + a => $dir_attrs, + }, + { + filename => '..', + longname => 'drwxr-xr-x 2 kohauser kohauser 4096 Nov 14 22:15 ..', + a => $dir_attrs, + }, { filename => 'QUOTES_413514.CEQ', longname => '-rw-r--r-- 1 kohauser kohauser 1234 Nov 14 22:15 QUOTES_413514.CEQ', @@ -317,6 +331,11 @@ subtest 'list_files() tests' => sub { is( $dir->{type}, 'directory', 'directory gets type "directory"' ); is( $dir->{perms}, '0755', 'directory perms are flattened too' ); + ok( + !( grep { $_->{filename} =~ /^\.\.?$/ } @{$files} ), + "'.' and '..' entries are excluded from the listing" + ); + $transport->{connection} = undef; $schema->storage->txn_rollback;