From 3de53001d4eacb6bee9fa3a4cb1eadc7f808e615 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Fri, 27 Jun 2025 16:42:30 -0300 Subject: [PATCH] Bug 40270: Fix uninitialized value warnings in SIP logging The SIP server generates warnings when logging account information for failed logins: Use of uninitialized value $args[0] in sprintf at /kohadevbox/koha/C4/SIP/Sip.pm line 314 Use of uninitialized value $args[1] in sprintf at /kohadevbox/koha/C4/SIP/Sip.pm line 314 This occurs when $self->{account}->{id} or $self->{account}->{institution} are undefined during failed authentication attempts. This patch fixes the issue by: 1. Adding a safer sprintf implementation in siplog that handles undefined values 2. Providing default values for undefined account information in logging calls To test: 1. Start SIP server 2. Attempt to connect with invalid credentials using sip_cli_emulator.pl: ./misc/sip_cli_emulator.pl -a 127.0.0.1 -p 6001 -su invaliduser -sp wrongpass -l kohalibrary2 --patron koha -m patron_information 3. Check /var/log/koha/kohadev/sip-output.log => FAIL: 'Use of uninitialized value...' warns 4. Apply this patch 5. Repeat 2 => SUCCESS: No "Use of uninitialized value" warnings appear => SUCCESS: Logging shows 'undef/undef' for failed logins instead of empty strings 6. Try a valid login => SUCCESS: Still works correctly for valid logins 7. Sign off :-D Signed-off-by: David Nind Signed-off-by: Nick Clemens Signed-off-by: Lucas Gass (cherry picked from commit 9cad25967d0814cf78920f6f7cfb776989cacbe1) Signed-off-by: Paul Derscheid (cherry picked from commit 4e1cd02932c0df32077f04591547799552b2fc65) Signed-off-by: Fridolin Somers --- C4/SIP/SIPServer.pm | 5 +++-- C4/SIP/Sip.pm | 10 +++++++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/C4/SIP/SIPServer.pm b/C4/SIP/SIPServer.pm index 96c1809fbe..0038271ca6 100644 --- a/C4/SIP/SIPServer.pm +++ b/C4/SIP/SIPServer.pm @@ -313,8 +313,9 @@ sub raw_transport { Koha::Logger->put_mdc( "peeraddr", $self->{server}->{peeraddr} ); siplog("LOG_DEBUG", "raw_transport: uname/inst: '%s/%s'", - $self->{account}->{id}, - $self->{account}->{institution}); + $self->{account}->{id} // 'undef', + $self->{account}->{institution} // 'undef' + ); if (! $self->{account}->{id}) { siplog("LOG_ERR","Login failed shutting down"); return; diff --git a/C4/SIP/Sip.pm b/C4/SIP/Sip.pm index fa65951bd9..8d27f64af1 100644 --- a/C4/SIP/Sip.pm +++ b/C4/SIP/Sip.pm @@ -231,7 +231,15 @@ sub siplog { : $level eq 'LOG_WARNING' ? 'warn' : 'error'; - my $message = @args ? sprintf($mask, @args) : $mask; + my $message; + if (@args) { + + # Replace undefined values with 'undef' to prevent sprintf warnings + my @safe_args = map { defined($_) ? $_ : 'undef' } @args; + $message = sprintf( $mask, @safe_args ); + } else { + $message = $mask; + } $message = remove_password_from_message($message);