Bugzilla – Attachment 187110 Details for
Bug 40911
SIP Patron->new() generates ORM warnings with undefined/empty patron_id
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 40911: Fix SIP Patron->new() warnings with undefined/empty patron_id
Bug-40911-Fix-SIP-Patron-new-warnings-with-undefin.patch (text/plain), 3.62 KB, created by
Tomás Cohen Arazi (tcohen)
on 2025-09-30 15:16:10 UTC
(
hide
)
Description:
Bug 40911: Fix SIP Patron->new() warnings with undefined/empty patron_id
Filename:
MIME Type:
Creator:
Tomás Cohen Arazi (tcohen)
Created:
2025-09-30 15:16:10 UTC
Size:
3.62 KB
patch
obsolete
>From ed292411a5d883f9c985131768486d669b04f03b Mon Sep 17 00:00:00 2001 >From: =?UTF-8?q?Tom=C3=A1s=20Cohen=20Arazi?= <tomascohen@theke.io> >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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 40911
:
187109
| 187110