Alex Rousskov wrote:
> ... //< HTTP request headers after adaptation and redirection
There are some magic words here "adaptation and redirection"
OK I did not count the redirection.
So I am posting a new patch which, I hope, satisfy all Amos and Alex
requirements:
- http::>h always logs the virgin request headers (before
adaptation/redirections)
- http::>ha logs the request headers after adaptation and redirection.
- http::>ha is always enabled (even if adaptation is off)
Alex Rousskov wrote:
......
* Should we add "virgin" to >h definition in src/cf.data.pre?
Yep
* I would polish src/cf.data.pre text a little:
[http::]>ha The adapted HTTP request headers.
Optional header name argument as for >h
OK.
* 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.
There is no USE_ADAPTATION block now
* 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.
OK I add some comments
Please commit when you are comfortable with the code.
Because I change again the patch I will wait today and if there are no
any objections I will commit it tomorrow.
Regards,
Christos
Thank you,
Alex.
=== modified file 'src/AccessLogEntry.h'
--- src/AccessLogEntry.h 2009-12-22 01:12:53 +0000
+++ src/AccessLogEntry.h 2010-01-27 21:36:21 +0000
@@ -47,7 +47,9 @@
{
public:
- AccessLogEntry() : url(NULL) , reply(NULL), request(NULL) {}
+ AccessLogEntry() : url(NULL) , reply(NULL), request(NULL),
+ adapted_request(NULL)
+ {}
const char *url;
@@ -134,12 +136,17 @@
public:
Headers() : request(NULL),
+ adapted_request(NULL),
+
#if ICAP_CLIENT
icap(NULL),
#endif
reply(NULL) {}
- char *request;
+ char *request; //< virgin HTTP request headers
+
+ char *adapted_request; //< HTTP request headers after adaptation and redirection
+
#if ICAP_CLIENT
char * icap; ///< last matching ICAP response header.
@@ -159,7 +166,9 @@
} _private;
HierarchyLogEntry hier;
HttpReply *reply;
- HttpRequest *request;
+ HttpRequest *request; //< virgin HTTP request
+ HttpRequest *adapted_request; //< HTTP request after adaptation and redirection
+
#if ICAP_CLIENT
/** \brief This subclass holds log info for ICAP part of request
=== modified file 'src/cf.data.pre'
--- src/cf.data.pre 2010-01-21 12:48:36 +0000
+++ src/cf.data.pre 2010-01-27 21:41:41 +0000
@@ -2495,8 +2495,10 @@
HTTP cache related format codes:
- [http::]>h Request header. Optional header name argument
+ [http::]>h Virgin request header. Optional header name argument
on the format header[:[separator]element]
+ [http::]>ha The HTTP request headers after adaptation and redirection.
+ Optional header name argument as for >h
[http::]<h Reply header. Optional header name argument
as for >h
[http::]un User name
=== modified file 'src/client_side.cc'
--- src/client_side.cc 2010-01-13 01:13:17 +0000
+++ src/client_side.cc 2010-01-27 21:00:28 +0000
@@ -461,7 +461,17 @@
mb.init();
packerToMemInit(&p, &mb);
request->header.packInto(&p);
- aLogEntry->headers.request = xstrdup(mb.buf);
+ //This is the request after adaptation or redirection
+ aLogEntry->headers.adapted_request = xstrdup(mb.buf);
+
+ // the virgin request is saved to aLogEntry->request
+ if (aLogEntry->request) {
+ packerClean(&p);
+ mb.reset();
+ packerToMemInit(&p, &mb);
+ aLogEntry->request->header.packInto(&p);
+ aLogEntry->headers.request = xstrdup(mb.buf);
+ }
#if ICAP_CLIENT
packerClean(&p);
@@ -559,7 +569,7 @@
if (!Config.accessList.log || checklist->fastCheck()) {
if (request)
- al.request = HTTPMSGLOCK(request);
+ al.adapted_request = HTTPMSGLOCK(request);
accessLogLog(&al, checklist);
updateCounters();
=== modified file 'src/client_side_request.cc'
--- src/client_side_request.cc 2010-01-22 01:13:11 +0000
+++ src/client_side_request.cc 2010-01-27 21:02:01 +0000
@@ -1256,6 +1256,10 @@
{
assert(calloutContext);
+ /*Save the original request for logging purposes*/
+ if (!calloutContext->http->al.request)
+ calloutContext->http->al.request = HTTPMSGLOCK(request);
+
if (!calloutContext->http_access_done) {
debugs(83, 3, HERE << "Doing calloutContext->clientAccessCheck()");
calloutContext->http_access_done = true;
=== modified file 'src/log/access_log.cc'
--- src/log/access_log.cc 2009-12-22 01:12:53 +0000
+++ src/log/access_log.cc 2010-01-27 21:09:18 +0000
@@ -359,6 +359,10 @@
LFT_REQUEST_HEADER_ELEM,
LFT_REQUEST_ALL_HEADERS,
+ LFT_ADAPTED_REQUEST_HEADER,
+ LFT_ADAPTED_REQUEST_HEADER_ELEM,
+ LFT_ADAPTED_REQUEST_ALL_HEADERS,
+
LFT_REPLY_HEADER,
LFT_REPLY_HEADER_ELEM,
LFT_REPLY_ALL_HEADERS,
@@ -512,6 +516,8 @@
{"<tt", LFT_TOTAL_SERVER_SIDE_RESPONSE_TIME},
{"dt", LFT_DNS_WAIT_TIME},
+ {">ha", LFT_ADAPTED_REQUEST_HEADER},
+ {">ha", LFT_ADAPTED_REQUEST_ALL_HEADERS},
{">h", LFT_REQUEST_HEADER},
{">h", LFT_REQUEST_ALL_HEADERS},
{"<h", LFT_REPLY_HEADER},
@@ -765,6 +771,17 @@
break;
+ case LFT_ADAPTED_REQUEST_HEADER:
+
+ if (al->request)
+ sb = al->adapted_request->header.getByName(fmt->data.header.header);
+
+ out = sb.termedBuf();
+
+ quote = 1;
+
+ break;
+
case LFT_REPLY_HEADER:
if (al->reply)
sb = al->reply->header.getByName(fmt->data.header.header);
@@ -954,6 +971,16 @@
break;
+ case LFT_ADAPTED_REQUEST_HEADER_ELEM:
+ if (al->adapted_request)
+ sb = al->adapted_request->header.getByNameListMember(fmt->data.header.header, fmt->data.header.element, fmt->data.header.separator);
+
+ out = sb.termedBuf();
+
+ quote = 1;
+
+ break;
+
case LFT_REPLY_HEADER_ELEM:
if (al->reply)
sb = al->reply->header.getByNameListMember(fmt->data.header.header, fmt->data.header.element, fmt->data.header.separator);
@@ -971,6 +998,13 @@
break;
+ case LFT_ADAPTED_REQUEST_ALL_HEADERS:
+ out = al->headers.adapted_request;
+
+ quote = 1;
+
+ break;
+
case LFT_REPLY_ALL_HEADERS:
out = al->headers.reply;
@@ -1393,6 +1427,8 @@
case LFT_ICAP_REP_HEADER:
#endif
+ case LFT_ADAPTED_REQUEST_HEADER:
+
case LFT_REQUEST_HEADER:
case LFT_REPLY_HEADER:
@@ -1415,6 +1451,11 @@
case LFT_REQUEST_HEADER:
lt->type = LFT_REQUEST_HEADER_ELEM;
break;
+
+ case LFT_ADAPTED_REQUEST_HEADER:
+ lt->type = LFT_ADAPTED_REQUEST_HEADER_ELEM;
+ break;
+
case LFT_REPLY_HEADER:
lt->type = LFT_REPLY_HEADER_ELEM;
break;
@@ -1440,6 +1481,11 @@
case LFT_REQUEST_HEADER:
lt->type = LFT_REQUEST_ALL_HEADERS;
break;
+
+ case LFT_ADAPTED_REQUEST_HEADER:
+ lt->type = LFT_ADAPTED_REQUEST_ALL_HEADERS;
+ break;
+
case LFT_REPLY_HEADER:
lt->type = LFT_REPLY_ALL_HEADERS;
break;
@@ -1553,7 +1599,7 @@
case LFT_ICAP_REP_HEADER_ELEM:
#endif
case LFT_REQUEST_HEADER_ELEM:
-
+ case LFT_ADAPTED_REQUEST_HEADER_ELEM:
case LFT_REPLY_HEADER_ELEM:
if (t->data.header.separator != ',')
@@ -1567,6 +1613,9 @@
case LFT_REQUEST_HEADER_ELEM:
type = LFT_REQUEST_HEADER_ELEM;
break;
+ case LFT_ADAPTED_REQUEST_HEADER_ELEM:
+ type = LFT_ADAPTED_REQUEST_HEADER_ELEM;
+ break;
case LFT_REPLY_HEADER_ELEM:
type = LFT_REPLY_HEADER_ELEM;
break;
@@ -1588,7 +1637,7 @@
break;
case LFT_REQUEST_ALL_HEADERS:
-
+ case LFT_ADAPTED_REQUEST_ALL_HEADERS:
case LFT_REPLY_ALL_HEADERS:
#if ICAP_CLIENT
@@ -1601,6 +1650,9 @@
case LFT_REQUEST_ALL_HEADERS:
type = LFT_REQUEST_HEADER;
break;
+ case LFT_ADAPTED_REQUEST_ALL_HEADERS:
+ type = LFT_ADAPTED_REQUEST_HEADER;
+ break;
case LFT_REPLY_ALL_HEADERS:
type = LFT_REPLY_HEADER;
break;
@@ -2377,6 +2429,9 @@
safe_free(aLogEntry->headers.reply);
safe_free(aLogEntry->cache.authuser);
+ safe_free(aLogEntry->headers.adapted_request);
+ HTTPMSGUNLOCK(aLogEntry->adapted_request);
+
HTTPMSGUNLOCK(aLogEntry->reply);
HTTPMSGUNLOCK(aLogEntry->request);
#if ICAP_CLIENT
