Attachment 52785 Details for Bug 15006 – Bug 15006: [COUNTERPATCH] Timeout logic in sip_protocol_loop

[patch] Bug 15006: [COUNTERPATCH] Timeout logic in sip_protocol_loop

Bug-15006-COUNTERPATCH-Timeout-logic-in-sipprotoco.patch (text/plain), 5.11 KB, created by Marcel de Rooy on 2016-06-24 12:01:56 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Marcel de Rooy

Created: 2016-06-24 12:01:56 UTC

Size: 5.11 KB

patch

obsolete

>From 7160150cf0b877f996144c52117d73a82f5be17f Mon Sep 17 00:00:00 2001
>From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>Date: Fri, 17 Jun 2016 13:42:53 +0200
>Subject: [PATCH] Bug 15006: [COUNTERPATCH] Timeout logic in sip_protocol_loop
>Content-Type: text/plain; charset=utf-8
>
>Instead of adding yet another client timeout, I propose to use the
>policy timeout that we also communicate in send_acs_status.
>Although I agree that we perhaps better move it from institution to
>account/login level, this c/should be done on another report. We
>may have issues with existing configurations.
>
>In order to find a compromise between bug 13432 and the originally
>proposed 15006 patch, this patch allows you to put a zero in the
>policy timeout entry in the SIPconfig.
>
>The timeout logic is moved to a Sip.pm subroutine. Will add a test :)
>
>Test plan:
>[1] Check timeout after login with policy/timeout > 0.
>[2] Check timeout after login with policy/timeout = 0. (No timeout!)
>[3] Check timeout after login with no policy timeout. Fallback to service.
>[4] Check timeout after login with no policy and service timeout.
>    Should fallback to 30 seconds for both login process and after login.
>
>Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>---
> C4/SIP/SIPServer.pm |   19 ++++++++-----------
> C4/SIP/Sip.pm       |   27 ++++++++++++++++++++++++++-
> 2 files changed, 34 insertions(+), 12 deletions(-)
>
>diff --git a/C4/SIP/SIPServer.pm b/C4/SIP/SIPServer.pm
>index 1b04676..2bac91b 100755
>--- a/C4/SIP/SIPServer.pm
>+++ b/C4/SIP/SIPServer.pm
>@@ -15,7 +15,7 @@ use C4::SIP::Sip::Constants qw(:all);
> use C4::SIP::Sip::Configuration;
> use C4::SIP::Sip::Checksum qw(checksum verify_cksum);
> use C4::SIP::Sip::MsgType qw( handle login_core );
>-use C4::SIP::Sip qw( read_SIP_packet );
>+use C4::SIP::Sip qw( read_SIP_packet get_timeout );
> 
> use base qw(Net::Server::PreFork);
> 
>@@ -128,7 +128,7 @@ sub raw_transport {
>     my $strikes = 3;
> 
>     local $SIG{ALRM} = sub { die "raw_transport Timed Out!\n"; };
>-    my $timeout = $service->{timeout} || 30;
>+    my $timeout = get_timeout( $self, { transport => 1 } );
>     syslog("LOG_DEBUG", "raw_transport: timeout is %d", $timeout);
> 
>     while( $strikes && ( !$self->{account} || !$self->{account}->{id} )) {
>@@ -189,7 +189,7 @@ sub telnet_transport {
>     my $account = undef;
>     my $input;
>     my $config  = $self->{config};
>-    my $timeout = $self->{service}->{timeout} || $config->{timeout} || 30;
>+    my $timeout = get_timeout( $self, { transport => 1 } );
>     syslog("LOG_DEBUG", "telnet_transport: timeout is %s", $timeout);
> 
>     eval {
>@@ -251,7 +251,8 @@ sub sip_protocol_loop {
>     my $self = shift;
>     my $service = $self->{service};
>     my $config  = $self->{config};
>-    my $timeout = $self->{service}->{client_timeout} || $config->{client_timeout};
>+
>+    my $timeout = get_timeout( $self, { client => 1 } );
>     my $input;
> 
>     # The spec says the first message will be:
>@@ -268,12 +269,10 @@ sub sip_protocol_loop {
>     #
>     # In short, we'll take any valid message here.
>     #my $expect = SC_STATUS;
>-    local $SIG{ALRM} = sub { die "SIP Timed Out!\n"; } if $timeout;
>+    local $SIG{ALRM} = sub { die "SIP Timed Out!\n"; };
>     my $expect = '';
>     while (1) {
>-        if ($timeout) {
>-            alarm $timeout;
>-        }
>+        alarm $timeout;
>         $input = read_SIP_packet(*STDIN);
>         unless ($input) {
>             return;        # EOF
>@@ -299,9 +298,7 @@ sub sip_protocol_loop {
>         }
>         # We successfully received and processed what we were expecting
>         $expect = '';
>-        if ($timeout ) {
>-            alarm 0;
>-        }
>+        alarm 0;
>     }
> }
> 
>diff --git a/C4/SIP/Sip.pm b/C4/SIP/Sip.pm
>index a6cf97f..89ad9d1 100644
>--- a/C4/SIP/Sip.pm
>+++ b/C4/SIP/Sip.pm
>@@ -23,13 +23,14 @@ BEGIN {
> 
> 	@EXPORT_OK = qw(y_or_n timestamp add_field maybe_add add_count
> 		    denied sipbool boolspace write_msg read_SIP_packet
>+            get_timeout
> 		    $error_detection $protocol_version $field_delimiter
> 		    $last_response);
> 
> 	%EXPORT_TAGS = (
> 		    all => [qw(y_or_n timestamp add_field maybe_add
> 			       add_count denied sipbool boolspace write_msg
>-			       read_SIP_packet
>+                   read_SIP_packet get_timeout
> 			       $error_detection $protocol_version
> 			       $field_delimiter $last_response)]);
> }
>@@ -251,4 +252,28 @@ sub write_msg {
>     $last_response = $msg;
> }
> 
>+# get_timeout( $server, { transport|client => 1, fallback => some_val } );
>+#
>+# Centralizes all timeout logic from SIPServer into one routine
>+
>+sub get_timeout {
>+    my ( $server, $params ) = @_;
>+
>+    my $fallback = $params->{fallback} || 30;
>+    if( $params->{transport} ) {
>+        # We do not allow zero values here
>+        return $server->{service}->{timeout} || $fallback;
>+    } elsif( $params->{client} ) {
>+        if( exists $server->{policy}->{timeout} ) {
>+        # We do allow zero values here to indicate no timeout
>+            my $val = $server->{policy}->{timeout};
>+            $val = 0 if ! $val > 0; # just precautious
>+            return $val;
>+        }
>+        return $server->{service}->{timeout} || $fallback;
>+    } else {
>+        return $fallback;
>+    }
>+}
>+
> 1;
>-- 
>1.7.10.4

Actions: View | Diff | Splinter Review

Attachments on bug 15006: 45039 | 45583 | 45594 | 52477 | 52478 | 52479 | 52480 | 52784 | 52785 | 52786 | 52787 | 52849 | 52850 | 52851 | 52852 | 52853 | 53009 | 53010 | 53011 | 53012 | 53013 | 53014 | 53132 | 53133 | 53134 | 53135 | 53309 | 53310 | 53311 | 53312 | 53355 | 53356 | 53357 | 53358 | 53359 | 53360 | 53363 | 53364 | 53365 | 53366 | 53367 | 53368 | 53369