[squid-dev] [PATCH] adapting 100-Continue / A Bug 4067 fix

[Tsantilas Christos]

On 11/10/2014 09:36 AM, Amos Jeffries wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 10/11/2014 10:02 a.m., Tsantilas Christos wrote:
>> I am re-posting the patch. There are not huge changes.
>
> Looking over this in more detail...
>
> Whats the point of having buildHttpRequest() a separate method from
> processRequest() ?

It makes the code more readable. It is a private method for use 
internally by the Http::Server class.
I am suggesting to leave it as is. We can fix method name or its 
documentation.

>
> The documentation for buildHttpRequest() is wrong, no parsing takes
> place there. It is purely post-parse processing of some parsed HTTP
> request. ie processParsedRequest() action.

The HttpRequest::CreateFromUrlAndMethod still does parsing.
But yes we are using pre-parsed informations to build HTTP request object.


Is it ok the following documentation for buildHttpRequest?

/// Handles parsing results and build an error reply if parsing
/// is failed, or parses the url and build the HttpRequest object
/// using parsing results.
/// Return false if parsing is failed, true otherwise.

>
> I can sort of see a vague use of something like buildHttpRequest() in
> ICAP traffic processing, but not in its current form. The logic there
> directly dumps HTTP error messages into the client socket FD. ONly
> when that is fixed will the HttpRequest object creation be re-usable code.
>
> +1 whether you inline the two functions again or not.
>
> Amos