[reSIProcate] ParseBuffer::assertNotEof() ?

[Jason Fischl]

comments inline.

On 9/29/06, Byron Campen <bcampen at estacado.net> 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).
>
The current design leaves it up to the TU to catch parse exceptions
and to send a failure response to the request as a result. I agree
this is far from ideal.


>         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!
>
The above comment is not really correct. resip has had a lazy parser
since the beginning. Any application code that triggers parsing needs
to check for parse exceptions. In many cases, it should be possible to
put a single try/catch that handles ParseExceptions somewhere fairly
high up in a ThreadIf subclass main loop.  I think we can safely put a
try/catch in DialogUsageManager::internalProcess that can handle lots
of these cases and automatically send a failure response when a parse
exception occurs as a result of an inbound sip request. This should
catch most of these issues.

>         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!
>
Adding isWellFormed() to ParserCategory seems like a good idea.

>         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.
>
In general this is not a good idea. Trigger a parse of all header
fields including all of the headers that you don't care about can make
your application less tolerant. For example, most proxies only need to
look at a handful of headers and none of the bodies.

> 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 at sip.callshop.lvdx.com>;tag=\r\nTo:
> > <sip:11 at sip.callshop.lvdx.com>;tag=\r\nContact:
> > <sip:11 at 83.146.x.x:5070>\r\nSupported: replaces\r\nProxy-
> > Authorization:
> > Digest username=\"11\", realm=\"sip."..., mPosition = 0x809dd76 "\r
> > \nTo:
> > <sip:11 at sip.callshop.lvdx.com>;tag=\r\nContact:
> > <sip:11 at 83.146.x.x: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 at sip.callshop.lvdx.com>;tag=\r\nContact:
> > <sip:11 at 83.146.x.x: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 at list.sipfoundry.org
> > https://list.sipfoundry.org/mailman/listinfo/resiprocate-devel
>
>
>
> _______________________________________________
> resiprocate-devel mailing list
> resiprocate-devel at list.sipfoundry.org
> https://list.sipfoundry.org/mailman/listinfo/resiprocate-devel
>
>
>