*bump*
If there are no objections I will merge this to trunk.
Amos
On 6/01/2014 4:21 p.m., Amos Jeffries wrote:
> Updated patch attached. It mostly follows the plan you laid out (I was
> partially through an almost identical plan already, I merged the two
> from where I was up to).
>
> We now have a two-class HTTP Parser hierarchy:
> * base class Http1::Parser
> - which contains the generic (but HTTP/1.x specific) parse members and
> accessors
>
> * child class Http1::RequestParser
> - which contains the reuqest-specific state members and parse logics
> necessary to extend Http1::Parser into a HTTP Request message parser.
>
> The parser still only parses the request-line and greps the mime headers
> as a single token/blob into an SBuf for later old-parser stages to work
> with.
>
>
>
> On 4/01/2014 5:38 a.m., Alex Rousskov wrote:
>>
>>> /* NP: don't be tempted to move this down or remove again.
>>> * It's the only DDoS protection old-String has against long URL */
>>> if ( hp->bufsiz <= 0) {
>>> debugs(33, 5, "Incomplete request, waiting for end of request
>>> line");
>>> return NULL;
>>> } else if ( (size_t)hp->bufsiz >= Config.maxRequestHeaderSize &&
>>> headersEnd(hp->buf, Config.maxRequestHeaderSize) == 0) {
>>> debugs(33, 5, "parseHttpRequest: Too large request");
>>> hp->request_parse_status = Http::scHeaderTooLarge;
>>> return parseHttpRequestAbort(csd, "error:request-too-large");
>>> }
>>
>> BTW, the above should be moved into the RequestParser code, with an
>> adjusted comment, and headersEnd() call moved/merged into the overall
>> parsing logic.
>
> I have done this in the patch as well while making the garbage prefix
> whitespace incrementally parse+discard.
>
> Although I am not yet 100% certain that I got all the places in the code
> shuffling the I/O buffer based on ClientHttpRequest::req_sz (which now
> omits any prefix garbage bytes and has already been consumed). Testing
> for the last day in regular traffic seems to show things fully working.
>
> Amos
>
