On 7/04/2013 5:48 p.m., Amos Jeffries wrote:
What this does is convert ErrorState object to using the generic
libformat.la parser and macro expansions instead of its own rather
limited custom ones for error pages and deny_info URL creation.
Alex: If possible could you guys throw this through WebPolygraph to see
if the different in parsing shows up at all?
I'm checking the performance variance for error-only workload now.
Change Summary:
1) register the 1-byte error page macros as libformat codes
2) convert the ErrorState::Convert macro expander to use those codes
instead of char literals
3) extend the libformat to handle several process-wide informational
macros and a human-readabhe RFC1123 time format
4) extend the Format::assemble method to call the adapted
ErrorState::Convert function when it encounters byte codes requiring
ErrorState special handling
5) convert the ErrorState::ConvertText and
ErrorState::DenyInfoLocation methods to use the libformat API to parse
and assemble the output page
NOTES on problems remaining:
*a) Several of the error page macros are better served by other
logformat macro expansions. However on some the interaction between
deny_info and ERR_* template usages has required that the
ErrorState::Convert logics be used for now.
*b) Doing a whole parse cycle for every error response is very costly.
We are already loading the text page files on every error response.
This adds the un-optimized libformat parser lag on top of that. To
avoid this we really need to add a cache for error page templates and
strings, such that each one is only loaded and parsed once. From that
we can more speedily assemble the expansions final output.
*c) For now AccessLogEntry required by the libformat API is generated
by ErrorState just before use. Ideally the code creating the
ErrorState should pass it an already filled out ALE object for the
transaction. This way we will get a lot more of the libformat macros
filled out. That is done in a few places where easily possible, but
many places still cannot do it. This restricts the format codes which
will work on any given error.
Amos
=== modified file 'src/AccessLogEntry.h'
--- src/AccessLogEntry.h 2013-05-23 08:18:09 +0000
+++ src/AccessLogEntry.h 2013-06-01 12:59:51 +0000
@@ -48,10 +48,10 @@
#include "ssl/gadgets.h"
#endif
-/* forward decls */
+class ErrorState;
+class CustomLog;
class HttpReply;
class HttpRequest;
-class CustomLog;
class AccessLogEntry: public RefCountable
{
@@ -60,7 +60,7 @@
typedef RefCount<AccessLogEntry> Pointer;
AccessLogEntry() : url(NULL), tcpClient(), reply(NULL), request(NULL),
- adapted_request(NULL) {}
+ adapted_request(NULL), errorPage(NULL) {}
~AccessLogEntry();
/// Fetch the client IP log string into the given buffer.
@@ -277,6 +277,10 @@
}
icap;
#endif
+
+ /// details of the error page presented to the client (if any)
+ // NP: circular reference, so only to be set by ErrorPageState::ConvertText
+ ErrorState *errorPage;
};
class ACLChecklist;
=== modified file 'src/client_side.cc'
--- src/client_side.cc 2013-05-26 01:57:47 +0000
+++ src/client_side.cc 2013-06-02 00:50:34 +0000
@@ -2539,7 +2539,7 @@
assert (repContext);
repContext->setReplyToError(ERR_TOO_BIG,
Http::scBadRequest, Http::METHOD_NONE, NULL,
- clientConnection->remote, NULL, NULL, NULL);
+ clientConnection, NULL, NULL, NULL);
context->registerWithConn();
context->pullData();
}
@@ -2634,7 +2634,7 @@
// Create an error object and fill it
ErrorState *err = new ErrorState(ERR_SECURE_CONNECT_FAIL,
Http::scServiceUnavailable, request);
- err->src_addr = clientConnection->remote;
+ err->tcpClient = clientConnection;
Ssl::ErrorDetail *errDetail = new Ssl::ErrorDetail(
SQUID_X509_V_ERR_DOMAIN_MISMATCH,
sslServerBump->serverCert.get(), NULL);
@@ -2681,15 +2681,15 @@
assert (repContext);
switch (hp->request_parse_status) {
case Http::scHeaderTooLarge:
- repContext->setReplyToError(ERR_TOO_BIG, Http::scBadRequest,
method, http->uri, conn->clientConnection->remote, NULL, conn->in.buf, NULL);
+ repContext->setReplyToError(ERR_TOO_BIG, Http::scBadRequest,
method, http->uri, conn->clientConnection, NULL, conn->in.buf, NULL);
break;
case Http::scMethodNotAllowed:
repContext->setReplyToError(ERR_UNSUP_REQ,
Http::scMethodNotAllowed, method, http->uri,
- conn->clientConnection->remote, NULL,
conn->in.buf, NULL);
+ conn->clientConnection, NULL,
conn->in.buf, NULL);
break;
default:
repContext->setReplyToError(ERR_INVALID_REQ,
hp->request_parse_status, method, http->uri,
- conn->clientConnection->remote, NULL,
conn->in.buf, NULL);
+ conn->clientConnection, NULL,
conn->in.buf, NULL);
}
assert(context->http->out.offset == 0);
context->pullData();
@@ -2704,7 +2704,7 @@
setLogUri(http, http->uri, true);
clientReplyContext *repContext = dynamic_cast<clientReplyContext
*>(node->data.getRaw());
assert (repContext);
- repContext->setReplyToError(ERR_INVALID_URL, Http::scBadRequest,
method, http->uri, conn->clientConnection->remote, NULL, NULL, NULL);
+ repContext->setReplyToError(ERR_INVALID_URL, Http::scBadRequest,
method, http->uri, conn->clientConnection, NULL, NULL, NULL);
assert(context->http->out.offset == 0);
context->pullData();
goto finish;
@@ -2724,7 +2724,7 @@
clientReplyContext *repContext = dynamic_cast<clientReplyContext
*>(node->data.getRaw());
assert (repContext);
repContext->setReplyToError(ERR_UNSUP_HTTPVERSION,
Http::scHttpVersionNotSupported, method, http->uri,
- conn->clientConnection->remote, NULL,
HttpParserHdrBuf(hp), NULL);
+ conn->clientConnection, NULL,
HttpParserHdrBuf(hp), NULL);
assert(context->http->out.offset == 0);
context->pullData();
goto finish;
@@ -2741,7 +2741,7 @@
setLogUri(http, http->uri, true);
clientReplyContext *repContext = dynamic_cast<clientReplyContext
*>(node->data.getRaw());
assert (repContext);
- repContext->setReplyToError(ERR_INVALID_REQ, Http::scBadRequest,
method, http->uri, conn->clientConnection->remote, NULL, NULL, NULL);
+ repContext->setReplyToError(ERR_INVALID_REQ, Http::scBadRequest,
method, http->uri, conn->clientConnection, NULL, NULL, NULL);
assert(context->http->out.offset == 0);
context->pullData();
goto finish;
@@ -2828,7 +2828,7 @@
clientReplyContext *repContext = dynamic_cast<clientReplyContext
*>(node->data.getRaw());
assert (repContext);
repContext->setReplyToError(ERR_UNSUP_REQ, Http::scNotImplemented,
request->method, NULL,
- conn->clientConnection->remote,
request.getRaw(), NULL, NULL);
+ conn->clientConnection, request.getRaw(),
NULL, NULL);
assert(context->http->out.offset == 0);
context->pullData();
goto finish;
@@ -2841,7 +2841,7 @@
conn->quitAfterError(request.getRaw());
repContext->setReplyToError(ERR_INVALID_REQ,
Http::scLengthRequired, request->method,
NULL,
- conn->clientConnection->remote,
request.getRaw(), NULL, NULL);
+ conn->clientConnection, request.getRaw(),
NULL, NULL);
assert(context->http->out.offset == 0);
context->pullData();
goto finish;
@@ -2856,7 +2856,7 @@
assert (repContext);
conn->quitAfterError(request.getRaw());
repContext->setReplyToError(ERR_INVALID_REQ,
Http::scExpectationFailed, request->method, http->uri,
- conn->clientConnection->remote,
request.getRaw(), NULL, NULL);
+ conn->clientConnection,
request.getRaw(), NULL, NULL);
assert(context->http->out.offset == 0);
context->pullData();
goto finish;
@@ -2897,7 +2897,7 @@
conn->quitAfterError(request.getRaw());
repContext->setReplyToError(ERR_TOO_BIG,
Http::scRequestEntityTooLarge,
Http::METHOD_NONE, NULL,
- conn->clientConnection->remote,
http->request, NULL, NULL);
+ conn->clientConnection, http->request,
NULL, NULL);
assert(context->http->out.offset == 0);
context->pullData();
goto finish;
@@ -4646,3 +4646,16 @@
/* NOTE: pinning.pinned should be kept. This combined with fd == -1 at the
end of a request indicates that the host
* connection has gone away */
}
+
+ErrorState *
+ClientBuildError(err_type page_id, Http::StatusCode status, char const *url,
+ const Comm::ConnectionPointer &client, HttpRequest * request)
+{
+ ErrorState *err = new ErrorState(page_id, status, request);
+ err->tcpClient = client;
+
+ if (url)
+ err->url = xstrdup(url);
+
+ return err;
+}
=== modified file 'src/client_side.h'
--- src/client_side.h 2013-05-30 14:58:49 +0000
+++ src/client_side.h 2013-06-02 00:39:07 +0000
@@ -47,6 +47,7 @@
class ClientHttpRequest;
class clientStreamNode;
class ChunkedCodingParser;
+class ErrorState;
class HelperReply;
namespace AnyP
{
@@ -430,4 +431,6 @@
void clientHttpConnectionsClose(void);
void httpRequestFree(void *);
+ErrorState *ClientBuildError(err_type, Http::StatusCode, char const *url,
const Comm::ConnectionPointer &c, HttpRequest *r);
+
#endif /* SQUID_CLIENTSIDE_H */
=== modified file 'src/client_side_reply.cc'
--- src/client_side_reply.cc 2013-05-30 14:58:49 +0000
+++ src/client_side_reply.cc 2013-06-02 00:32:46 +0000
@@ -72,7 +72,6 @@
/* Local functions */
extern "C" CSS clientReplyStatus;
-ErrorState *clientBuildError(err_type, Http::StatusCode, char const *,
Ip::Address &, HttpRequest *);
/* privates */
@@ -102,7 +101,7 @@
void
clientReplyContext::setReplyToError(
err_type err, Http::StatusCode status, const HttpRequestMethod& method,
char const *uri,
- Ip::Address &addr, HttpRequest * failedrequest, const char
*unparsedrequest,
+ const Comm::ConnectionPointer &client, HttpRequest * failedrequest, const
char *unparsedrequest,
#if USE_AUTH
Auth::UserRequest::Pointer auth_user_request
#else
@@ -110,7 +109,7 @@
#endif
)
{
- ErrorState *errstate = clientBuildError(err, status, uri, addr,
failedrequest);
+ ErrorState *errstate = ClientBuildError(err, status, uri, client,
failedrequest);
if (unparsedrequest)
errstate->request_hdrs = xstrdup(unparsedrequest);
@@ -654,7 +653,7 @@
/// Deny loops
if (r->flags.loopDetected) {
http->al->http.code = Http::scForbidden;
- err = clientBuildError(ERR_ACCESS_DENIED, Http::scForbidden, NULL,
http->getConn()->clientConnection->remote, http->request);
+ err = ClientBuildError(ERR_ACCESS_DENIED, Http::scForbidden, NULL,
http->getConn()->clientConnection, http->request);
createStoreEntry(r->method, RequestFlags());
errorAppendEntry(http->storeEntry(), err);
triggerInitialStoreRead();
@@ -698,8 +697,8 @@
debugs(88, 4, "clientProcessOnlyIfCachedMiss: '" <<
RequestMethodStr(http->request->method) << " " << http->uri << "'");
http->al->http.code = Http::scGateway_Timeout;
- ErrorState *err = clientBuildError(ERR_ONLY_IF_CACHED_MISS,
Http::scGateway_Timeout, NULL,
-
http->getConn()->clientConnection->remote, http->request);
+ ErrorState *err = ClientBuildError(ERR_ONLY_IF_CACHED_MISS,
Http::scGateway_Timeout, NULL,
+ http->getConn()->clientConnection,
http->request);
removeClientStoreReference(&sc, http);
startError(err);
}
@@ -865,8 +864,8 @@
if (EBIT_TEST(entry->flags, ENTRY_SPECIAL)) {
http->logType = LOG_TCP_DENIED;
- ErrorState *err = clientBuildError(ERR_ACCESS_DENIED,
Http::scForbidden, NULL,
-
http->getConn()->clientConnection->remote, http->request);
+ ErrorState *err = ClientBuildError(ERR_ACCESS_DENIED,
Http::scForbidden, NULL,
+ http->getConn()->clientConnection,
http->request);
startError(err);
return;
}
@@ -904,7 +903,7 @@
if (!Config2.onoff.enable_purge) {
http->logType = LOG_TCP_DENIED;
- ErrorState *err = clientBuildError(ERR_ACCESS_DENIED,
Http::scForbidden, NULL, http->getConn()->clientConnection->remote,
http->request);
+ ErrorState *err = ClientBuildError(ERR_ACCESS_DENIED,
Http::scForbidden, NULL, http->getConn()->clientConnection, http->request);
startError(err);
return;
}
@@ -1852,12 +1851,9 @@
void
clientReplyContext::sendBodyTooLargeError()
{
- Ip::Address tmp_noaddr;
- tmp_noaddr.SetNoAddr(); // TODO: make a global const
+ Comm::ConnectionPointer client = http->getConn() != NULL ?
http->getConn()->clientConnection : NULL;
http->logType = LOG_TCP_DENIED_REPLY;
- ErrorState *err = clientBuildError(ERR_TOO_BIG, Http::scForbidden, NULL,
- http->getConn() != NULL ?
http->getConn()->clientConnection->remote : tmp_noaddr,
- http->request);
+ ErrorState *err = ClientBuildError(ERR_TOO_BIG, Http::scForbidden, NULL,
client, http->request);
removeClientStoreReference(&(sc), http);
HTTPMSGUNLOCK(reply);
startError(err);
@@ -1869,9 +1865,8 @@
clientReplyContext::sendPreconditionFailedError()
{
http->logType = LOG_TCP_HIT;
- ErrorState *const err =
- clientBuildError(ERR_PRECONDITION_FAILED, Http::scPreconditionFailed,
- NULL, http->getConn()->clientConnection->remote,
http->request);
+ ErrorState *const err = ClientBuildError(ERR_PRECONDITION_FAILED,
Http::scPreconditionFailed,
+ NULL, http->getConn()->clientConnection,
http->request);
removeClientStoreReference(&sc, http);
HTTPMSGUNLOCK(reply);
startError(err);
@@ -1975,11 +1970,8 @@
if (page_id == ERR_NONE)
page_id = ERR_ACCESS_DENIED;
- Ip::Address tmp_noaddr;
- tmp_noaddr.SetNoAddr();
- err = clientBuildError(page_id, Http::scForbidden, NULL,
- http->getConn() != NULL ?
http->getConn()->clientConnection->remote : tmp_noaddr,
- http->request);
+ Comm::ConnectionPointer client = http->getConn() != NULL ?
http->getConn()->clientConnection : NULL;
+ err = ClientBuildError(page_id, Http::scForbidden, NULL, client,
http->request);
removeClientStoreReference(&sc, http);
@@ -2199,16 +2191,3 @@
*/
http->storeEntry(e);
}
-
-ErrorState *
-clientBuildError(err_type page_id, Http::StatusCode status, char const *url,
- Ip::Address &src_addr, HttpRequest * request)
-{
- ErrorState *err = new ErrorState(page_id, status, request);
- err->src_addr = src_addr;
-
- if (url)
- err->url = xstrdup(url);
-
- return err;
-}
=== modified file 'src/client_side_reply.h'
--- src/client_side_reply.h 2013-05-30 14:58:49 +0000
+++ src/client_side_reply.h 2013-06-01 12:59:51 +0000
@@ -72,7 +72,7 @@
/// replaces current response store entry with the given one
void setReplyToStoreEntry(StoreEntry *e);
/// builds error using clientBuildError() and calls setReplyToError() below
- void setReplyToError(err_type, Http::StatusCode, const HttpRequestMethod&,
char const *, Ip::Address &, HttpRequest *, const char *,
+ void setReplyToError(err_type, Http::StatusCode, const HttpRequestMethod&,
char const *, const Comm::ConnectionPointer &, HttpRequest *, const char *,
#if USE_AUTH
Auth::UserRequest::Pointer);
#else
=== modified file 'src/client_side_request.cc'
--- src/client_side_request.cc 2013-05-26 01:08:42 +0000
+++ src/client_side_request.cc 2013-06-02 00:35:01 +0000
@@ -103,8 +103,6 @@
static void clientFollowXForwardedForCheck(allow_t answer, void *data);
#endif /* FOLLOW_X_FORWARDED_FOR */
-ErrorState *clientBuildError(err_type, Http::StatusCode, char const *url,
Ip::Address &, HttpRequest *);
-
CBDATA_CLASS_INIT(ClientRequestContext);
void *
@@ -612,7 +610,7 @@
assert (repContext);
repContext->setReplyToError(ERR_CONFLICT_HOST, Http::scConflict,
http->request->method, NULL,
- http->getConn()->clientConnection->remote,
+ http->getConn()->clientConnection,
http->request,
NULL,
#if USE_AUTH
@@ -838,13 +836,8 @@
page_id = ERR_ACCESS_DENIED;
}
- Ip::Address tmpnoaddr;
- tmpnoaddr.SetNoAddr();
- error = clientBuildError(page_id, status,
- NULL,
- http->getConn() != NULL ?
http->getConn()->clientConnection->remote : tmpnoaddr,
- http->request
- );
+ Comm::ConnectionPointer client = (http->getConn() != NULL ?
http->getConn()->clientConnection : NULL);
+ error = ClientBuildError(page_id, status, NULL, client, http->request);
#if USE_AUTH
error->auth_user_request =
@@ -2078,13 +2071,10 @@
// setReplyToError, but it seems unlikely that the errno reflects the
// true cause of the error at this point, so I did not pass it.
if (calloutContext) {
- Ip::Address noAddr;
- noAddr.SetNoAddr();
ConnStateData * c = getConn();
- calloutContext->error = clientBuildError(ERR_ICAP_FAILURE,
Http::scInternalServerError,
- NULL,
- c != NULL ? c->clientConnection->remote :
noAddr,
- request
+ Comm::ConnectionPointer client = (c ? c->clientConnection : NULL);
+ calloutContext->error = ClientBuildError(ERR_ICAP_FAILURE,
Http::scInternalServerError,
+ NULL, client, request
);
#if USE_AUTH
calloutContext->error->auth_user_request =
=== modified file 'src/errorpage.cc'
--- src/errorpage.cc 2013-03-17 12:19:16 +0000
+++ src/errorpage.cc 2013-06-01 13:51:38 +0000
@@ -31,11 +31,13 @@
*/
#include "squid.h"
#include "cache_cf.h"
+#include "client_side_request.h"
#include "comm/Connection.h"
#include "comm/Write.h"
#include "disk.h"
#include "err_detail_type.h"
#include "errorpage.h"
+#include "format/Format.h"
#include "ftp.h"
#include "Store.h"
#include "html_quote.h"
@@ -568,7 +570,7 @@
return "ERR_UNKNOWN"; /* should not happen */
}
-ErrorState::ErrorState(err_type t, Http::StatusCode status, HttpRequest * req)
:
+ErrorState::ErrorState(err_type t, Http::StatusCode status, HttpRequest * req,
AccessLogEntry *ale) :
type(t),
page_id(t),
err_language(NULL),
@@ -582,7 +584,7 @@
port(0),
dnsError(),
ttl(0),
- src_addr(),
+ tcpClient(),
redirect_url(NULL),
callback(NULL),
callback_data(NULL),
@@ -591,7 +593,10 @@
#if USE_SSL
detail(NULL),
#endif
- detailCode(ERR_DETAIL_NONE)
+ detailCode(ERR_DETAIL_NONE),
+ building_deny_info_url(false),
+ allowRecursion(true),
+ ale_(ale)
{
memset(&ftp, 0, sizeof(ftp));
@@ -601,7 +606,6 @@
if (req != NULL) {
request = req;
HTTPMSGLOCK(request);
- src_addr = req->client_addr;
}
}
@@ -694,6 +698,9 @@
ErrorState::~ErrorState()
{
+ // ensure that the AccessLogEntry object has no reference to us
+ assert(!ale_ || ale_->errorPage == NULL);
+
HTTPMSGUNLOCK(request);
safe_free(redirect_url);
safe_free(url);
@@ -718,7 +725,6 @@
ErrorState::Dump(MemBuf * mb)
{
MemBuf str;
- char ntoabuf[MAX_IPSTRLEN];
str.reset();
/* email subject line */
@@ -746,7 +752,8 @@
str.Printf("TimeStamp: %s\r\n\r\n", mkrfc1123(squid_curtime));
/* - IP stuff */
- str.Printf("ClientIP: %s\r\n", src_addr.NtoA(ntoabuf,MAX_IPSTRLEN));
+// char ntoabuf[MAX_IPSTRLEN];
+//XXX str.Printf("ClientIP: %s\r\n", src_addr.NtoA(ntoabuf,MAX_IPSTRLEN));
if (request && request->hier.host[0] != '\0') {
str.Printf("ServerIP: %s\r\n", request->hier.host);
@@ -796,42 +803,18 @@
#define CVT_BUF_SZ 512
const char *
-ErrorState::Convert(char token, bool building_deny_info_url, bool
allowRecursion)
+ErrorState::Convert(Format::ByteCode_t token)
{
static MemBuf mb;
const char *p = NULL; /* takes priority over mb if set */
int do_quote = 1;
int no_urlescape = 0; /* if true then item is NOT to be further
URL-encoded */
- char ntoabuf[MAX_IPSTRLEN];
mb.reset();
switch (token) {
- case 'a':
-#if USE_AUTH
- if (request && request->auth_user_request != NULL)
- p = request->auth_user_request->username();
- if (!p)
-#endif
- p = "-";
- break;
-
- case 'b':
- mb.Printf("%d", getMyPort());
- break;
-
- case 'B':
- if (building_deny_info_url) break;
- p = request ? ftpUrlWith2f(request) : "[no URL]";
- break;
-
- case 'c':
- if (building_deny_info_url) break;
- p = errorPageName(type);
- break;
-
- case 'D':
+ case Format::LFT_ERR_PAGE_UD: // '%D'
if (!allowRecursion)
p = "%D"; // if recursion is not allowed, do not convert
#if USE_SSL
@@ -840,9 +823,11 @@
detail->useRequest(request);
const String &errDetail = detail->toString();
if (errDetail.defined()) {
- MemBuf *detail_mb = ConvertText(errDetail.termedBuf(), false);
- mb.append(detail_mb->content(), detail_mb->contentSize());
- delete detail_mb;
+ allowRecursion = false;
+ MemBuf detail_mb;
+ ConvertText(errDetail.termedBuf(), detail_mb);
+ allowRecursion = true;
+ mb.append(detail_mb.content(), detail_mb.contentSize());
do_quote = 0;
}
}
@@ -851,18 +836,18 @@
mb.Printf("[No Error Detail]");
break;
- case 'e':
+ case Format::LFT_ERR_PAGE_LE: // '%e'
mb.Printf("%d", xerrno);
break;
- case 'E':
+ case Format::LFT_ERR_PAGE_UE: // '%E'
if (xerrno)
mb.Printf("(%d) %s", xerrno, strerror(xerrno));
else
mb.Printf("[No Error]");
break;
- case 'f':
+ case Format::LFT_ERR_PAGE_LF: // '%f'
if (building_deny_info_url) break;
/* FTP REQUEST LINE */
if (ftp.request)
@@ -871,7 +856,7 @@
p = "nothing";
break;
- case 'F':
+ case Format::LFT_ERR_PAGE_UF: // '%F'
if (building_deny_info_url) break;
/* FTP REPLY LINE */
if (ftp.reply)
@@ -880,7 +865,7 @@
p = "nothing";
break;
- case 'g':
+ case Format::LFT_ERR_PAGE_LG: // '%g'
if (building_deny_info_url) break;
/* FTP SERVER RESPONSE */
if (ftp.listing) {
@@ -891,38 +876,13 @@
}
break;
- case 'h':
- mb.Printf("%s", getMyHostname());
- break;
-
- case 'H':
- if (request) {
- if (request->hier.host[0] != '\0') // if non-empty string.
- p = request->hier.host;
- else
- p = request->GetHost();
- } else if (!building_deny_info_url)
- p = "[unknown host]";
- break;
-
- case 'i':
- mb.Printf("%s", src_addr.NtoA(ntoabuf,MAX_IPSTRLEN));
- break;
-
- case 'I':
- if (request && request->hier.tcpServer != NULL)
- p = request->hier.tcpServer->remote.NtoA(ntoabuf,MAX_IPSTRLEN);
- else if (!building_deny_info_url)
- p = "[unknown]";
- break;
-
- case 'l':
+ case Format::LFT_ERR_PAGE_LL: // '%l'
if (building_deny_info_url) break;
mb.append(error_stylesheet.content(), error_stylesheet.contentSize());
do_quote = 0;
break;
- case 'L':
+ case Format::LFT_ERR_PAGE_UL: // '%L'
if (building_deny_info_url) break;
if (Config.errHtmlText) {
mb.Printf("%s", Config.errHtmlText);
@@ -931,7 +891,7 @@
p = "[not available]";
break;
- case 'm':
+ case Format::LFT_ERR_PAGE_LM: // '%m'
if (building_deny_info_url) break;
#if USE_AUTH
p = auth_user_request->denyMessage("[not available]");
@@ -940,28 +900,13 @@
#endif
break;
- case 'M':
- if (request)
- p = RequestMethodStr(request->method);
- else if (!building_deny_info_url)
- p= "[unknown method]";
- break;
-
- case 'o':
+ case Format::LFT_ERR_PAGE_LO: // '%o'
p = request ? request->extacl_message.termedBuf() :
external_acl_message;
if (!p && !building_deny_info_url)
p = "[not available]";
break;
- case 'p':
- if (request) {
- mb.Printf("%d", (int) request->port);
- } else if (!building_deny_info_url) {
- p = "[unknown port]";
- }
- break;
-
- case 'P':
+ case Format::LFT_ERR_PAGE_UP: // '%P'
if (request) {
p = AnyP::ProtocolType_str[request->protocol];
} else if (!building_deny_info_url) {
@@ -969,7 +914,7 @@
}
break;
- case 'R':
+ case Format::LFT_ERR_PAGE_UR: // '%R'
if (building_deny_info_url) {
p = (request->urlpath.size() != 0 ? request->urlpath.termedBuf() :
"/");
no_urlescape = 1;
@@ -999,7 +944,7 @@
}
break;
- case 's':
+ case Format::LFT_ERR_PAGE_LS: // '%s'
/* for backward compat we make %s show the full URL. Drop this in some
future release. */
if (building_deny_info_url) {
p = request ? urlCanonical(request) : url;
@@ -1008,7 +953,7 @@
p = visible_appname_string;
break;
- case 'S':
+ case Format::LFT_ERR_PAGE_US: // '%S'
if (building_deny_info_url) {
p = visible_appname_string;
break;
@@ -1029,49 +974,14 @@
}
break;
- case 't':
- mb.Printf("%s", Time::FormatHttpd(squid_curtime));
- break;
-
- case 'T':
- mb.Printf("%s", mkrfc1123(squid_curtime));
- break;
-
- case 'U':
- /* Using the fake-https version of canonical so error pages see
https:// */
- /* even when the url-path cannot be shown as more than '*' */
- if (request)
- p = urlCanonicalFakeHttps(request);
- else if (url)
- p = url;
- else if (!building_deny_info_url)
- p = "[no URL]";
- break;
-
- case 'u':
- if (request)
- p = urlCanonical(request);
- else if (url)
- p = url;
- else if (!building_deny_info_url)
- p = "[no URL]";
- break;
-
- case 'w':
- if (Config.adminEmail)
- mb.Printf("%s", Config.adminEmail);
- else if (!building_deny_info_url)
- p = "[unknown]";
- break;
-
- case 'W':
+ case Format::LFT_ERR_PAGE_UW: // '%W'
if (building_deny_info_url) break;
if (Config.adminEmail && Config.onoff.emailErrData)
Dump(&mb);
no_urlescape = 1;
break;
- case 'x':
+ case Format::LFT_ERR_PAGE_LX: // '%x'
#if USE_SSL
if (detail)
mb.Printf("%s", detail->errorName());
@@ -1081,7 +991,7 @@
p = "[Unknown Error Code]";
break;
- case 'z':
+ case Format::LFT_ERR_PAGE_LZ: // '%z'
if (building_deny_info_url) break;
if (dnsError.size() > 0)
p = dnsError.termedBuf();
@@ -1091,7 +1001,7 @@
p = "[unknown]";
break;
- case 'Z':
+ case Format::LFT_ERR_PAGE_UZ: // '%Z'
if (building_deny_info_url) break;
if (err_msg)
p = err_msg;
@@ -1099,13 +1009,7 @@
p = "[unknown]";
break;
- case '%':
- p = "%";
- break;
-
default:
- mb.Printf("%%%c", token);
- do_quote = 0;
break;
}
@@ -1128,24 +1032,17 @@
void
ErrorState::DenyInfoLocation(const char *name, HttpRequest *aRequest, MemBuf
&result)
{
+ // XXX: make this process much faster by pre-parsing the deny_info pattern
+ // and just assembling the Location: output here.
+
char const *m = name;
- char const *p = m;
- char const *t;
if (m[0] == '3')
m += 4; // skip "3xx:"
- while ((p = strchr(m, '%'))) {
- result.append(m, p - m); /* copy */
- t = Convert(*++p, true, true); /* convert */
- result.Printf("%s", t); /* copy */
- m = p + 1; /* advance */
- }
-
- if (*m)
- result.Printf("%s", m); /* copy tail */
-
- assert((size_t)result.contentSize() == strlen(result.content()));
+ building_deny_info_url = true;
+ ConvertText(m, result);
+ building_deny_info_url = false;
}
HttpReply *
@@ -1278,7 +1175,8 @@
debugs(4, 2, HERE << "No existing error page language negotiated for "
<< errorPageName(page_id) << ". Using default error file.");
}
- MemBuf *result = ConvertText(m, true);
+ MemBuf *result = new MemBuf;
+ ConvertText(m, *result);
#if USE_ERR_LOCALES
if (localeTmpl)
delete localeTmpl;
@@ -1286,25 +1184,53 @@
return result;
}
-MemBuf *ErrorState::ConvertText(const char *text, bool allowRecursion)
+void
+ErrorState::ConvertText(const char *text, MemBuf &buf)
{
- MemBuf *content = new MemBuf;
- const char *p;
- const char *m = text;
- assert(m);
- content->init();
-
- while ((p = strchr(m, '%'))) {
- content->append(m, p - m); /* copy */
- const char *t = Convert(*++p, false, allowRecursion); /* convert */
- content->Printf("%s", t); /* copy */
- m = p + 1; /* advance */
- }
-
- if (*m)
- content->Printf("%s", m); /* copy tail */
-
- assert((size_t)content->contentSize() == strlen(content->content()));
-
- return content;
+ // if request exists, we might have access to the transaction ALE object
+ if (ale_ == NULL && request != NULL &&
request->clientConnectionManager.valid() &&
+ request->clientConnectionManager->currentobject != NULL) {
+ ClientSocketContext::Pointer c =
request->clientConnectionManager->currentobject;
+ ale_ = c->http->al;
+ }
+
+ // if we are in a recursive pattern (%D or %S) ALE may already be setup
+ // or if the creator of this ErrorState was kind enough to supply the real
transaction ALE object
+ if (ale_ == NULL) {
+ ale_ = new AccessLogEntry();
+
+ // fill ale_ with all the details Format::assemble() will need to
process error page codes.
+
+ if (request) {
+ // XXX: we only know about one request here.
+ ale_->request = request;
+ HTTPMSGLOCK(request);
+ ale_->adapted_request = request;
+ HTTPMSGLOCK(request);
+ ale_->_private.method_str = RequestMethodStr(request->method);
+ const char *hierHost = (request->hier.host[0] != '\0' ?
request->hier.host : request->GetHost());
+ ale_->hier.note(request->hier.tcpServer, hierHost);
+
+ if (request->clientConnectionManager.valid())
+ ale_->tcpClient =
request->clientConnectionManager->clientConnection;
+ }
+
+ // if there is no request information (error before parsing it)
+ if (ale_->tcpClient == NULL)
+ ale_->tcpClient = tcpClient;
+
+ ale_->url = (request ? urlCanonicalFakeHttps(request) : url);
+ }
+
+ buf.init();
+
+ // NP: let fmt.assemble() have temporary access to 'this' state info
+ ale_->errorPage = cbdataReference(this);
+
+ // parse the text using Format:: parser
+ Format::Format fmt("errorPageTemporary");
+ fmt.parse(text);
+ fmt.assemble(buf, ale_, 0);
+
+ cbdataReferenceDone(ale_->errorPage);
}
=== modified file 'src/errorpage.h'
--- src/errorpage.h 2013-03-16 04:57:43 +0000
+++ src/errorpage.h 2013-04-19 03:41:32 +0000
@@ -34,10 +34,12 @@
#ifndef SQUID_ERRORPAGE_H
#define SQUID_ERRORPAGE_H
+#include "AccessLogEntry.h"
#include "cbdata.h"
#include "comm/forward.h"
#include "err_detail_type.h"
#include "err_type.h"
+#include "format/ByteCode.h"
#include "http/StatusCode.h"
#include "ip/Address.h"
#include "SquidString.h"
@@ -97,7 +99,7 @@
class ErrorState
{
public:
- ErrorState(err_type type, Http::StatusCode, HttpRequest * request);
+ ErrorState(err_type type, Http::StatusCode, HttpRequest * request,
AccessLogEntry *ale = NULL);
ErrorState(); // not implemented.
~ErrorState();
@@ -109,6 +111,17 @@
/// set error type-specific detail code
void detailError(int dCode) {detailCode = dCode;}
+ /** \deprecated
+ * Map some Error page and deny_info template % codes into textual output.
+ * Only the LFT_ERR_PAGE_* ones are mapped, others are ignored.
+ *
+ * Several of the codes produce blocks of non-URL compatible results.
+ * When processing the deny_info location URL they will be skipped.
+ *
+ * \param token The token following % which need to be
converted
+ */
+ const char *Convert(Format::ByteCode_t token);
+
private:
/**
* Locates error page template to be used for this error
@@ -119,10 +132,10 @@
/**
* Convert the given template string into textual output
*
- * \param text The string to be converted
- * \param allowRecursion Whether to convert codes which output may
contain codes
+ * \param text The string to be converted
+ * \param buf The buffer to be filled. Will be init()'d before use.
*/
- MemBuf *ConvertText(const char *text, bool allowRecursion);
+ void ConvertText(const char *text, MemBuf &buf);
/**
* Generates the Location: header value for a deny_info error page
@@ -131,18 +144,6 @@
void DenyInfoLocation(const char *name, HttpRequest *request, MemBuf
&result);
/**
- * Map the Error page and deny_info template % codes into textual output.
- *
- * Several of the codes produce blocks of non-URL compatible results.
- * When processing the deny_info location URL they will be skipped.
- *
- * \param token The token following % which need to be
converted
- * \param building_deny_info_url Perform special deny_info actions, such
as URL-encoding and token skipping.
- * \ allowRecursion True if the codes which do recursions should
converted
- */
- const char *Convert(char token, bool building_deny_info_url, bool
allowRecursion);
-
- /**
* CacheManager / Debug dump of the ErrorState object.
* Writes output into the given MemBuf.
\retval 0 successful completion.
@@ -164,7 +165,9 @@
String dnsError; ///< DNS lookup error message
time_t ttl;
- Ip::Address src_addr;
+ /// client TCP connection details, for use when request does not exist.
+ Comm::ConnectionPointer tcpClient;
+
char *redirect_url;
ERCB *callback;
void *callback_data;
@@ -186,7 +189,17 @@
/// type-specific detail about the transaction error;
/// overwrites xerrno; overwritten by detail, if any.
int detailCode;
+
private:
+ /// Perform special deny_info actions, such as URL-encoding and token
skipping.
+ bool building_deny_info_url;
+
+ /// Whether to convert codes which output may contain codes
+ bool allowRecursion;
+
+ /// an AccessLogEntry we are using to generate output
+ AccessLogEntry::Pointer ale_;
+
CBDATA_CLASS2(ErrorState);
};
=== modified file 'src/esi/Esi.cc'
--- src/esi/Esi.cc 2013-03-18 04:55:51 +0000
+++ src/esi/Esi.cc 2013-06-02 00:41:38 +0000
@@ -1451,8 +1451,6 @@
/* don't touch incoming, it's a pointer into buffered anyway */
}
-ErrorState *clientBuildError (err_type, Http::StatusCode, char const *,
Ip::Address &, HttpRequest *);
-
/* This can ONLY be used before we have sent *any* data to the client */
void
ESIContext::fail ()
@@ -1469,7 +1467,7 @@
flags.error = 1;
/* create an error object */
// XXX: with the in-direction on remote IP. does the
http->getConn()->clientConnection exist?
- ErrorState * err = clientBuildError(errorpage, errorstatus, NULL,
http->getConn()->clientConnection->remote, http->request);
+ ErrorState * err = ClientBuildError(errorpage, errorstatus, NULL,
http->getConn()->clientConnection, http->request);
err->err_msg = errormessage;
errormessage = NULL;
rep = err->BuildHttpReply();
=== modified file 'src/format/ByteCode.h'
--- src/format/ByteCode.h 2013-02-11 23:11:12 +0000
+++ src/format/ByteCode.h 2013-04-19 00:56:57 +0000
@@ -136,6 +136,7 @@
LFT_TIME_SUBSECOND,
LFT_TIME_LOCALTIME,
LFT_TIME_GMT,
+ LFT_TIME_GMT_RFC1123,
/* processing time details */
LFT_TIME_TO_HANDLE_REQUEST,
@@ -143,6 +144,12 @@
LFT_TOTAL_SERVER_SIDE_RESPONSE_TIME,
LFT_DNS_WAIT_TIME,
+ /* Squid application details */
+ LFT_SQUID_KID_HOSTNAME,
+ LFT_SQUID_KID_PORT,
+ LFT_SQUID_KID_APPSTRING,
+ LFT_SQUID_KID_ADMINEMAIL,
+
/* Squid internal processing details */
LFT_SQUID_STATUS,
LFT_SQUID_ERROR,
@@ -196,6 +203,26 @@
LFT_SSL_USER_CERT_ISSUER,
#endif
+ /* Squid Error Page special items U=Upper, L=lower cased alphabet codes */
+ LFT_ERR_PAGE_UD,
+ LFT_ERR_PAGE_LE,
+ LFT_ERR_PAGE_UE,
+ LFT_ERR_PAGE_LF,
+ LFT_ERR_PAGE_UF,
+ LFT_ERR_PAGE_LG,
+ LFT_ERR_PAGE_LL,
+ LFT_ERR_PAGE_UL,
+ LFT_ERR_PAGE_LM,
+ LFT_ERR_PAGE_LO,
+ LFT_ERR_PAGE_UP,
+ LFT_ERR_PAGE_UR,
+ LFT_ERR_PAGE_LS,
+ LFT_ERR_PAGE_US,
+ LFT_ERR_PAGE_UW,
+ LFT_ERR_PAGE_LX,
+ LFT_ERR_PAGE_LZ,
+ LFT_ERR_PAGE_UZ,
+
LFT_NOTE,
LFT_PERCENT /* special string cases for escaped
chars */
} ByteCode_t;
=== modified file 'src/format/Format.cc'
--- src/format/Format.cc 2013-05-24 23:40:50 +0000
+++ src/format/Format.cc 2013-06-01 12:59:51 +0000
@@ -9,11 +9,14 @@
#include "format/Quoting.h"
#include "format/Token.h"
#include "fqdncache.h"
+#include "globals.h"
#include "HttpRequest.h"
#include "MemBuf.h"
#include "rfc1738.h"
+#include "SquidConfig.h"
#include "SquidTime.h"
#include "Store.h"
+#include "tools.h"
#include "URL.h"
#if USE_SSL
#include "ssl/ErrorDetail.h"
@@ -442,7 +445,7 @@
break;
case LFT_TIME_LOCALTIME:
-
+ case LFT_TIME_GMT_RFC1123:
case LFT_TIME_GMT: {
const char *spec;
@@ -453,6 +456,12 @@
if (!spec)
spec = "%d/%b/%Y:%H:%M:%S %z";
t = localtime(&squid_curtime);
+
+ } else if (fmt->type == LFT_TIME_GMT_RFC1123) {
+ if (!spec)
+ spec = "%a, %d %b %Y %H:%M:%S GMT";
+ t = gmtime(&squid_curtime);
+
} else {
if (!spec)
spec = "%d/%b/%Y:%H:%M:%S";
@@ -825,6 +834,23 @@
// or internal error messages).
break;
+ case LFT_SQUID_KID_HOSTNAME:
+ out = getMyHostname();
+ break;
+
+ case LFT_SQUID_KID_PORT:
+ outint = getMyPort();
+ doint = 1;
+ break;
+
+ case LFT_SQUID_KID_APPSTRING:
+ out = visible_appname_string;
+ break;
+
+ case LFT_SQUID_KID_ADMINEMAIL:
+ out = Config.adminEmail;
+ break;
+
case LFT_SQUID_STATUS:
if (al->http.timedout || al->http.aborted) {
snprintf(tmp, sizeof(tmp), "%s%s", LogTags_str[al->cache.code],
@@ -866,6 +892,31 @@
}
break;
+ case LFT_ERR_PAGE_UD:
+ case LFT_ERR_PAGE_LE:
+ case LFT_ERR_PAGE_UE:
+ case LFT_ERR_PAGE_LF:
+ case LFT_ERR_PAGE_UF:
+ case LFT_ERR_PAGE_LG:
+ case LFT_ERR_PAGE_LL:
+ case LFT_ERR_PAGE_UL:
+ case LFT_ERR_PAGE_LM:
+ case LFT_ERR_PAGE_LO:
+ case LFT_ERR_PAGE_UP:
+ case LFT_ERR_PAGE_UR:
+ case LFT_ERR_PAGE_LS:
+ case LFT_ERR_PAGE_US:
+ case LFT_ERR_PAGE_UW:
+ case LFT_ERR_PAGE_LX:
+ case LFT_ERR_PAGE_LZ:
+ case LFT_ERR_PAGE_UZ:
+ // NP: these code types are used only by the ErrorState page
generator
+ // for backward compatibility use the old code to generate the
display output
+ if (al->errorPage) {
+ out = al->errorPage->Convert(fmt->type);
+ }
+ break;
+
case LFT_SQUID_HIERARCHY:
if (al->hier.ping.timedout)
mb.append("TIMEOUT_", 8);
=== modified file 'src/format/Token.cc'
--- src/format/Token.cc 2013-05-22 01:04:34 +0000
+++ src/format/Token.cc 2013-06-01 12:59:51 +0000
@@ -1,4 +1,5 @@
#include "squid.h"
+#include "base/StringArea.h"
#include "format/Config.h"
#include "format/Token.h"
#include "format/TokenTableEntry.h"
@@ -31,6 +32,46 @@
{"%", LFT_PERCENT},
+ // 1-byte Tokens used by the Error page templates
+
+ // these ones map easily ..
+ {"a", LFT_USER_LOGIN},
+ {"b", LFT_SQUID_KID_PORT},
+ {"B", LFT_SERVER_REQ_URI},
+ {"c", LFT_SQUID_ERROR},
+ /*{"d", LFT_TIME_TO_HANDLE_REQUEST},*/
+ {"h", LFT_SQUID_KID_HOSTNAME},
+ {"H", LFT_SERVER_FQDN_OR_PEER_NAME},
+ {"i", LFT_CLIENT_IP_ADDRESS},
+ {"I", LFT_SERVER_IP_ADDRESS},
+ {"M", LFT_REQUEST_METHOD},
+ {"p", LFT_SERVER_PORT},
+ {"t", LFT_TIME_LOCALTIME},
+ {"T", LFT_TIME_GMT_RFC1123},
+ {"u", LFT_CLIENT_REQ_URI},
+ {"U", LFT_REQUEST_URI},
+ {"w", LFT_SQUID_KID_ADMINEMAIL},
+
+ // NP: this bunch are specific to the ErrorState object generator
+ {"D", LFT_ERR_PAGE_UD},
+ {"e", LFT_ERR_PAGE_LE},
+ {"E", LFT_ERR_PAGE_UE},
+ {"f", LFT_ERR_PAGE_LF},
+ {"F", LFT_ERR_PAGE_UF},
+ {"g", LFT_ERR_PAGE_LG},
+ {"l", LFT_ERR_PAGE_LL},
+ {"L", LFT_ERR_PAGE_UL},
+ {"m", LFT_ERR_PAGE_LM},
+ {"o", LFT_ERR_PAGE_LO},
+ {"P", LFT_ERR_PAGE_UP},
+ {"R", LFT_ERR_PAGE_UR},
+ {"s", LFT_ERR_PAGE_LS},
+ {"S", LFT_ERR_PAGE_US},
+ {"W", LFT_ERR_PAGE_UW},
+ {"x", LFT_ERR_PAGE_LX},
+ {"z", LFT_ERR_PAGE_LZ},
+ {"Z", LFT_ERR_PAGE_UZ},
+
{NULL, LFT_NONE} /* this must be last */
};
@@ -373,7 +414,12 @@
}
if (type == LFT_NONE) {
- fatalf("Can't parse configuration token: '%s'\n", def);
+ debugs(46, 3, "Can't parse configuration token: '" <<
StringArea(def,min(strlen(def), static_cast<size_t>(10))) << "'");
+ // NP: unknown token found. Assume it should have been %% instead of %.
+ if (def[0] == '%') {
+ type = LFT_PERCENT;
+ ++cur;
+ }
}
if (*cur == ' ') {
=== modified file 'src/redirect.cc'
--- src/redirect.cc 2013-04-04 06:15:00 +0000
+++ src/redirect.cc 2013-04-19 13:10:43 +0000
@@ -292,13 +292,9 @@
clientStreamNode *node = (clientStreamNode
*)http->client_stream.tail->prev->data;
clientReplyContext *repContext = dynamic_cast<clientReplyContext
*>(node->data.getRaw());
assert (repContext);
- Ip::Address tmpnoaddr;
- tmpnoaddr.SetNoAddr();
+ Comm::ConnectionPointer client = http->getConn() != NULL ?
http->getConn()->clientConnection : NULL;
repContext->setReplyToError(ERR_GATEWAY_FAILURE, status,
- http->request->method, NULL,
- http->getConn() != NULL &&
http->getConn()->clientConnection != NULL ?
- http->getConn()->clientConnection->remote
: tmpnoaddr,
- http->request,
+ http->request->method, NULL, client,
http->request,
NULL,
#if USE_AUTH
http->getConn() != NULL &&
http->getConn()->getAuth() != NULL ?
