On 9/9/2012 2:54 PM, Amos Jeffries wrote:
On 9/09/2012 10:08 p.m., Eliezer Croitoru wrote:
I took the url_rewrite helper and used what ever seems needed to
create a url_store_rewrite helper.
I also added the needed variables for the next step and freeing them
where and if needed.
this is a more of a basic sketch.
Eliezer
You dont need all the "added for storeurl", "changed X" comments, they
just bloat the code and patch.
OK,these was more for me.
Recommendations on how to manage myself in such huge code are more then
welcome.(using eclipse)
Please don't add multiple empty lines in a row. This is happening in
several places.
will be fixed.
src/client_side_request.cc:
* in ClientRequestContext::clientStoreurlStart the else condition is
what gets done if there is no access list configured. This should be
defaulting to *not* changing the store-URL. call
clientStoreurlDone(NULL) instead of storeurlStart()
Done.
* in doCallouts() please move the store-url bits to after adaptation and
URL-redirect bits. Those checks may result in a non-storable response
being generated by Squid straight to the client.
I wasn't sure about it, Thanks.
++ It is questionable whether the store-URL should also be below the
no_cache_done bits as well. Since requests with"cache deny" are not
going to be loaded from cache, but maybe they will be stored later by
future Squid (which will need store-URL changes then) - in current squid
they are useless passing to the storeurl helper.
When we will be there then...
Are there any visible plans?
src/client_side_request.h:
* please name the new variable "store_uri" in line with the other
surrounding variable naming style.
Done
src/redirect.cc:
* why is storeurlHandleReply() being added? all it does is receive the
reply and call a callback handler. It is that handler which is the only
part store-url specific and set by storeurlStart. If it was just the
debugs, they should be changed to debugs(61, 5, HERE << "{...
Thanks. applied
* in redirectInit() you are using new on the redirectors variable when
it should be the storeurl_rewriters variable.
oops. ~0-0~
Fixed
To answer your query comment... CBDATA_INIT_TYPE does memory allocation
and new/delete method definition setup for the redirectStateData
objects. Since they are shared by both systems you can ignore that part
of redirectInit().
Although the static variable should be moved down
next to the if(init) and changed to a bool type - that is irrelevant to
the storeurl stuff though.
Well i'm already here so..
* in storeUrlStart() the INTERNAL_SERVER_ERROR log message is still
saying "redirector"
fixed
Fix those and I think this is a good part-1 patch that can be applied to
trunk.
Amos
--
Eliezer Croitoru
https://www1.ngtech.co.il
IT consulting for Nonprofit organizations
eliezer <at> ngtech.co.il
=== modified file 'src/ClientRequestContext.h'
--- src/ClientRequestContext.h 2012-08-14 11:53:07 +0000
+++ src/ClientRequestContext.h 2012-09-09 12:52:34 +0000
@@ -33,6 +33,8 @@
void clientAccessCheckDone(const allow_t &answer);
void clientRedirectStart();
void clientRedirectDone(char *result);
+ void clientStoreurlStart();
+ void clientStoreurlDone(char *result);
void checkNoCache();
void checkNoCacheDone(const allow_t &answer);
#if USE_ADAPTATION
@@ -53,7 +55,9 @@
ClientHttpRequest *http;
ACLChecklist *acl_checklist; /* need ptr back so we can unreg if needed */
int redirect_state;
+ int storeurl_state;
+ bool storeurl_done;
bool host_header_verify_done;
bool http_access_done;
bool adapted_http_access_done;
=== modified file 'src/client_side.cc'
--- src/client_side.cc 2012-09-06 13:12:26 +0000
+++ src/client_side.cc 2012-09-09 12:54:28 +0000
@@ -714,6 +714,7 @@
ClientHttpRequest::freeResources()
{
safe_free(uri);
+ safe_free(store_uri);
safe_free(log_uri);
safe_free(redirect.location);
range_iter.boundary.clean();
=== modified file 'src/client_side_request.cc'
--- src/client_side_request.cc 2012-08-28 19:12:13 +0000
+++ src/client_side_request.cc 2012-09-09 13:01:23 +0000
@@ -131,6 +131,7 @@
static int clientHierarchical(ClientHttpRequest * http);
static void clientInterpretRequestHeaders(ClientHttpRequest * http);
static RH clientRedirectDoneWrapper;
+static RH clientStoreurlDoneWrapper;
static void checkNoCacheDoneWrapper(allow_t, void *);
extern "C" CSR clientGetMoreData;
extern "C" CSS clientReplyStatus;
@@ -151,10 +152,11 @@
debugs(85,3, HERE << this << " ClientRequestContext destructed");
}
-ClientRequestContext::ClientRequestContext(ClientHttpRequest *anHttp) : http(cbdataReference(anHttp)), acl_checklist (NULL), redirect_state (REDIRECT_NONE), error(NULL), readNextRequest(false)
+ClientRequestContext::ClientRequestContext(ClientHttpRequest *anHttp) : http(cbdataReference(anHttp)), acl_checklist (NULL), redirect_state (REDIRECT_NONE), storeurl_state (REDIRECT_NONE), error(NULL), readNextRequest(false)
{
http_access_done = false;
redirect_done = false;
+ storeurl_done = false;
no_cache_done = false;
interpreted_req_hdrs = false;
#if USE_SSL
@@ -916,6 +918,31 @@
redirectStart(http, clientRedirectDoneWrapper, this);
}
+static void
+clientStoreurlAccessCheckDone(allow_t answer, void *data)
+{
+ ClientRequestContext *context = (ClientRequestContext *)data;
+ ClientHttpRequest *http = context->http;
+ context->acl_checklist = NULL;
+
+ if (answer == ACCESS_ALLOWED)
+ storeurlStart(http, clientStoreurlDoneWrapper, context);
+ else
+ context->clientStoreurlDone(NULL);
+}
+
+void
+ClientRequestContext::clientStoreurlStart()
+{
+ debugs(33, 5, HERE << "' ClientRequestContext::clientStoreurlStart " << http->uri << "'");
+
+ if (Config.accessList.storeurl) {
+ acl_checklist = clientAclChecklistCreate(Config.accessList.storeurl, http);
+ acl_checklist->nonBlockingCheck(clientStoreurlAccessCheckDone, this);
+ } else
+ clientStoreurlDone(NULL)
+}
+
static int
clientHierarchical(ClientHttpRequest * http)
{
@@ -1262,6 +1289,37 @@
http->doCallouts();
}
+void
+clientStoreurlDoneWrapper(void *data, char *result)
+{
+ ClientRequestContext *calloutContext = (ClientRequestContext *)data;
+
+ if (!calloutContext->httpStateIsValid())
+ return;
+
+ calloutContext->clientStoreurlDone(result);
+}
+
+void
+ClientRequestContext::clientStoreurlDone(char *result)
+{
+ debugs(85, 5, "clientStoreurlDone: '" << http->uri << "' result=" << (result ? result : "NULL"));
+ assert(storeurl_state == REDIRECT_PENDING);
+ storeurl_state = REDIRECT_DONE;
+
+ /*
+ * the next step is to implement here the actuall store_url_rewrite proccess of pushing
+ * the store_url into thr http request.
+ */
+ if (result) {
+ debugs(85, DBG_CRITICAL, "A result for storu_url accepted: " << result);
+ } else {
+ debugs(85, DBG_CRITICAL, "A result for storu_url was not accepted: " << result);
+ }
+
+ http->doCallouts();
+}
+
/** Test cache allow/deny configuration
* Sets flags.cachable=1 if caching is not denied.
*/
@@ -1689,6 +1747,18 @@
if (ih != NULL)
ih->logType = logType;
#endif
+
+ if (!calloutContext->storeurl_done) {
+ calloutContext->storeurl_done = true;
+ assert(calloutContext->storeurl_state == REDIRECT_NONE);
+
+ if (Config.Program.storeurl) {
+ debugs(83, 3, HERE << "Doing calloutContext->clientSotreurlStart()");
+ calloutContext->storeurl_state = REDIRECT_PENDING;
+ calloutContext->clientStoreurlStart();
+ return;
+ }
+ }
}
#if !_USE_INLINE_
=== modified file 'src/client_side_request.h'
--- src/client_side_request.h 2012-08-28 19:12:13 +0000
+++ src/client_side_request.h 2012-09-09 13:03:15 +0000
@@ -98,6 +98,7 @@
HttpRequest *request; /* Parsed URL ... */
char *uri;
char *log_uri;
+ char *store_uri;
struct {
int64_t offset;
@@ -209,6 +210,7 @@
/* ones that should be elsewhere */
extern void redirectStart(ClientHttpRequest *, RH *, void *);
extern void tunnelStart(ClientHttpRequest *, int64_t *, int *);
+extern void storeurlStart(ClientHttpRequest *, RH *, void *);
#if _USE_INLINE_
#include "Store.h"
=== modified file 'src/redirect.cc'
--- src/redirect.cc 2012-08-28 19:12:13 +0000
+++ src/redirect.cc 2012-09-09 13:04:48 +0000
@@ -71,8 +71,11 @@
static HLPCB redirectHandleReply;
static void redirectStateFree(redirectStateData * r);
static helper *redirectors = NULL;
+static helper *storeurl_rewriters = NULL;
static OBJH redirectStats;
+static OBJH storeurlStats;
static int n_bypassed = 0;
+static int n_storeurl_bypassed = 0;
CBDATA_TYPE(redirectStateData);
static void
@@ -81,7 +84,7 @@
redirectStateData *r = static_cast<redirectStateData *>(data);
char *t;
void *cbdata;
- debugs(61, 5, "redirectHandleRead: {" << (reply && *reply != '\0' ? reply : "<NULL>") << "}");
+ debugs(61, 5, HERE << (reply && *reply != '\0' ? reply : "<NULL>") << "}");
if (reply) {
if ((t = strchr(reply, ' ')))
@@ -119,6 +122,21 @@
"because all redirectors were busy: %d\n", n_bypassed);
}
+static void
+storeurlStats(StoreEntry * sentry)
+{
+ if (storeurl_rewriters == NULL) {
+ storeAppendPrintf(sentry, "No Store Url Rewritters defined\n");
+ return;
+ }
+
+ helperStats(sentry, storeurl_rewriters, "Store Url Rewriters Statistics");
+
+ if (Config.onoff.storeurl_bypass)
+ storeAppendPrintf(sentry, "\nNumber of requests bypassed "
+ "because all Store Url Rewriters were busy: %d\n", n_storeurl_bypassed);
+}
+
/**** PUBLIC FUNCTIONS ****/
void
@@ -237,46 +255,171 @@
redirectRegisterWithCacheManager(void)
{
Mgr::RegisterAction("redirector", "URL Redirector Stats", redirectStats, 0, 1);
+ Mgr::RegisterAction("storeurl", "Store URL Rewritter Stats", storeurlStats, 0, 1);
}
void
redirectInit(void)
{
- static int init = 0;
-
redirectRegisterWithCacheManager();
-
- if (!Config.Program.redirect)
+ if (!Config.Program.redirect && !Config.Program.storeurl)
return;
- if (redirectors == NULL)
- redirectors = new helper("redirector");
-
- redirectors->cmdline = Config.Program.redirect;
-
- redirectors->childs.updateLimits(Config.redirectChildren);
-
- redirectors->ipc_type = IPC_STREAM;
-
- helperOpenServers(redirectors);
-
- if (!init) {
- init = 1;
- CBDATA_INIT_TYPE(redirectStateData);
- }
+ if (Config.Program.redirect){
+ if (redirectors == NULL)
+ redirectors = new helper("redirector");
+
+ redirectors->cmdline = Config.Program.redirect;
+ redirectors->childs.updateLimits(Config.redirectChildren);
+ redirectors->ipc_type = IPC_STREAM;
+ helperOpenServers(redirectors);
+ }
+
+ if (Config.Program.storeurl){
+ if (storeurl_rewriters == NULL)
+ storeurl_rewriters = new helper("storeurl");
+
+ storeurl_rewriters->cmdline = Config.Program.storeurl;
+ storeurl_rewriters->childs.updateLimits(Config.storeurlChildren);
+ storeurl_rewriters->ipc_type = IPC_STREAM;
+ helperOpenServers(storeurl_rewriters);
+ }
+ static bool init = false;
+ if (init) {
+ init = true;
+ CBDATA_INIT_TYPE(redirectStateData);
+ }
}
void
redirectShutdown(void)
{
- if (!redirectors)
+ if (!redirectors && !storeurl_rewriters)
return;
- helperShutdown(redirectors);
+ if (redirectors)
+ helperShutdown(redirectors);
+
+ if (storeurl_rewriters)
+ helperShutdown(storeurl_rewriters);
if (!shutting_down)
return;
delete redirectors;
redirectors = NULL;
+ delete storeurl_rewriters;
+ storeurl_rewriters = NULL;
+}
+
+void
+storeurlStart(ClientHttpRequest * http, RH * handler, void *data)
+{
+ ConnStateData * conn = http->getConn();
+ redirectStateData *r = NULL;
+ const char *fqdn;
+ char buf[MAX_REDIRECTOR_REQUEST_STRLEN];
+ int sz;
+ http_status status;
+ char claddr[MAX_IPSTRLEN];
+ char myaddr[MAX_IPSTRLEN];
+ assert(http);
+ assert(handler);
+ debugs(61, 5, "storeurlStart: '" << http->uri << "'");
+
+ if (Config.onoff.storeurl_bypass && storeurl_rewriters->stats.queue_size) {
+ /* Skip store_url_rewritter if there is one request queued */
+ ++n_storeurl_bypassed;
+ handler(data, NULL);
+ return;
+ }
+
+ r = cbdataAlloc(redirectStateData);
+ r->orig_url = xstrdup(http->uri);
+ if (conn != NULL)
+ r->client_addr = conn->log_addr;
+ else
+ r->client_addr.SetNoAddr();
+ r->client_ident = NULL;
+#if USE_AUTH
+ if (http->request->auth_user_request != NULL) {
+ r->client_ident = http->request->auth_user_request->username();
+ debugs(61, 5, HERE << "auth-user=" << (r->client_ident?r->client_ident:"NULL"));
+ }
+#endif
+
+ // HttpRequest initializes with null_string. So we must check both defined() and size()
+ if (!r->client_ident && http->request->extacl_user.defined() && http->request->extacl_user.size()) {
+ r->client_ident = http->request->extacl_user.termedBuf();
+ debugs(61, 5, HERE << "acl-user=" << (r->client_ident?r->client_ident:"NULL"));
+ }
+
+ if (!r->client_ident && conn != NULL && conn->clientConnection != NULL && conn->clientConnection->rfc931[0]) {
+ r->client_ident = conn->clientConnection->rfc931;
+ debugs(61, 5, HERE << "ident-user=" << (r->client_ident?r->client_ident:"NULL"));
+ }
+
+#if USE_SSL
+
+ if (!r->client_ident && conn != NULL && Comm::IsConnOpen(conn->clientConnection)) {
+ r->client_ident = sslGetUserEmail(fd_table[conn->clientConnection->fd].ssl);
+ debugs(61, 5, HERE << "ssl-user=" << (r->client_ident?r->client_ident:"NULL"));
+ }
+#endif
+
+ if (!r->client_ident)
+ r->client_ident = dash_str;
+
+ r->method_s = RequestMethodStr(http->request->method);
+
+ r->handler = handler;
+
+ r->data = cbdataReference(data);
+
+ if ((fqdn = fqdncache_gethostbyaddr(r->client_addr, 0)) == NULL)
+ fqdn = dash_str;
+
+ sz = snprintf(buf, MAX_REDIRECTOR_REQUEST_STRLEN, "%s %s/%s %s %s myip=%s myport=%d\n",
+ r->orig_url,
+ r->client_addr.NtoA(claddr,MAX_IPSTRLEN),
+ fqdn,
+ r->client_ident[0] ? rfc1738_escape(r->client_ident) : dash_str,
+ r->method_s,
+ http->request->my_addr.NtoA(myaddr,MAX_IPSTRLEN),
+ http->request->my_addr.GetPort());
+
+ if ((sz<=0) || (sz>=MAX_REDIRECTOR_REQUEST_STRLEN)) {
+ if (sz<=0) {
+ status = HTTP_INTERNAL_SERVER_ERROR;
+ debugs(61, DBG_CRITICAL, "ERROR: Gateway Failure. Can not build request to be passed to storeurl rewriter. Request ABORTED.");
+ } else {
+ status = HTTP_REQUEST_URI_TOO_LARGE;
+ debugs(61, DBG_CRITICAL, "ERROR: Gateway Failure. Request passed to storeurl rewriter exceeds MAX_REDIRECTOR_REQUEST_STRLEN (" << MAX_REDIRECTOR_REQUEST_STRLEN << "). Request ABORTED.");
+ }
+
+ clientStreamNode *node = (clientStreamNode *)http->client_stream.tail->prev->data;
+ clientReplyContext *repContext = dynamic_cast<clientReplyContext *>(node->data.getRaw());
+ assert (repContext);
+ Ip::Address tmpnoaddr;
+ tmpnoaddr.SetNoAddr();
+ repContext->setReplyToError(ERR_GATEWAY_FAILURE, status,
+ http->request->method, NULL,
+ http->getConn() != NULL && http->getConn()->clientConnection != NULL ?
+ http->getConn()->clientConnection->remote : tmpnoaddr,
+ http->request,
+ NULL,
+#if USE_AUTH
+ http->getConn() != NULL && http->getConn()->auth_user_request != NULL ?
+ http->getConn()->auth_user_request : http->request->auth_user_request);
+#else
+ NULL);
+#endif
+
+ node = (clientStreamNode *)http->client_stream.tail->data;
+ clientStreamRead(node, http, node->readBuffer);
+ return;
+ }
+
+ debugs(61,6, HERE << "sending '" << buf << "' to the helper");
+ helperSubmit(storeurl_rewriters, buf, redirectHandleReply, r);
}
=== modified file 'src/structs.h'
--- src/structs.h 2012-08-28 13:00:30 +0000
+++ src/structs.h 2012-09-09 13:09:18 +0000
@@ -306,6 +306,8 @@
#endif
wordlist *redirect;
+ wordlist *storeurl;
+
#if USE_UNLINKD
char *unlinkd;
@@ -323,6 +325,8 @@
#endif
HelperChildConfig redirectChildren;
+ HelperChildConfig storeurlChildren;
+
time_t authenticateGCInterval;
time_t authenticateTTL;
time_t authenticateIpTTL;
@@ -421,6 +425,7 @@
int nonhierarchical_direct;
int strip_query_terms;
int redirector_bypass;
+ int storeurl_bypass;
int ignore_unknown_nameservers;
int client_pconns;
int server_pconns;
@@ -484,6 +489,7 @@
acl_access *brokenPosts;
#endif
acl_access *redirector;
+ acl_access *storeurl;
acl_access *reply;
acl_address *outgoing_address;
#if USE_HTCP
@@ -983,6 +989,7 @@
1; /* this should be killed, also in httpstateflags */
unsigned int refresh:1;
unsigned int redirected:1;
+ unsigned int storeurl_rewritted:1;
unsigned int need_validation:1;
unsigned int fail_on_validation_err:1; ///< whether we should fail if validation fails
unsigned int stale_if_hit:1; ///< reply is stale if it is a hit
