[squid-dev] [PATCH] Add reply_header_add

Alex Rousskov rousskov at measurement-factory.com
Wed Mar 16 03:15:24 UTC 2016


On 03/15/2016 07:29 PM, Nathan Hoad wrote:
> On 15 March 2016 at 12:11, Alex Rousskov wrote:
>> On 03/14/2016 05:46 PM, Nathan Hoad wrote:

>> * You have not adjusted HTTP response headers produced by
>> Http::One::Server::writeControlMsgAndCall(). Please either apply the
>> same logic to those headers or explicitly document that control message
>> headers are out of this option reach.

> Done.


Thank you. After those improvements, you can probably see a new pattern
emerging:

> if (Config2.onoff.mangle_request_headers)
>     httpHdrMangleList(hdr_out, request, ROR_REQUEST);
> 
> httpHdrAdd(hdr_out, request, al, Config.request_header_add);

and


> httpHdrMangleList(&rep->header, http->request, ROR_REPLY);
> httpHdrAdd(&rep->header, http->request, http->al, Config.reply_header_add);

and

> httpHdrMangleList(hdr, request, ROR_REPLY);
> 
> httpHdrAdd(hdr, request, http->al, Config.reply_header_add);

and

> $ bzr grep '    httpHdrMangleList' | wc -l
> 3

and, I would imagine,

> bzr grep '    httpHdrAdd' | wc -l
> 3


Yes, there are only three cases, and in all of them, we apply the same
set of two operations. Thus, you can now move the httpHdrAdd() call into
httpHdrMangleList()!

If you do that, move the code that gets the right Config.*_header_access
mangler from the beginning of httpHdrMangle() to httpHdrMangleList()
itself and adjust it to also check Config2.onoff.mangle_request_headers
during ROR_REQUEST.

Adjust as needed: httpHdrMangle() is a private/static function and you
will be working with all three httpHdrMangleList() calls in existence
(AFAICT).


I cannot require these additional changes from you, but if you need an
extra incentive/motivation, then just look at the loop in
httpHdrMangleList() and what happens at the start of httpHdrMangle()
that the loop calls on every iteration. Consider the common case of no
Config.*_header_access manglers configured. See how httpHdrMangleList()
always iterates through all header fields while repeating the same
checks and doing nothing useful at all?

With the above changes (that move those checks to be done once, in front
of the loop, and that will entirely eliminate looping in the common
case), you would be making Squid a tiny bit faster despite adding a new
feature!


Thank you,

Alex.



More information about the squid-dev mailing list