SIPServer

SIPServer can incorrectly calculate output message length, which can lead to clients hanging

Bug #1628995 reported by Jeff Godin
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
SIPServer
Confirmed
Medium
Jeff Godin

Bug Description

When writing an output message, SIPServer can incorrectly calculate the length of the message, resulting in an incomplete message being sent. Since the message lacks the SIP2 Message Terminator character (CR, 0x0d), client behavior is unpredictable. Some clients will hang.

The issue seems to stem from a bytes vs characters mismatch here:

my $rv = POSIX::write(fileno(STDOUT), $outmsg, length($outmsg))

One common trigger is poorly-encoded characters present in a title or author field, either due to malformed marcxml or due to a potential encoding issue elsewhere (in the MODS transform used for mvr objects).

This probably impacts the output message corresponding with most messages that expect an author or title, including (but probably not limited to):

17
64 + fine_details
64 + hold_items
64 + unavail_holds

Revision history for this message
Jeff Godin (jgodin) wrote :

A properly placed "use bytes;" appears to result in length() returning a correct value, and does not appear to create other side effects.

Another fix for this may be related to work being done in bug 1542495 and/or bug 1463943, which may change the way that encodings are handled in general within SIPServer.

Revision history for this message
Jeff Godin (jgodin) wrote :

The "use bytes;" approach is in working branch user/jeff/lp1628995_fix_output_message_length_calculation

http://git.evergreen-ils.org/?p=working/SIPServer.git;a=shortlog;h=refs/heads/user/jeff/lp1628995_fix_output_message_length_calculation

Revision history for this message
Jeff Godin (jgodin) wrote :

Acknowledging here that the bytes module documentation states: "use of this module for anything other than debugging purposes is strongly discouraged."

We know that we have some work to do with regard to our encoding approach, and the hope is that that more comprehensive work will obsolete this less-than-elegant fix.

After conversation with miker and others, I'm in favor of trying the alternate approach of "require bytes;" followed by calling bytes::length() instead of length().

Working branch with that approach and an appropriate set of comments forthcoming.

Revision history for this message
Jason Stephenson (jstephenson) wrote :

I belatedly set the status to confirmed, since I did confirm it, not only with a test client, but also via tcpdump looking at the packet contents. The message is, indeed, short when there are certain multi-byte character sequences.

Changed in sipserver:
status: New → Confirmed
Revision history for this message
Jason Stephenson (jstephenson) wrote :

BTW, I think my fixes for bug 1542495 and bug 1463943 also resolve this issue.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.