Re: parsing quoted-string HTTP header fields

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.

Amos

-- 
Please be using
   Current Stable Squid 2.7.STABLE9 or 3.1.12
   Beta testers wanted for 3.2.0.7 and 3.1.12.1