Re: [PATCH] log virgin HTTP request headers

Tsantilas Christos 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
>> adaptation or headers_access doing the alteration.
>
> The only objection I have is that the >h is already implemented to log
> headers after adaptation for squid3.0 and squid3.1

I know. However its documented as merely "request header" with
indication that it's adapted first. It's historic from squid-2 where no
adaptation happened to them. So IMO the fact that it displays the
adapted header is kind of a regression.

>
> I did not have in my mind the headers_access, so in the patch the new
> format code activated only if the adaptation is active.
> Should the http::>hv (or http::>ha) be always active?

Yes. IMO always active.

If you can find a nice spot prior to *_header_access to save the virgin
data that would be good. But ...

(Out of scope)
   we also have bugs open to make the header_access happen after
adaptation. Which makes more sense than the current flow order since we
really do want the original headers to go to ICAP and header_access to
prevent/allow stuff going outbound.

Amos

-- 
Please be using
   Current Stable Squid 2.7.STABLE7 or 3.0.STABLE21
   Current Beta Squid 3.1.0.15