Attachment 173502 Details for Bug 37582 – Bug 37582: Add ability to for SIP to convert control and separator characters to spaces

[patch] Bug 37582: Add ability to for SIP to convert control and separator characters to spaces

Bug-37582-Add-ability-to-for-SIP-to-convert-contro.patch (text/plain), 9.62 KB, created by Martin Renvoize (ashimema) on 2024-10-28 10:39:49 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Renvoize (ashimema)

Created: 2024-10-28 10:39:49 UTC

Size: 9.62 KB

patch

obsolete

>From 6f9ac59ca8fe06a08d9c66abc1d8b3bc88ce11c2 Mon Sep 17 00:00:00 2001
>From: Kyle M Hall <kyle@bywatersolutions.com>
>Date: Tue, 17 Sep 2024 18:28:11 +0000
>Subject: [PATCH] Bug 37582: Add ability to for SIP to convert control and
> separator characters to spaces
>
>This came up with a SIP vendor recently - when reading responses they were only getting part of a patron's information.
>It seemed to have been caused by patrons with multiple restrictions and a new line in the response
>
>This patch also does minor refactoring to write_msg to pass the server variable that contains
>all the previously passed parameters, and removes the never used "file" parameter.
>
>Test Plan:
>1) Add multiple debarments to a patron
>2) Run a sip patron information response, note the presence of newlines
>3) Apply this patch
>4) Add the new option convert_control_characters to your SIP account,
>   set the value to " -- "
>5) Restart all the things!
>7) Run a sip patron information response, note the newlines have been replaced!
>
>Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
>Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>---
> C4/SIP/Sip.pm         | 19 +++++++++----------
> C4/SIP/Sip/MsgType.pm | 34 +++++++++++++++++-----------------
> etc/SIPconfig.xml     |  2 ++
> 3 files changed, 28 insertions(+), 27 deletions(-)
>
>diff --git a/C4/SIP/Sip.pm b/C4/SIP/Sip.pm
>index c29bff282ff..cfaa202dac7 100644
>--- a/C4/SIP/Sip.pm
>+++ b/C4/SIP/Sip.pm
>@@ -187,11 +187,16 @@ sub boolspace {
> #
> 
> sub write_msg {
>-    my ($self, $msg, $file, $terminator, $encoding) = @_;
>+    my ($self, $msg, $server) = @_;
>+    my $terminator = $server->{account}->{terminator};
>+    my $encoding = $server->{account}->{encoding};
> 
>     $terminator ||= q{};
>     $terminator = ( $terminator eq 'CR' ) ? $CR : $CRLF;
> 
>+    my $separator = $server->{account}->{convert_nonprinting_characters};
>+    $msg =~ s/[^[:print:]]/$separator/g if defined $separator;
>+
>     $msg = encode($encoding, $msg) if ( $encoding );
> 
>     my $cksum;
>@@ -206,15 +211,9 @@ sub write_msg {
>         $msg .= sprintf('%04.4X', $cksum);
>     }
> 
>-
>-    if ($file) {
>-        $file->autoflush(1);
>-        print $file $msg, $terminator;
>-    } else {
>-        STDOUT->autoflush(1);
>-        print $msg, $terminator;
>-        siplog("LOG_INFO", "OUTPUT MSG: '$msg'");
>-    }
>+    STDOUT->autoflush(1);
>+    print $msg, $terminator;
>+    siplog("LOG_INFO", "OUTPUT MSG: '$msg'");
> 
>     $last_response = $msg;
> }
>diff --git a/C4/SIP/Sip/MsgType.pm b/C4/SIP/Sip/MsgType.pm
>index 12bfd526f5d..6ad48672fd1 100644
>--- a/C4/SIP/Sip/MsgType.pm
>+++ b/C4/SIP/Sip/MsgType.pm
>@@ -509,7 +509,7 @@ sub handle_patron_status {
>         $koha_patron->update_lastseen('connection');
>     }
>     $resp = build_patron_status( $patron, $lang, $fields, $server );
>-    $self->write_msg( $resp, undef, $server->{account}->{terminator}, $server->{account}->{encoding} );
>+    $self->write_msg( $resp, $server );
>     return (PATRON_STATUS_REQ);
> }
> 
>@@ -649,7 +649,7 @@ sub handle_checkout {
>         }
>     }
> 
>-    $self->write_msg( $resp, undef, $server->{account}->{terminator}, $server->{account}->{encoding} );
>+    $self->write_msg( $resp, $server );
>     return (CHECKOUT);
> }
> 
>@@ -748,7 +748,7 @@ sub handle_checkin {
>     $resp .= maybe_add( FID_SCREEN_MSG, $status->screen_msg, $server );
>     $resp .= maybe_add( FID_PRINT_LINE, $status->print_line, $server );
> 
>-    $self->write_msg( $resp, undef, $server->{account}->{terminator}, $server->{account}->{encoding} );
>+    $self->write_msg( $resp, $server );
> 
>     return (CHECKIN);
> }
>@@ -795,7 +795,7 @@ sub handle_block_patron {
>     }
> 
>     $resp = build_patron_status( $patron, $patron->language, $fields, $server );
>-    $self->write_msg( $resp, undef, $server->{account}->{terminator}, $server->{account}->{encoding} );
>+    $self->write_msg( $resp, $server );
>     return (BLOCK_PATRON);
> }
> 
>@@ -836,7 +836,7 @@ sub handle_request_acs_resend {
> 
>         # We haven't sent anything yet, so respond with a
>         # REQUEST_SC_RESEND msg (p. 16)
>-        $self->write_msg( REQUEST_SC_RESEND, undef, $server->{account}->{terminator}, $server->{account}->{encoding} );
>+        $self->write_msg( REQUEST_SC_RESEND, $server );
>     } elsif ( ( length($last_response) < 9 )
>         || substr( $last_response, -9, 2 ) ne 'AY' ) {
> 
>@@ -850,7 +850,7 @@ sub handle_request_acs_resend {
>         # Cut out the sequence number and checksum, since the old
>         # checksum is wrong for the resent message.
>         my $rebuilt = substr( $last_response, 0, -9 );
>-        $self->write_msg( $rebuilt, undef, $server->{account}->{terminator}, $server->{account}->{encoding} );
>+        $self->write_msg( $rebuilt, $server );
>     }
> 
>     return REQUEST_ACS_RESEND;
>@@ -932,7 +932,7 @@ sub handle_login {
>         $status = login_core( $server, $uid, $pwd );
>     }
> 
>-    $self->write_msg( LOGIN_RESP . $status, undef, $server->{account}->{terminator}, $server->{account}->{encoding} );
>+    $self->write_msg( LOGIN_RESP . $status, $server );
>     return $status ? LOGIN : '';
> }
> 
>@@ -1110,7 +1110,7 @@ sub handle_patron_info {
>         $resp .= maybe_add( FID_SCREEN_MSG, INVALID_CARD, $server );
>     }
> 
>-    $self->write_msg( $resp, undef, $server->{account}->{terminator}, $server->{account}->{encoding} );
>+    $self->write_msg( $resp, $server );
>     return (PATRON_INFO);
> }
> 
>@@ -1137,7 +1137,7 @@ sub handle_end_patron_session {
>     $resp .= maybe_add( FID_SCREEN_MSG, $screen_msg, $server );
>     $resp .= maybe_add( FID_PRINT_LINE, $print_line, $server );
> 
>-    $self->write_msg( $resp, undef, $server->{account}->{terminator}, $server->{account}->{encoding} );
>+    $self->write_msg( $resp, $server );
> 
>     return (END_PATRON_SESSION);
> }
>@@ -1219,7 +1219,7 @@ sub handle_fee_paid {
>     $resp .= maybe_add( FID_SCREEN_MSG,     $status->screen_msg, $server );
>     $resp .= maybe_add( FID_PRINT_LINE,     $status->print_line, $server );
> 
>-    $self->write_msg( $resp, undef, $server->{account}->{terminator}, $server->{account}->{encoding} );
>+    $self->write_msg( $resp, $server );
> 
>     return (FEE_PAID);
> }
>@@ -1312,7 +1312,7 @@ sub handle_item_information {
>         $resp .= $item->build_custom_field_string( $server );
>     }
> 
>-    $self->write_msg( $resp, undef, $server->{account}->{terminator}, $server->{account}->{encoding} );
>+    $self->write_msg( $resp, $server );
> 
>     return (ITEM_INFORMATION);
> }
>@@ -1362,7 +1362,7 @@ sub handle_item_status_update {
>     $resp .= maybe_add( FID_SCREEN_MSG, $status->screen_msg, $server );
>     $resp .= maybe_add( FID_PRINT_LINE, $status->print_line, $server );
> 
>-    $self->write_msg( $resp, undef, $server->{account}->{terminator}, $server->{account}->{encoding} );
>+    $self->write_msg( $resp, $server );
> 
>     return (ITEM_STATUS_UPDATE);
> }
>@@ -1416,7 +1416,7 @@ sub handle_patron_enable {
> 
>     $resp .= add_field( FID_INST_ID, $ils->institution, $server );
> 
>-    $self->write_msg( $resp, undef, $server->{account}->{terminator}, $server->{account}->{encoding} );
>+    $self->write_msg( $resp, $server );
> 
>     return (PATRON_ENABLE);
> }
>@@ -1481,7 +1481,7 @@ sub handle_hold {
>     $resp .= maybe_add( FID_SCREEN_MSG, $status->screen_msg, $server );
>     $resp .= maybe_add( FID_PRINT_LINE, $status->print_line, $server );
> 
>-    $self->write_msg( $resp, undef, $server->{account}->{terminator}, $server->{account}->{encoding} );
>+    $self->write_msg( $resp, $server );
> 
>     return (HOLD);
> }
>@@ -1568,7 +1568,7 @@ sub handle_renew {
>     $resp .= maybe_add( FID_SCREEN_MSG, $status->screen_msg, $server );
>     $resp .= maybe_add( FID_PRINT_LINE, $status->print_line, $server );
> 
>-    $self->write_msg( $resp, undef, $server->{account}->{terminator}, $server->{account}->{encoding} );
>+    $self->write_msg( $resp, $server );
> 
>     return (RENEW);
> }
>@@ -1621,7 +1621,7 @@ sub handle_renew_all {
>     $resp .= maybe_add( FID_SCREEN_MSG, $status->screen_msg, $server );
>     $resp .= maybe_add( FID_PRINT_LINE, $status->print_line, $server );
> 
>-    $self->write_msg( $resp, undef, $server->{account}->{terminator}, $server->{account}->{encoding} );
>+    $self->write_msg( $resp, $server );
> 
>     return (RENEW_ALL);
> }
>@@ -1727,7 +1727,7 @@ sub send_acs_status {
> 
>     # Do we want to tell the terminal its location?
> 
>-    $self->write_msg( $msg, undef, $server->{account}->{terminator}, $server->{account}->{encoding} );
>+    $self->write_msg( $msg, $server );
>     return 1;
> }
> 
>diff --git a/etc/SIPconfig.xml b/etc/SIPconfig.xml
>index a1432b13f34..22f2f0faf03 100644
>--- a/etc/SIPconfig.xml
>+++ b/etc/SIPconfig.xml
>@@ -59,6 +59,7 @@
>       <login id="lpl-sc" password="1234" institution="LPL" allow_fields="AO,AA,AE"/>
>       
>       <login id="lpl-sc-beacock" password="xyzzy"
>+             convert_nonprinting_characters=" "
>              delimiter="|" error-detect="enabled" institution="LPL"
>              send_patron_home_library_in_af="1"
>              cv_send_00_on_success="1"
>@@ -84,6 +85,7 @@
>              lost_status_for_missing="4"
>              blocked_item_types="VM|MU"
>              seen_on_item_information="mark_found"> 
>+             
>              
>              
>              
>-- 
>2.47.0

Actions: View | Diff | Splinter Review

Attachments on bug 37582: 171642 | 171643 | 171644 | 173081 | 173082 | 173306 | 173502 | 173503 | 173712 | 173715