Re: [reSIProcate] HeaderNames issue

[Yannick Guay <yannick.guay@xxxxxxxxx>]

right, this is more or less the same as what I had here. thanks!

2013/2/7 Scott Godin <slgodin@xxxxxxxxx>

Ah - in this case the Header type is NONE and not UNKNOWN.  NONE is beyond MAX_HEADERS and the HeaderNames array is not large enough to have a slot for NONE.  I have fixed this issue by putting a check in Headers::getHeaderName:

const Data&

Headers::getHeaderName(int type)

{

   if(type < MAX_HEADERS)

   {

      return HeaderNames[type+1];

   }

   else

   {

      return Data::Empty;

   }

}

I will commit this fix.  Thanks for pointing it out.

Scott

On Thu, Feb 7, 2013 at 12:34 PM, Yannick Guay <yannick.guay@xxxxxxxxx> wrote:

Hi again,

I finally found the combination of code and packet that cause the segmentation fault crash. I've attached a revised version of the code snippet that causes the issue.

I understand that this type of faulty packet is sort of unlikely to happen in a real world. It would be better if resiprocate was able to deal with it appropriately and throw the exception as expected.

I'm hoping to receive guidance as to how Data Headers::HeaderNames[ ] could be initialized appropriately to be able to deal with resip::Headers::NONE.

best regards,

-Yannick

2013/2/7 Yannick Guay <yannick.guay@xxxxxxxxx>

Hi Scott,

sorry that it took me so long to get back to you, I was swamped with other things here. 

As you said, the crash must have been caused by something completely different as I cannot reproduce it anymore.

I will revert the temporary changes I had made here in our local branch and keep an eye on it just in case it comes back. Will keep you posted if/when I have more details.

thanks for looking into it.

regards,

-Yannick

2013/1/28 Scott Godin <slgodin@xxxxxxxxx>

Hi Yannick,

I ran the test code you posted against the trunk revision and did not reproduce a crash.  The Parse Exception threw as expected, but it did not crash.  When I look at the code the HeaderName array, is an array of Data objects - these objects self initialize to an empty string.  From what I can tell, when you create a URI object that is not associated with any header the HeaderType is set to Headers::UNKNOWN (-1) - see Uri.cxx constructor line 60.  When Headers::getHeaderName is called it adds 1 to the header type and references it in the Headers array.  This should just access a Data object that represents an empty string.  I'm not seeing the issue.  Perhaps something else is causing memory corruption in your copy of the code?

Scott

On Fri, Jan 18, 2013 at 2:56 PM, Yannick Guay <yannick.guay@xxxxxxxxx> wrote:

Hi Scott,

Please find, attached a snippet of code that triggers the crash. Below is the exact location where it crashes:

File name: rutil/ParseBuffer.cxx

Method name: ParseBuffer::fail(const char* file, unsigned int line, const Data& detail) const  

Line number: 957

The version of resiprocate we're using here at Cassisian is 1.8.5. As well, please note the "tgrp=" user parameter which is empty in the snippet, this is what triggers the segfault, feeding ParseBuffer with a non-empty user parameter makes the bug go away.

best regards,

-Yannick

2013/1/17 <slgodin@xxxxxxxxx>

sample code that illustrates this trap?