Re: parsing quoted-string HTTP header fields

From: Alex Rousskov <rousskov_at_measurement-factory.com>
Date: Tue, 31 May 2011 10:30:20 -0600 (MDT)

On Tue, 31 May 2011, Amos Jeffries wrote:

> On 31/05/11 01:40, Tsantilas Christos wrote:
>> This is the third patch.
>>
>> In this patch also solved a small buffer overread which exist in the
>> original httpHeaderParseQuotedString function. The loop:
>>
>> while (end <= (start+len) && *end != '\\' && *end != '\"' && *end > 0x1F
>> && *end != 0x7F)
>> if (*end <= 0x1F || *end == 0x7F) {
>> ...
>> allowed to access (and parsing affected by) the char after the end of
>> parsed string. It did not have any bad effect for null terminated strings.
>>
>>
>> On 05/27/2011 03:12 PM, Amos Jeffries wrote:
>>> On 27/05/11 23:21, Tsantilas Christos wrote:
>>>> Hi all,
>>>>
>>>> Just trying to clarify what we want to implement at the end, because I
>>>> am confused. I am responsible for the confusion because I give two "(3)"
>>>> options, and I send buggy implementations for the "(1)" and the "second
>>>> (3)" option.
>>>>
>>>> From what I can understand, currently, we have the following options:
>>>> 1) Just ignore any "\r" or "\n" character. This is the fastest and
>>>> simpler approach
>>>> 2) Require "[\r]\n " or "[\r]\n\t" as line separator and replace it with
>>>> a space.
>>>>
>>>> From the discussion the (1) may be dangerous because strings like this
>>>> "1\r23" will be converted to "123" which maybe it is dangerous.
>>>>
>>>> So I suppose we should implement the (2) option. Is it OK?
>>>
>>> Agreed.
>>>
>>> What we have been debugging in the other half of the thread was "\r\n "
>>> or "\r\n\t".
>>>
>>> I think it just needs:
>>> * the two buffer overread bugs Alex spotted removed,
>>> * the \r made optional.
>>>
>>> Amos
>>
>
> You don't have to escape ' in the debugs(..., \'\\r\' ) just the \\r.
>
> Please put "HERE" into the debugs for tracking.
>
>
> Looks okay to me. Passes the new criteria.

I did not notice any problems except for quoting mentioned by Amos
above.

Perhaps you can add a TODO comment to replace the entire LWS.

Cheers,

Alex.
Received on Tue May 31 2011 - 16:30:21 MDT

This archive was generated by hypermail 2.2.0 : Wed Jun 01 2011 - 12:00:07 MDT