Hello,
Attached is a patch with the changes recommended by Amos.
Thank you,
Nathan.
On 25 March 2016 at 04:29, Amos Jeffries <[email protected]> wrote:
> On 17/03/2016 5:25 p.m., Nathan Hoad wrote:
>> On 17 March 2016 at 13:33, Alex Rousskov
>> <[email protected]> wrote:
>>> 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.
>>
>> Good catch! To lessen work for the committer, I've attached a version
>> of the patch with that change.
>>
>
>
> in src/HttpHeaderTools.cc:
> * you dont have to compare pointers with "!= nullptr" or "== nullptr".
>
> in src/enums.h:
>
> * since you are touching req_or_rep enum anyway please move it to
> HttpHeaderTools.h
> - then you can remove the #include for enums.h again
>
> in src/cf.data.pre:
>
> * use " [ acl ... ] " instead of "[acl1] ..."
>
>
> Thanks
> Amos
>
> _______________________________________________
> squid-dev mailing list
> [email protected]
> http://lists.squid-cache.org/listinfo/squid-dev
Implement reply_header_add.
This work is submitted on behalf of Bloomberg L.P.
=== modified file 'src/HttpHeaderTools.cc'
--- src/HttpHeaderTools.cc 2016-01-24 17:41:43 +0000
+++ src/HttpHeaderTools.cc 2016-03-28 21:44:35 +0000
@@ -40,6 +40,7 @@
#include <string>
static void httpHeaderPutStrvf(HttpHeader * hdr, Http::HdrType id, const char *fmt, va_list vargs);
+static void httpHdrAdd(HttpHeader *heads, HttpRequest *request, const AccessLogEntryPointer &al, HeaderWithAclList &headersAdd);
void
httpHeaderMaskInit(HttpHeaderMask * mask, int value)
@@ -267,29 +268,12 @@
* \retval 1 Header has no access controls to test
*/
static int
-httpHdrMangle(HttpHeaderEntry * e, HttpRequest * request, int req_or_rep)
+httpHdrMangle(HttpHeaderEntry * e, HttpRequest * request, HeaderManglers *hms)
{
int retval;
- /* check with anonymizer tables */
- HeaderManglers *hms = NULL;
assert(e);
- if (ROR_REQUEST == req_or_rep) {
- hms = Config.request_header_access;
- } else if (ROR_REPLY == req_or_rep) {
- hms = Config.reply_header_access;
- } else {
- /* error. But let's call it "request". */
- hms = Config.request_header_access;
- }
-
- /* manglers are not configured for this message kind */
- if (!hms) {
- debugs(66, 7, "no manglers configured for message kind " << req_or_rep);
- return 1;
- }
-
const headerMangler *hm = hms->find(*e);
/* mangler or checklist went away. default allow */
@@ -323,18 +307,40 @@
/** Mangles headers for a list of headers. */
void
-httpHdrMangleList(HttpHeader * l, HttpRequest * request, int req_or_rep)
+httpHdrMangleList(HttpHeader *l, HttpRequest *request, const AccessLogEntryPointer &al, req_or_rep_t req_or_rep)
{
HttpHeaderEntry *e;
HttpHeaderPos p = HttpHeaderInitPos;
- int headers_deleted = 0;
- while ((e = l->getEntry(&p)))
- if (0 == httpHdrMangle(e, request, req_or_rep))
- l->delAt(p, headers_deleted);
-
- if (headers_deleted)
- l->refreshMask();
+ /* check with anonymizer tables */
+ HeaderManglers *hms = nullptr;
+
+ HeaderWithAclList *headersAdd = nullptr;
+
+ switch (req_or_rep) {
+ case ROR_REQUEST:
+ hms = Config.request_header_access;
+ headersAdd = Config.request_header_add;
+ break;
+ case ROR_REPLY:
+ hms = Config.reply_header_access;
+ headersAdd = Config.reply_header_add;
+ break;
+ }
+
+ if (hms) {
+ int headers_deleted = 0;
+ while ((e = l->getEntry(&p)))
+ if (0 == httpHdrMangle(e, request, hms))
+ l->delAt(p, headers_deleted);
+
+ if (headers_deleted)
+ l->refreshMask();
+ }
+
+ if (headersAdd && !headersAdd->empty()) {
+ httpHdrAdd(l, request, al, *headersAdd);
+ }
}
static
=== modified file 'src/HttpHeaderTools.h'
--- src/HttpHeaderTools.h 2016-01-01 00:12:18 +0000
+++ src/HttpHeaderTools.h 2016-03-28 21:46:06 +0000
@@ -29,6 +29,13 @@
typedef std::list<HeaderWithAcl> HeaderWithAclList;
+/* Distinguish between Request and Reply (for header mangling) */
+typedef enum {
+ ROR_REQUEST,
+ ROR_REPLY
+} req_or_rep_t;
+
+
// Currently a POD
class headerMangler
{
@@ -119,7 +126,7 @@
const char *getStringPrefix(const char *str, size_t len);
-void httpHdrMangleList(HttpHeader *, HttpRequest *, int req_or_rep);
+void httpHdrMangleList(HttpHeader *, HttpRequest *, const AccessLogEntryPointer &al, req_or_rep_t req_or_rep);
#endif
=== modified file 'src/SquidConfig.h'
--- src/SquidConfig.h 2016-03-12 20:27:35 +0000
+++ src/SquidConfig.h 2016-03-28 21:44:35 +0000
@@ -463,6 +463,8 @@
HeaderManglers *reply_header_access;
///request_header_add access list
HeaderWithAclList *request_header_add;
+ ///reply_header_add access list
+ HeaderWithAclList *reply_header_add;
///note
Notes notes;
char *coredump_dir;
@@ -545,7 +547,6 @@
public:
struct {
int enable_purge;
- int mangle_request_headers;
} onoff;
uid_t effectiveUserID;
gid_t effectiveGroupID;
=== modified file 'src/cache_cf.cc'
--- src/cache_cf.cc 2016-03-01 09:58:44 +0000
+++ src/cache_cf.cc 2016-03-28 21:44:35 +0000
@@ -808,8 +808,6 @@
// TODO: replace with a dedicated "purge" ACL option?
Config2.onoff.enable_purge = (ACLMethodData::ThePurgeCount > 0);
- Config2.onoff.mangle_request_headers = (Config.request_header_access != NULL);
-
if (geteuid() == 0) {
if (NULL != Config.effectiveUser) {
=== modified file 'src/cf.data.pre'
--- src/cf.data.pre 2016-03-12 20:27:35 +0000
+++ src/cf.data.pre 2016-03-28 21:47:18 +0000
@@ -109,6 +109,21 @@
${service_name} expands into the current Squid service instance
name identifier which is provided by -n on the command line.
+ Logformat Macros
+
+ Logformat macros can be used in many places outside of the logformat
+ directive. In theory, all of the logformat codes can be used as %macros,
+ where they are supported. In practice, a %macro expands as a dash (-) when
+ the transaction does not yet have enough information and a value is needed.
+
+ There is no definitive list of what tokens are available at the various
+ stages of the transaction.
+
+ And some information may already be available to Squid but not yet
+ committed where the macro expansion code can access it (report
+ such instances!). The macro will be expanded into a single dash
+ ('-') in such cases. Not all macros have been tested.
+
COMMENT_END
# options still not yet ported from 2.7 to 3.x
@@ -6066,7 +6081,7 @@
LOC: Config.request_header_add
DEFAULT: none
DOC_START
- Usage: request_header_add field-name field-value acl1 [acl2] ...
+ Usage: request_header_add field-name field-value [ acl ... ]
Example: request_header_add X-Client-CA "CA=%ssl::>cert_issuer" all
This option adds header fields to outgoing HTTP requests (i.e.,
@@ -6086,20 +6101,47 @@
string format is used, then the surrounding quotes are removed
while escape sequences and %macros are processed.
- In theory, all of the logformat codes can be used as %macros.
- However, unlike logging (which happens at the very end of
- transaction lifetime), the transaction may not yet have enough
- information to expand a macro when the new header value is needed.
- And some information may already be available to Squid but not yet
- committed where the macro expansion code can access it (report
- such instances!). The macro will be expanded into a single dash
- ('-') in such cases. Not all macros have been tested.
-
One or more Squid ACLs may be specified to restrict header
injection to matching requests. As always in squid.conf, all
ACLs in an option ACL list must be satisfied for the insertion
to happen. The request_header_add option supports fast ACLs
only.
+
+ See also: reply_header_add.
+DOC_END
+
+NAME: reply_header_add
+TYPE: HeaderWithAclList
+LOC: Config.reply_header_add
+DEFAULT: none
+DOC_START
+ Usage: reply_header_add field-name field-value [ acl ... ]
+ Example: reply_header_add X-Client-CA "CA=%ssl::>cert_issuer" all
+
+ This option adds header fields to outgoing HTTP responses (i.e., response
+ headers delivered by Squid to the client). This option has no effect on
+ cache hit detection. The equivalent adaptation vectoring point in
+ ICAP terminology is post-cache RESPMOD. This option does not apply to
+ successful CONNECT replies.
+
+ Field-name is a token specifying an HTTP header name. If a
+ standard HTTP header name is used, Squid does not check whether
+ the new header conflicts with any existing headers or violates
+ HTTP rules. If the response to be modified already contains a
+ field with the same name, the old field is preserved but the
+ header field values are not merged.
+
+ Field-value is either a token or a quoted string. If quoted
+ string format is used, then the surrounding quotes are removed
+ while escape sequences and %macros are processed.
+
+ One or more Squid ACLs may be specified to restrict header
+ injection to matching responses. As always in squid.conf, all
+ ACLs in an option ACL list must be satisfied for the insertion
+ to happen. The reply_header_add option supports fast ACLs
+ only.
+
+ See also: request_header_add.
DOC_END
NAME: note
=== modified file 'src/client_side_reply.cc'
--- src/client_side_reply.cc 2016-03-11 18:00:51 +0000
+++ src/client_side_reply.cc 2016-03-28 21:44:35 +0000
@@ -1578,7 +1578,7 @@
/* TODO: else case: drop any controls intended specifically for our surrogate ID */
}
- httpHdrMangleList(hdr, request, ROR_REPLY);
+ httpHdrMangleList(hdr, request, http->al, ROR_REPLY);
}
void
=== modified file 'src/enums.h'
--- src/enums.h 2016-01-01 00:12:18 +0000
+++ src/enums.h 2016-03-28 21:44:55 +0000
@@ -179,12 +179,6 @@
DIGEST_READ_DONE
} digest_read_state_t;
-/* Distinguish between Request and Reply (for header mangling) */
-enum {
- ROR_REQUEST,
- ROR_REPLY
-};
-
/* CygWin & Windows NT Port */
#if _SQUID_WINDOWS_
/*
=== modified file 'src/http.cc'
--- src/http.cc 2016-03-11 18:00:51 +0000
+++ src/http.cc 2016-03-28 21:44:35 +0000
@@ -83,8 +83,6 @@
static void httpMaybeRemovePublic(StoreEntry *, Http::StatusCode);
static void copyOneHeaderFromClientsideRequestToUpstreamRequest(const HttpHeaderEntry *e, const String strConnection, const HttpRequest * request,
HttpHeader * hdr_out, const int we_do_ranges, const HttpStateFlags &);
-//Declared in HttpHeaderTools.cc
-void httpHdrAdd(HttpHeader *heads, HttpRequest *request, const AccessLogEntryPointer &al, HeaderWithAclList &headers_add);
HttpStateData::HttpStateData(FwdState *theFwdState) :
AsyncJob("HttpStateData"),
@@ -1942,11 +1940,7 @@
}
/* Now mangle the headers. */
- if (Config2.onoff.mangle_request_headers)
- httpHdrMangleList(hdr_out, request, ROR_REQUEST);
-
- if (Config.request_header_add && !Config.request_header_add->empty())
- httpHdrAdd(hdr_out, request, al, *Config.request_header_add);
+ httpHdrMangleList(hdr_out, request, al, ROR_REQUEST);
strConnection.clean();
}
=== modified file 'src/servers/Http1Server.cc'
--- src/servers/Http1Server.cc 2016-01-24 17:41:43 +0000
+++ src/servers/Http1Server.cc 2016-03-28 21:44:35 +0000
@@ -276,11 +276,13 @@
void
Http::One::Server::writeControlMsgAndCall(HttpReply *rep, AsyncCall::Pointer &call)
{
+ const ClientHttpRequest *http = pipeline.front()->http;
+
// apply selected clientReplyContext::buildReplyHeader() mods
// it is not clear what headers are required for control messages
rep->header.removeHopByHopEntries();
rep->header.putStr(Http::HdrType::CONNECTION, "keep-alive");
- httpHdrMangleList(&rep->header, pipeline.front()->http->request, ROR_REPLY);
+ httpHdrMangleList(&rep->header, http->request, http->al, ROR_REPLY);
MemBuf *mb = rep->pack();
_______________________________________________
squid-dev mailing list
[email protected]
http://lists.squid-cache.org/listinfo/squid-dev
