< Previous by Date Date Index Next by Date >
< Previous in Thread Thread Index Next in Thread >

Re: [reSIProcate] ParseBuffer::assertNotEof() ?



Looking at my own case below, is the packet with an empty `tag=' parameter valid?

To: "Booth2" <sip:11@xxxxxxxxxxxxxxxxxxxxx>;tag=

Line 28 of DataParameter.cxx calls pb.assertNotEof(); - could the following be done safely:

if(pb.eof()) {
 mValue = Data("");
 return;
}

or is there some other code that depends on the parameter having a value?


Byron Campen wrote:

This sounds like a bug in dum. If an exception due to a parse failure is making it out of dum, then it is very likely the stack has leaked a TransactionState. If the exception is not caught in the appropriate place, not only is the SipMessage leaked, but there is no way to respond to it, meaning the TransactionState will never get cleaned up either (the stack waits indefinitely for the TU to respond).

Unfortunately, there are many such bugs, because a lot of the code was written before we transitioned to a lazy-parser, and the code was never fixed. The upshot is that calling something like msg.header (h_HeaderName).someAccessor() will probably be what triggers the parse of the header, and if that header is malformed, kaboom!

Fixing all the instances of this will take some time. I tried my best to clear up any errors of this type that existed in the stack (and I think I've gotten them all). I think that, at this point, it would make sense to add bool LazyParser::isWellFormed(), so we can check whether the header is well-formed in an exception-safe manner. This will make fixing the existing code a little easier. And for those who are writing new code, keep this in mind!

Now, I added a bandage for this problem that people can use in the meantime. As a configure option, you can set PEDANTIC_STACK to true, which cause the stack to do a full parse of SipMessages on receipt. If a problem is discovered, it will be rejected immediately. (ie, no potential for a parse to fail later down the line, since everything is already parsed) This is an expensive solution, but at least you have the option.

Best regards,
Byron Campen



Occasionally, my application is receiving a packet which causes
assertNotEof() to throw an exception.

How is this exception meant to be dealt with? Am I intended to catch it
with a try { } catch { } block around dum->process()?  Or should this
exception be caught and handled (presumably by discarding the packet)
within the stack?

Here is the content of pb when the exception is thrown, the code in
DataParameter.cxx suggests the empty ;tag= is the fault:

(gdb) print pb
$1 = (class resip::ParseBuffer &) @0xbfffe114: {
  static Whitespace = 0xb7ac4264 " \t\r\n",
  static ParamTerm = 0xb7ac4261 ";?",
  mBuff = 0x809dd4a "\"Booth2\"
<sip:11@xxxxxxxxxxxxxxxxxxxxx>;tag=\r\nTo:
<sip:11@xxxxxxxxxxxxxxxxxxxxx>;tag=\r\nContact:
<sip:11@xxxxxxxxxx:5070>\r\nSupported: replaces\r\nProxy- Authorization:
Digest username=\"11\", realm=\"sip."..., mPosition = 0x809dd76 "\r \nTo:
<sip:11@xxxxxxxxxxxxxxxxxxxxx>;tag=\r\nContact:
<sip:11@xxxxxxxxxx:5070>\r\nSupported: replaces\r\nProxy- Authorization:
Digest username=\"11\", realm=\"sip.callshop.lvdx.com\",  algorithm=MD5,
uri=\"sip:"...,
  mEnd = 0x809dd76 "\r\nTo:
<sip:11@xxxxxxxxxxxxxxxxxxxxx>;tag=\r\nContact:
<sip:11@xxxxxxxxxx:5070>\r\nSupported: replaces\r\nProxy- Authorization:
Digest username=\"11\", realm=\"sip.callshop.lvdx.com\",  algorithm=MD5,
uri=\"sip:"...,
  mErrorContext = @0xb7dc5c18}


Regards,

Daniel
_______________________________________________
resiprocate-devel mailing list
resiprocate-devel@xxxxxxxxxxxxxxxxxxx
https://list.sipfoundry.org/mailman/listinfo/resiprocate-devel