Bug 42360: Move separator validation to C4::Context::csv_delimiter
So we protect other calls Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This commit is contained in:
parent
1d9c8fd390
commit
5827f2f774
|
|
@ -52,6 +52,10 @@ use Koha::Config::SysPrefs;
|
||||||
use Koha::Config;
|
use Koha::Config;
|
||||||
use Koha::Database;
|
use Koha::Database;
|
||||||
|
|
||||||
|
# NOTE: The below list should match the corresponding options in the .pref file
|
||||||
|
# for CSVDelimiter
|
||||||
|
use constant ALLOWED_CSV_DELIMITERS => ( ';', 'tabulation', ',', '/', '\\', '#' );
|
||||||
|
|
||||||
=head1 NAME
|
=head1 NAME
|
||||||
|
|
||||||
C4::Context - Maintain and manipulate the context of a Koha script
|
C4::Context - Maintain and manipulate the context of a Koha script
|
||||||
|
|
@ -485,6 +489,13 @@ sub delete_preference {
|
||||||
|
|
||||||
sub csv_delimiter {
|
sub csv_delimiter {
|
||||||
my ( $self, $value ) = @_;
|
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') || ',';
|
my $delimiter = $value || $self->preference('CSVDelimiter') || ',';
|
||||||
$delimiter = "\t" if $delimiter eq 'tabulation';
|
$delimiter = "\t" if $delimiter eq 'tabulation';
|
||||||
return $delimiter;
|
return $delimiter;
|
||||||
|
|
|
||||||
|
|
@ -51,8 +51,8 @@ This will return a list of all the available delimiters.
|
||||||
sub GetDelimiterChoices {
|
sub GetDelimiterChoices {
|
||||||
my $default = C4::Context->preference('CSVDelimiter');
|
my $default = C4::Context->preference('CSVDelimiter');
|
||||||
|
|
||||||
# NOTE: The below list should match the corresponding options in the .pref file
|
# Use the centralized list of allowed delimiters from C4::Context
|
||||||
my @dels = ( ';', 'tabulation', ',', '/', '\\', '#' );
|
my @dels = C4::Context::ALLOWED_CSV_DELIMITERS;
|
||||||
|
|
||||||
return {
|
return {
|
||||||
values => \@dels,
|
values => \@dels,
|
||||||
|
|
|
||||||
|
|
@ -119,14 +119,7 @@ if ($basename_input) {
|
||||||
#NOTE: Ideally it would be nice to validate the Filter parameter,
|
#NOTE: Ideally it would be nice to validate the Filter parameter,
|
||||||
#but that would take a fair bit of work...
|
#but that would take a fair bit of work...
|
||||||
|
|
||||||
#NOTE: Validate sep parameter
|
our $sep = C4::Context->csv_delimiter( scalar $input->param("sep") );
|
||||||
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;
|
|
||||||
}
|
|
||||||
|
|
||||||
$template->param(
|
$template->param(
|
||||||
do_it => $do_it,
|
do_it => $do_it,
|
||||||
|
|
|
||||||
14
t/Context.t
14
t/Context.t
|
|
@ -89,7 +89,7 @@ subtest 'needs_install() tests' => sub {
|
||||||
|
|
||||||
subtest 'csv_delimiter() tests' => sub {
|
subtest 'csv_delimiter() tests' => sub {
|
||||||
|
|
||||||
plan tests => 4;
|
plan tests => 7;
|
||||||
|
|
||||||
t::lib::Mocks::mock_preference( 'CSVDelimiter', undef );
|
t::lib::Mocks::mock_preference( 'CSVDelimiter', undef );
|
||||||
is( C4::Context->csv_delimiter, ',', "csv_delimiter returns comma if system preference CSVDelimiter is undefined" );
|
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",
|
C4::Context->csv_delimiter, "\t",
|
||||||
"csv_delimiter returns '\t' if system preference CSVDelimiter is tabulation"
|
"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 {
|
subtest 'default_catalog_sort_by() tests' => sub {
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue