[squid-dev] [PATCH] HTTP request-line parser upgrade

[Kinkie]

Hi,
  I am not sure whether this is the most uptodate version of the
patch; I'm auditing lp:~yadi/squid/parser-ng-requestline revno 13879.

Short story, I can find nothing obviously evil with it - code is well
documented and intent is clear; as far as I understand Polygraph,
Coadvisor and my casual testing all agree that it introduces no
regressions so for me it could go in right away.

Only suggestion (non-binding):

in HttpRequestMethod::HttpRequestMethod:
 // TODO: Optimize this linear search
 I suspect this code path is rather hot, and I'd recommend to use a
trie, a hash or a map here ASAP

Good job!


On Fri, Feb 6, 2015 at 4:08 PM, Amos Jeffries <squid3 at treenet.co.nz> wrote:
> This patch converts the request-line parse method from a char* string
> parser to using ::Parser::Tokenizer based processing.
>
> * the characters for each token are now limited to the RFC 7230
> compliant values. The URI is taken as a whole token and characters which
> are valid in only one sub-token segment are accepted regardless of their
> position. In relaxed parse that is extended beyond the valid URI
> characters to include the whitespace characters.
>
> * whitespace tolerance is extended to include "binary" whitespace VTAB,
> HTAB, CR and FF characters specified in RFC 7230.
>
> * The Squid specific tolerance for whitespace prefix to method is
> removed. RFC 2730 clarifies that tolerance before request-line is
> specfifically and only for whole empty lines (sequences of CRLF or LF).
>
> * The unit tests are extended to check strict and relaxed parse within
> the new characterset limits. Drip-feed incremental test updated to check
> both parser modes explicitly.
>
>
> * ::Parser:Tokenizer is extended with methods to skip or retrieve a
> token at the suffix of the stored buffer. This is used by the whitespace
> tolerant parse to process the URL and HTTP-version tokens from the line
> "backwards" from the LF position.
>
>
> CoAdvisor and Polygraph show no diffrence from trunk. Which is expected
> since coadvisor does not test RFC 7230 edge cases (yet), and polygraph
> is not stressing incremental parse capabilities.
>
> Amos
>
> _______________________________________________
> squid-dev mailing list
> squid-dev at lists.squid-cache.org
> http://lists.squid-cache.org/listinfo/squid-dev
>



-- 
    Francesco