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 <squ...@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@lists.squid-cache.org > http://lists.squid-cache.org/listinfo/squid-dev > -- Francesco _______________________________________________ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev
