[reSIProcate] HeaderNames issue

[Yannick Guay]

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


2013/2/7 Scott Godin <slgodin at gmail.com>

> 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/e9b9670e/attachment.htm>