Re: [PATCH] log virgin HTTP request headers

On 01/24/2010 08:20 PM, Amos Jeffries wrote:
> Tsantilas Christos wrote:
>> Hi all,
>> First of all sorry for late answer.
>> I am sending a second version of the patch.
>> The new patch uses the format code "http::>ha" to log the adapted
>> headers. The already existing format code http::>h logs the original
>> request headers.
>> This patch still does not handle the use of the request_header_access.
>> Please read below.
>>
>>
>> Amos Jeffries wrote:
>>> Tsantilas Christos wrote:
>>>> Hi all,
>>>> This patch adds a new format code which allow the user to log HTTP
>>>> request header or header fields before they are adapted.
>>>> The existing "http::>h" format code logs HTTP request headers after
>>>> adaptation.
>>>> The new format code is the "http::>hv".
>>>>
>>>> This is a Measurement Factory project.
>>>>
>>>> Regards,
>>>> Christos
>>>>
>>>
>>> Um, I would think this makes more sense done the other way around.
>>>
>>> With the default >h displaying the virgin headers received from the
>>> client and some other code ( >ha ?) for the adapted headers. Be it
>>
>> OK Amos this patch does it.
>>
>>> adaptation or headers_access doing the alteration.
>>
>> The request_header_access does not modify the request send by the
>> client but has to do with the http request will be send to the server
>> by squid.
>> Also currently does not have any effect to request headers logging.
>>
>> The request_header_access implemented inside http.cc file. If we want
>> to consider it as request adaptation mechanism maybe we should move it
>> to client_side* code.
>>
>> Regards,
>> Christos
>>
>>>
>>> Likewise on replies.
>>>
>>> Amos
>
> Thats fine. +1 from me.

* Should we add "virgin" to >h definition in src/cf.data.pre?

* I would polish src/cf.data.pre text a little:

  [http::]>ha The adapted HTTP request headers.
                Optional header name argument as for >h

* In the first hunk of src/client_side.cc, I would use a single "#if
USE_ADAPTATION" block to set both aLogEntry->headers.adapted_request and
aLogEntry->headers.request instead of testing USE_ADAPTATION twice. If
this is not possible for some reason, ignore me.

* Please add comments to document the new adapted_request fields in the
header files. Something like

... //< HTTP request headers after adaptation and redirection

may work well.

Please commit when you are comfortable with the code.

Thank you,

Alex.
Received on Tue Jan 26 2010 - 23:47:52 MST