- For now I am not changing the mem-obj->url and leave the branch as it is since there is no need for it now.
Once the feature will be fine The change will be placed.

- The patch adds the helper basic surrounding code and a bit action code to make sure it is not breaking anything.

- The point that changes everything is at the StoreEntry creation which uses the StoreUrl and not the originalUrl.

It makes everything simpler since there is not one check that actually should be done to validate the url else then what already in squid by store code.

- There is a change in how helpers are started and shutdown to support multiple helpers.

Responses are more then welcome.

Eliezer
--
Eliezer Croitoru
https://www1.ngtech.co.il
sip:ngt...@sip2sip.info
IT consulting for Nonprofit organizations
eliezer <at> ngtech.co.il
=== modified file 'src/ClientRequestContext.h'
--- src/ClientRequestContext.h  2012-11-30 11:08:47 +0000
+++ src/ClientRequestContext.h  2012-12-01 11:33:41 +0000
@@ -34,7 +34,9 @@
     void clientAccessCheck2();
     void clientAccessCheckDone(const allow_t &answer);
     void clientRedirectStart();
+    void clientStoreUrlStart();
     void clientRedirectDone(const HelperReply &reply);
+    void clientStoreUrlDone(const HelperReply &reply);
     void checkNoCache();
     void checkNoCacheDone(const allow_t &answer);
 #if USE_ADAPTATION
@@ -55,6 +57,7 @@
     ClientHttpRequest *http;
     ACLChecklist *acl_checklist;        /* need ptr back so we can unreg if 
needed */
     int redirect_state;
+    int storeurl_state;
 
     /**
      * URL-rewrite/redirect helper may return BH for internal errors.
@@ -63,6 +66,7 @@
      * This tracks the number of previous failures for the current context.
      */
     uint8_t redirect_fail_count;
+    uint8_t storeurl_fail_count;
 
     bool host_header_verify_done;
     bool http_access_done;
@@ -71,6 +75,7 @@
     bool adaptation_acl_check_done;
 #endif
     bool redirect_done;
+    bool storeurl_rewrite_done;
     bool no_cache_done;
     bool interpreted_req_hdrs;
     bool tosToClientDone;

=== modified file 'src/HttpRequest.h'
--- src/HttpRequest.h   2012-11-30 22:57:54 +0000
+++ src/HttpRequest.h   2012-12-01 12:26:32 +0000
@@ -165,6 +165,8 @@
 
     char *canonical;
 
+    char *store_url;
+
     RequestFlags flags;
 
     HttpHdrRange *range;

=== modified file 'src/SquidConfig.h'
--- src/SquidConfig.h   2012-10-29 04:59:58 +0000
+++ src/SquidConfig.h   2012-12-01 11:10:48 +0000
@@ -203,6 +203,7 @@
 #endif
 
         wordlist *redirect;
+        wordlist *store_url;
 #if USE_UNLINKD
 
         char *unlinkd;
@@ -220,6 +221,7 @@
 #endif
 
     HelperChildConfig redirectChildren;
+    HelperChildConfig storeUrlChildren;
     time_t authenticateGCInterval;
     time_t authenticateTTL;
     time_t authenticateIpTTL;
@@ -318,6 +320,7 @@
         int nonhierarchical_direct;
         int strip_query_terms;
         int redirector_bypass;
+        int store_url_bypass;
         int ignore_unknown_nameservers;
         int client_pconns;
         int server_pconns;
@@ -381,6 +384,7 @@
         acl_access *brokenPosts;
 #endif
         acl_access *redirector;
+        acl_access *store_url;
         acl_access *reply;
         AclAddress *outgoing_address;
 #if USE_HTCP

=== modified file 'src/URL.h'
--- src/URL.h   2012-09-21 14:57:30 +0000
+++ src/URL.h   2012-12-01 12:43:43 +0000
@@ -83,6 +83,7 @@
 void urlInitialize(void);
 HttpRequest *urlParse(const HttpRequestMethod&, char *, HttpRequest *request = 
NULL);
 const char *urlCanonical(HttpRequest *);
+const char *urlStoreUrl(HttpRequest *);
 char *urlCanonicalClean(const HttpRequest *);
 const char *urlCanonicalFakeHttps(const HttpRequest * request);
 bool urlIsRelative(const char *);

=== modified file 'src/client_side_reply.cc'
--- src/client_side_reply.cc    2012-11-12 02:25:12 +0000
+++ src/client_side_reply.cc    2012-12-01 12:51:21 +0000
@@ -797,7 +797,10 @@
 
     for (HttpRequestMethod m(Http::METHOD_NONE); m != Http::METHOD_ENUM_END; 
++m) {
         if (m.respMaybeCacheable()) {
-            if (StoreEntry *entry = storeGetPublic(url, m)) {
+            /* TODO: fix things to allow store_url requests to be deleted.
+             *   is it the place?
+             */
+               if (StoreEntry *entry = storeGetPublic(url, m)) {
                 debugs(88, 5, "purging " << RequestMethodStr(m) << ' ' << url);
 #if USE_HTCP
                 neighborsHtcpClear(entry, url, req, m, HTCP_CLR_INVALIDATION);
@@ -2160,7 +2163,15 @@
     if (http->request == NULL)
         http->request = HTTPMSGLOCK(new HttpRequest(m, AnyP::PROTO_NONE, 
null_string));
 
-    StoreEntry *e = storeCreateEntry(http->uri, http->log_uri, reqFlags, m);
+    StoreEntry *e;
+    if (http->request->store_url){
+       assert(http->request->store_url);
+       e = storeCreateEntry(http->request->store_url, http->log_uri, reqFlags, 
m);
+    }
+    else{
+       e = storeCreateEntry(http->uri, http->log_uri, reqFlags, m);
+    }
+
 
     sc = storeClientListAdd(e, this);
 

=== modified file 'src/client_side_request.cc'
--- src/client_side_request.cc  2012-12-01 01:20:32 +0000
+++ src/client_side_request.cc  2012-12-01 12:27:12 +0000
@@ -132,6 +132,7 @@
 static int clientHierarchical(ClientHttpRequest * http);
 static void clientInterpretRequestHeaders(ClientHttpRequest * http);
 static HLPCB clientRedirectDoneWrapper;
+static HLPCB clientStoreUrlDoneWrapper;
 static void checkNoCacheDoneWrapper(allow_t, void *);
 SQUIDCEXTERN CSR clientGetMoreData;
 SQUIDCEXTERN CSS clientReplyStatus;
@@ -152,7 +153,7 @@
     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;
@@ -920,6 +921,33 @@
         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)
+        redirectStart(http, clientStoreUrlDoneWrapper, context);
+    else {
+        HelperReply nilReply;
+        context->clientStoreUrlDone(nilReply);
+    }
+}
+
+void
+ClientRequestContext::clientStoreUrlStart()
+{
+    debugs(33, 5, HERE << "'" << http->uri << "'");
+
+    if (Config.accessList.store_url) {
+        acl_checklist = clientAclChecklistCreate(Config.accessList.store_url, 
http);
+        acl_checklist->nonBlockingCheck(clientStoreUrlAccessCheckDone, this);
+    } else
+        redirectStart(http, clientStoreUrlDoneWrapper, this);
+}
+
 static int
 clientHierarchical(ClientHttpRequest * http)
 {
@@ -1253,7 +1281,7 @@
                 const char * result = statusNote->firstValue();
                 status = (http_status) atoi(result);
             }
-
+                       /* not needed for store url*/
             if (status == HTTP_MOVED_PERMANENTLY
                     || status == HTTP_MOVED_TEMPORARILY
                     || status == HTTP_SEE_OTHER
@@ -1313,6 +1341,69 @@
     http->doCallouts();
 }
 
+void
+clientStoreUrlDoneWrapper(void *data, const HelperReply &result)
+{
+    ClientRequestContext *calloutContext = (ClientRequestContext *)data;
+
+    if (!calloutContext->httpStateIsValid())
+        return;
+
+    calloutContext->clientStoreUrlDone(result);
+}
+
+void
+ClientRequestContext::clientStoreUrlDone(const HelperReply &reply)
+{
+    HttpRequest *old_request = http->request;
+    debugs(85, 5, "'" << http->uri << "' result=" << reply);
+    assert(storeurl_state == REDIRECT_PENDING);
+    storeurl_state = REDIRECT_DONE;
+
+    // copy the helper response Notes to the HTTP request for logging
+    // do it early to ensure that no matter what the outcome the notes are 
present.
+    // TODO put them straight into the transaction state record (ALE?) 
eventually
+    if (!old_request->helperNotes)
+        old_request->helperNotes = new Notes;
+    old_request->helperNotes->add(reply.notes);
+
+    switch(reply.result) {
+    case HelperReply::Unknown:
+    case HelperReply::TT:
+        // Handler in redirect.cc should have already mapped Unknown
+        // IF it contained valid entry for the old URL-rewrite helper protocol
+        debugs(85, DBG_IMPORTANT, "ERROR: store-URL helper returned invalid 
result code. Wrong helper? " << reply);
+        break;
+
+    case HelperReply::BrokenHelper:
+        debugs(85, DBG_IMPORTANT, "ERROR: store-URL helper: " << reply << ", 
attempt #" << (storeurl_fail_count+1) << " of 2");
+        if (storeurl_fail_count < 2) { // XXX: make this configurable ?
+            ++storeurl_fail_count;
+            // reset state flag to try redirector again from scratch.
+            storeurl_rewrite_done = false;
+        }
+        break;
+
+    case HelperReply::Error:
+        // no change to be done.
+        break;
+
+    case HelperReply::Okay: {
+        Note::Pointer urlNote = reply.notes.find("rewrite-url");
+
+        // prevent broken helpers causing too much damage. If old URL == new 
URL skip the re-write.
+        if (urlNote != NULL && strcmp(urlNote->firstValue(), http->uri)) {
+
+            // XXX: put the the storeURL value somewhere.
+            http->request->store_url= xstrdup(urlNote->firstValue());
+        }
+    }
+    break;
+    }
+
+    http->doCallouts();
+}
+
 /** Test cache allow/deny configuration
  *  Sets flags.cachable=1 if caching is not denied.
  */
@@ -1642,6 +1733,18 @@
             return;
         }
 
+        if (!calloutContext->storeurl_rewrite_done) {
+                       calloutContext->storeurl_rewrite_done = true;
+                       assert(calloutContext->storeurl_state == REDIRECT_NONE);
+
+                       if (Config.Program.store_url) {
+                               debugs(83, 3, HERE << "Doing 
calloutContext->clientStoreUrlStart()");
+                               calloutContext->storeurl_state = 
REDIRECT_PENDING;
+                               calloutContext->clientStoreUrlStart();
+                               return;
+                       }
+               }
+
         if (!calloutContext->interpreted_req_hdrs) {
             debugs(83, 3, HERE << "Doing clientInterpretRequestHeaders()");
             calloutContext->interpreted_req_hdrs = 1;

=== modified file 'src/client_side_request.h'
--- src/client_side_request.h   2012-11-04 12:27:49 +0000
+++ src/client_side_request.h   2012-12-01 10:09:27 +0000
@@ -97,6 +97,7 @@
 
     HttpRequest *request;              /* Parsed URL ... */
     char *uri;
+    char *store_url;
     char *log_uri;
 
     struct {

=== modified file 'src/redirect.cc'
--- src/redirect.cc     2012-11-30 11:08:47 +0000
+++ src/redirect.cc     2012-12-01 13:30:17 +0000
@@ -68,7 +68,9 @@
 static HLPCB redirectHandleReply;
 static void redirectStateFree(redirectStateData * r);
 static helper *redirectors = NULL;
+static helper *storeurls = NULL;
 static OBJH redirectStats;
+static OBJH storeUrlStats;
 static int n_bypassed = 0;
 CBDATA_TYPE(redirectStateData);
 
@@ -177,6 +179,21 @@
                           "because all redirectors were busy: %d\n", 
n_bypassed);
 }
 
+static void
+storeUrlStats(StoreEntry * sentry)
+{
+    if (storeurls == NULL) {
+        storeAppendPrintf(sentry, "No StoreUrls defined\n");
+        return;
+    }
+
+    helperStats(sentry, storeurls, "StoreUrls Statistics");
+
+    if (Config.onoff.store_url_bypass)
+        storeAppendPrintf(sentry, "\nNumber of requests bypassed "
+                          "because all storeUrls were busy: %d\n", n_bypassed);
+}
+
 /**** PUBLIC FUNCTIONS ****/
 
 void
@@ -298,6 +315,7 @@
 redirectRegisterWithCacheManager(void)
 {
     Mgr::RegisterAction("redirector", "URL Redirector Stats", redirectStats, 
0, 1);
+    Mgr::RegisterAction("storeurl", "Store URL rewriter Stats", storeUrlStats, 
0, 1);
 }
 
 void
@@ -307,19 +325,36 @@
 
     redirectRegisterWithCacheManager();
 
-    if (!Config.Program.redirect)
-        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 (!Config.Program.redirect && !Config.Program.store_url)
+             return;
+
+    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.store_url){
+
+               if (storeurls == NULL)
+                       storeurls = new helper("store_url");
+
+               storeurls->cmdline = Config.Program.store_url;
+
+               storeurls->childs.updateLimits(Config.storeUrlChildren);
+
+               storeurls->ipc_type = IPC_STREAM;
+
+               helperOpenServers(storeurls);
+      }
 
     if (!init) {
         init = 1;
@@ -330,14 +365,22 @@
 void
 redirectShutdown(void)
 {
-    if (!redirectors)
-        return;
-
-    helperShutdown(redirectors);
+    if (!storeurls && !redirectors)
+       return;
+
+       if (redirectors)
+               helperShutdown(redirectors);
+
+    if (storeurls)
+       helperShutdown(storeurls);
 
     if (!shutting_down)
         return;
 
     delete redirectors;
     redirectors = NULL;
+
+    delete storeurls;
+    storeurls = NULL;
+
 }

=== modified file 'src/store_key_md5.cc'
--- src/store_key_md5.cc        2012-09-01 14:38:36 +0000
+++ src/store_key_md5.cc        2012-12-01 12:45:37 +0000
@@ -140,10 +140,14 @@
     static cache_key digest[SQUID_MD5_DIGEST_LENGTH];
     unsigned char m = (unsigned char) method.id();
     const char *url = urlCanonical(request);
+    const char *store_url = urlStoreUrl(request);
     SquidMD5_CTX M;
     SquidMD5Init(&M);
     SquidMD5Update(&M, &m, sizeof(m));
-    SquidMD5Update(&M, (unsigned char *) url, strlen(url));
+    if (store_url)
+       SquidMD5Update(&M, (unsigned char *) store_url, strlen(store_url));
+    else
+       SquidMD5Update(&M, (unsigned char *) url, strlen(url));
 
     if (request->vary_headers)
         SquidMD5Update(&M, (unsigned char *) request->vary_headers, 
strlen(request->vary_headers));

=== modified file 'src/url.cc'
--- src/url.cc  2012-11-18 10:37:19 +0000
+++ src/url.cc  2012-12-01 12:44:11 +0000
@@ -528,6 +528,15 @@
     return (request->canonical = xstrdup(urlbuf));
 }
 
+const char *
+urlStoreUrl(HttpRequest * request)
+{
+   if (request->store_url)
+          return xstrdup(request->store_url);
+   else
+          return NULL;
+}
+
 /** \todo AYJ: Performance: This is an *almost* duplicate of urlCanonical. But 
elides the query-string.
  *        After copying it on in the first place! Would be less code to merge 
the two with a flag parameter.
  *        and never copy the query-string part in the first place

Reply via email to