[reSIProcate] HeaderNames issue

[Scott Godin]

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 at gmail.com>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 at gmail.com>
>
>> 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 at gmail.com>
>>
>>> 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 at gmail.com>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 at gmail.com>
>>>>
>>>>  sample code that illustrates this trap?
>>>>
>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://list.resiprocate.org/pipermail/resiprocate-devel/attachments/20130207/47f32902/attachment.htm>