From d455dc31d4f423a154009823b0b8db925c5efe48 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Thu, 2 Mar 2017 14:24:17 +0100 Subject: [PATCH] Bug 18200: Fix a potential issue with preceding space in GetMarcUrls MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Trims the URL in order prevent prefixing a space with http:// Normally you won't have a preceding space here, but I saw it happening one day and it does not cost much to resolve it. Bonus: Adding few simple tests in t/db_dependent/Biblio.t. Test plan: [1] Run t/db_dependent/Biblio.t [2] Add a 856$u with preceding space (MARC21) [3] Check opac-detail, Online access with OPACXSLTDetailsDisplay empty. Signed-off-by: Marcel de Rooy Followed test plan, works as expected Signed-off-by: Marc Véron Signed-off-by: Jonathan Druart Signed-off-by: Kyle M Hall --- C4/Biblio.pm | 1 + t/db_dependent/Biblio.t | 17 ++++++++++++++--- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/C4/Biblio.pm b/C4/Biblio.pm index f475056f9c..d52d7807af 100644 --- a/C4/Biblio.pm +++ b/C4/Biblio.pm @@ -1988,6 +1988,7 @@ sub GetMarcUrls { } my @urls = $field->subfield('u'); foreach my $url (@urls) { + $url =~ s/^\s+|\s+$//g; # trim my $marcurl; if ( $marcflavour eq 'MARC21' ) { my $s3 = $field->subfield('3'); diff --git a/t/db_dependent/Biblio.t b/t/db_dependent/Biblio.t index 9b3cbe00a4..c6613df686 100755 --- a/t/db_dependent/Biblio.t +++ b/t/db_dependent/Biblio.t @@ -273,6 +273,17 @@ sub run_tests { is( ( $marcflavour eq 'UNIMARC' && @$a2 == @$a1 + 1 ) || ( $marcflavour ne 'UNIMARC' && @$a2 == @$a1 + 3 ), 1, 'Check the number of returned notes of GetMarcNotes' ); + + # test for GetMarcUrls + $marc_record->append_fields( + MARC::Field->new( '856', '', '', u => ' https://koha-community.org ' ), + MARC::Field->new( '856', '', '', u => 'koha-community.org' ), + ); + my $marcurl = GetMarcUrls( $marc_record, $marcflavour ); + is( @$marcurl, 2, 'GetMarcUrls returns two URLs' ); + like( $marcurl->[0]->{MARCURL}, qr/^https/, 'GetMarcUrls did not stumble over a preceding space' ); + ok( $marcflavour ne 'MARC21' || $marcurl->[1]->{MARCURL} =~ /^http:\/\//, + 'GetMarcUrls prefixed a MARC21 URL with http://' ); } sub get_title_field { @@ -327,21 +338,21 @@ sub create_issn_field { } subtest 'MARC21' => sub { - plan tests => 31; + plan tests => 34; run_tests('MARC21'); $schema->storage->txn_rollback; $schema->storage->txn_begin; }; subtest 'UNIMARC' => sub { - plan tests => 31; + plan tests => 34; run_tests('UNIMARC'); $schema->storage->txn_rollback; $schema->storage->txn_begin; }; subtest 'NORMARC' => sub { - plan tests => 31; + plan tests => 34; run_tests('NORMARC'); $schema->storage->txn_rollback; $schema->storage->txn_begin; -- 2.39.5