On 03/16/2016 05:40 PM, Nathan Hoad wrote: > I've opted to remove Config2.onoff.mangle_request_headers completely.
Even better! I did not realize it is not a "real" configuration option but a silly(?) cache for "Config.request_header_access != NULL". > -httpHdrMangle(HttpHeaderEntry * e, HttpRequest * request, int req_or_rep) > +httpHdrMangle(HttpHeaderEntry * e, HttpRequest * request, HeaderManglers > *hms, int req_or_rep) I do not think you need/use the last parameter, but its removal can be done when committing your patch. +1 BTW, do you know whether ACLs are required for HeaderWithAclList (i.e., our "ACLs may be specified" wording is misleading) or optional (i.e., our "Usage: ... field-value acl1 [acl2]" is wrong)? Thank you, Alex. > On 16 March 2016 at 14:15, Alex Rousskov > <[email protected]> wrote: >> 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. >> _______________________________________________ squid-dev mailing list [email protected] http://lists.squid-cache.org/listinfo/squid-dev
