From 71a4b9cb43cf56d151eae75726f46b301fe8a6c4 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Tue, 26 May 2026 15:43:06 +0200 Subject: [PATCH] Bug 42360: [25.05] Move separator validation to C4::Context::csv_delimiter So we protect other calls Signed-off-by: Jonathan Druart Signed-off-by: Wainui Witika-Park --- C4/Context.pm | 11 +++++++++++ C4/Reports.pm | 4 ++++ reports/acquisitions_stats.pl | 9 +-------- t/Context.t | 14 +++++++++++++- 4 files changed, 29 insertions(+), 9 deletions(-) diff --git a/C4/Context.pm b/C4/Context.pm index 7860205516..926ae9fed4 100644 --- a/C4/Context.pm +++ b/C4/Context.pm @@ -51,6 +51,10 @@ use Koha::Config::SysPrefs; use Koha::Config; use Koha; +# NOTE: The below list should match the corresponding options in the .pref file +# for CSVDelimiter +use constant ALLOWED_CSV_DELIMITERS => ( ';', 'tabulation', ',', '/', '\\', '#' ); + =head1 NAME C4::Context - Maintain and manipulate the context of a Koha script @@ -478,6 +482,13 @@ sub delete_preference { sub csv_delimiter { my ( $self, $value ) = @_; + + # Validate input against allowed delimiters + if ($value) { + my %allowed = map { $_ => 1 } ALLOWED_CSV_DELIMITERS; + $value = undef unless $allowed{$value}; + } + my $delimiter = $value || $self->preference('CSVDelimiter') || ','; $delimiter = "\t" if $delimiter eq 'tabulation'; return $delimiter; diff --git a/C4/Reports.pm b/C4/Reports.pm index 718fac1445..e9966c39b2 100644 --- a/C4/Reports.pm +++ b/C4/Reports.pm @@ -64,6 +64,10 @@ sub GetDelimiterChoices { my ( $choices, $default ) = $sth->fetchrow; my @dels = split /\|/, $choices; + #See BZ 41834 to see why this kludge is used here in older versions + # Use the centralized list of allowed delimiters from C4::Context + @dels = C4::Context::ALLOWED_CSV_DELIMITERS; + return { values => \@dels, default => $default, diff --git a/reports/acquisitions_stats.pl b/reports/acquisitions_stats.pl index 067ff11016..c04bc95757 100755 --- a/reports/acquisitions_stats.pl +++ b/reports/acquisitions_stats.pl @@ -119,14 +119,7 @@ if ($basename_input) { #NOTE: Ideally it would be nice to validate the Filter parameter, #but that would take a fair bit of work... -#NOTE: Validate sep parameter -our $sep = C4::Context->csv_delimiter(); -my $sep_input = scalar $input->param("sep"); -my $delimiter_choices = GetDelimiterChoices(); -my %allowed_seps = map { $_ => 1 } @{ $delimiter_choices->{values} }; -if ( $sep_input && $allowed_seps{$sep_input} ) { - $sep = $sep_input; -} +our $sep = C4::Context->csv_delimiter( scalar $input->param("sep") ); $template->param( do_it => $do_it, diff --git a/t/Context.t b/t/Context.t index 4a3b2de636..31b8a3104f 100755 --- a/t/Context.t +++ b/t/Context.t @@ -89,7 +89,7 @@ subtest 'needs_install() tests' => sub { subtest 'csv_delimiter() tests' => sub { - plan tests => 4; + plan tests => 7; t::lib::Mocks::mock_preference( 'CSVDelimiter', undef ); is( C4::Context->csv_delimiter, ',', "csv_delimiter returns comma if system preference CSVDelimiter is undefined" ); @@ -111,6 +111,18 @@ subtest 'csv_delimiter() tests' => sub { C4::Context->csv_delimiter, "\t", "csv_delimiter returns '\t' if system preference CSVDelimiter is tabulation" ); + + t::lib::Mocks::mock_preference( 'CSVDelimiter', ';' ); + is( C4::Context->csv_delimiter(','), ',', "csv_delimiter returns comma if passed" ); + is( + C4::Context->csv_delimiter(";"), ';', + "csv_delimiter returns semicolon if passed" + ); + + is( + C4::Context->csv_delimiter("£"), ';', + "csv_delimiter returns semicolon if not allowed" + ); }; subtest 'default_catalog_sort_by() tests' => sub {