On 08/23/2016 03:26 AM, Eduard Bagdasaryan wrote: > 2016-08-23 3:08 GMT+03:00 Alex Rousskov: >> I do not understand why you decided to use maxMethodLength in >> parseRequestFirstLine(). AFAICT, parseMethodField() already does >> everything we need: It logs an error message and sets parseStatusCode >> accordingly.
> Yes, it does partly what we need but it does not parse the delimiter( > and inform about possible failure). ... which is unrelated to the maxMethodLength move I questioned above. > skipDelimiter() does this with message: > >> invalid request-line: missing delimiter > > which does not hint that our case is a 'bad' method If the method parses OK but the delimiter is missing, then our case can be considered to be a case of a "missing delimiter" rather than of a "bad method". It is all relative, of course: If a request line starts with a few token characters, then it is impossible to say whether the method suffix is invalid or the following delimiter -- it takes two to tango. > and less informative than: > >> invalid request-line: method exceeds 32-byte limit How is a "missing delimiter" error is less informative than "method exceeds 32-byte limit", especially when the method does not actually exceed that limit? [Rhetorical] > I followed your sketch and refreshed the patch. > + debugs(74, ErrorLevel(), "invalid request-line exceeds " << > + Config.maxRequestHeaderSize << "-byte limit"); bzr grep "invalid request-line" src s/request-line/request-line: URI/ for consistency and clarity sake. > + const CharacterSet &delimiters = DelimiterCharacters(); I wonder whether we should make this variable static to avoid repeated function calls on a performance-sensitive code path. Same for the old "delimiters" variable left inside parseRequestFirstLine(), of course. These polishing touches can be done during commit. +1 Alex. _______________________________________________ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev