Bug 18200: Fix a potential issue with preceding space in GetMarcUrls

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 <m.de.rooy@rijksmuseum.nl>

Followed test plan, works as expected
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

diff --git a/C4/Biblio.pm b/C4/Biblio.pm
index f475056..d52d780 100644 (file)
--- 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 9b3cbe0..c6613df 100755 (executable)
--- 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;