From ed292411a5d883f9c985131768486d669b04f03b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Cohen=20Arazi?= Date: Tue, 30 Sep 2025 12:09:48 -0300 Subject: [PATCH] Bug 40911: Fix SIP Patron->new() warnings with undefined/empty patron_id This patch prevents warnings when C4::SIP::ILS::Patron->new() receives undefined, empty, or whitespace-only patron_id values that could occur from faulty plugins or malformed SIP requests. Changes: - Add early validation for undefined/empty patron_id before processing - Add validation for hashref keys to ensure they contain valid values - Add better debug logging for empty hashref scenarios - Prevent potential siplog formatting issues with hashref patron_id - No functional changes - behavior preserved for valid patron identifiers Test plan: 1. Apply the regression tests 2. Run: $ ktd --shell k$ prove t/db_dependent/SIP/Patron.t => FAIL: Tests fail! Warning validation expects no warnings but warnings occur 3. Apply this patch 4. Repeat step 2 => SUCCESS: Tests pass! No warnings generated for invalid input 5. Verify no warnings in SIP logs for malformed requests 6. Sign off :-D --- C4/SIP/ILS/Patron.pm | 37 ++++++++++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/C4/SIP/ILS/Patron.pm b/C4/SIP/ILS/Patron.pm index 1caf2888fe..31b8310608 100644 --- a/C4/SIP/ILS/Patron.pm +++ b/C4/SIP/ILS/Patron.pm @@ -49,21 +49,44 @@ sub new { my $type = ref($class) || $class; my $self; + # Return early if patron_id is undefined, empty, or whitespace-only + if ( !defined $patron_id + || ( !ref($patron_id) && ( $patron_id eq '' || $patron_id =~ /^\s*$/ ) ) ) + { + siplog( "LOG_DEBUG", "new ILS::Patron: empty or undefined patron_id" ); + return; + } + my $patron; if ( ref $patron_id eq "HASH" ) { - if ( $patron_id->{borrowernumber} ) { - $patron = Koha::Patrons->find( $patron_id->{borrowernumber} ); - } elsif ( $patron_id->{cardnumber} ) { - $patron = Koha::Patrons->find( { cardnumber => $patron_id->{cardnumber} } ); - } elsif ( $patron_id->{userid} ) { - $patron = Koha::Patrons->find( { userid => $patron_id->{userid} } ); + + # Validate hash has at least one defined, non-empty value + my $has_valid_key = 0; + if ( defined $patron_id->{borrowernumber} && $patron_id->{borrowernumber} ne '' ) { + $patron = Koha::Patrons->find( $patron_id->{borrowernumber} ); + $has_valid_key = 1; + } elsif ( defined $patron_id->{cardnumber} && $patron_id->{cardnumber} ne '' ) { + $patron = Koha::Patrons->find( { cardnumber => $patron_id->{cardnumber} } ); + $has_valid_key = 1; + } elsif ( defined $patron_id->{userid} && $patron_id->{userid} ne '' ) { + $patron = Koha::Patrons->find( { userid => $patron_id->{userid} } ); + $has_valid_key = 1; + } + + # Return early if hashref has no valid search keys + unless ($has_valid_key) { + siplog( "LOG_DEBUG", "new ILS::Patron: hashref contains no valid search keys" ); + return; } } else { $patron = Koha::Patrons->find_by_identifier($patron_id); } unless ($patron) { - siplog( "LOG_DEBUG", "new ILS::Patron(%s): no such patron", $patron_id ); + + # Format patron_id safely for logging + my $patron_id_str = ref($patron_id) eq 'HASH' ? 'hashref' : $patron_id; + siplog( "LOG_DEBUG", "new ILS::Patron(%s): no such patron", $patron_id_str ); return; } $kp = $patron->unblessed; -- 2.51.0