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 _______________________________________________ squid-dev mailing list [email protected] http://lists.squid-cache.org/listinfo/squid-dev
