[squid-dev] [PATCH] suffix parsing and skipping

[Amos Jeffries]

On 24/07/2015 1:55 a.m., Alex Rousskov wrote:
> On 07/23/2015 01:46 AM, Kinkie wrote:
> 
>> the only thing I don't understand is the XXX in
>> parsedSize(); it's not clear why the comment, 
> 
> The parsedSize() method is broken: It claims to return the number of
> bytes parsed, but it does not return that. It usually returns the number
> of bytes parsed from the left, but not from the right of the string. For
> example,
> 
>   skipOne()
>   skipOneTrailing()
> 
> should usually result in parsedSize() returning 2 because 2 bytes were
> parsed and removed from the string. It usually returns 1.
> 
> 
>> and what to use instead.
> 
> There is currently no good alternative. Discussing the best way forward
> is outside this project scope (but I welcome such a separate discussion,
> of course). Fortunately, no existing code misuses parsedSize() today AFAIK.
> 
> 
>> as soon as that's improved, +1.
> 
> Why?! This is not something the patch breaks. Do you want me to remove
> the XXX and just pretend that we have not discovered the problem?

The parsedSize() function is used by Parser to measure the HTTP message
size for testing whether the message has exceeded the admin configured
limits.

 The reverse-parse bytes is not worth accounting in current trunk code
because the incremental HTTP Parser is always using its primary
Tokenizer forwards. The special-case reverse-parse uses a separate
temporary Tokenizer is only used on a sub-section of line accounted for
by the LF seek pass.

That changes if the proposed Parser refactor Alex has in parallel gets
merged. Since it depends on sizes of things produced by the temporary
reverse Tokenizer. But I note the bug is not fixed in that patch either.


I -0 right now for reasons of minor new bug I found in the patch
already. Audit partially completed with details to follow.

Amos