Re: [PATCH] new request-line parser and unit tests

On 08/31/2010 04:33 PM, Amos Jeffries wrote:
> On Tue, 31 Aug 2010 09:47:18 -0600, Alex Rousskov
> <rousskov_at_measurement-factory.com> wrote:

>> * "// for now this hack retains that behaviour" code is actually
>> disabled with "#if 0&& ". Please adjust the guard or the comment.
>
> Okay comment changing.
>
>>
>> * CR+CRLF should be allowed because those CRs are a part of the final
>> CRLF sequence. IIRC, RFC 2616 recommends allowing CR*LF whenever CRLF is
>
>> required.
>
> They are allowed *if* followed by a LF which marks the line terminal.

Exactly.

> There was no mention I could see anywhere about sequences of CR*. LWS
> sequences for whitespace wrapping which permits middle-line CR only applies
> to headers which are outside this line.

Yes. I am only after allowing CR*LF, not implied LWS or bare CRs.

> These two and several other protocols using this type of request line do
> explicitly state "No CR or LF is allowed except in the final CRLF
> sequence".
>
> The noted exception that bare-CR are permitted in the HTTP mime-headers
> section implies that CR*CRLF would be possible and valid there. However,
> that section and its quirks are not relevant to this parser.

Sure.

Perhaps I did not grok the code right: I was under impression you only
allow at most one CR before LF because you follow CR checks with a i+1
LF checks:

     hmsg->buf[i + 1] == '\n'

I could miss a different case. If I am wrong, please ignore this bullet
with my apologies.

>> > * TODO: making this part of HttpParser
>>
>> Since the current diff is pretty much useless anyway, I would recommend
>> doing the above change now. It will help with goto-avoidance as well. I
>> agree that you do not have to do it now though.
>
> That involves changes the callers of this code and its API. I'd rather do
> that as part of the StringNG change so the API only changes once.

You can keep the callers intact:

     HttpParserParseReqLine(HttpParser *hmsg) {
         PROF_start(HttpParserParseReqLine);
         const int result = hmsg->parseReqLine();
         PROF_stop(HttpParserParseReqLine);
         return result;
     }

$0.02,

Alex.
Received on Tue Aug 31 2010 - 22:59:15 MDT