Recognize internal requests created by adaptation/redirection (#1504)

Before this fix, Squid set flags.internal for virgin requests but not
for adapted/redirected requests, leaving post-adaptation request
processing code in an inconsistent state, forwarding the
internalCheck()-compliant adapted/redirected requests as regular
requests, triggering forwarding loops and complicating code refactoring.

diff --git a/src/client_side.cc b/src/client_side.cc
index bd9cf6a7d5f6680c52f80c3074531ec635210fa0..b5fd933b369dc5ebcde663a44023b9f8a355513e 100644 (file)
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -1619,23 +1619,7 @@ clientProcessRequest(ConnStateData *conn, const Http1::RequestParserPointer &hp,
     }
 #endif
 
-    if (internalCheck(request->url.path())) {
-        if (internalHostnameIs(request->url.host()) && request->url.port() == getMyPort()) {
-            debugs(33, 2, "internal URL found: " << request->url.getScheme() << "://" << request->url.authority(true));
-            request->flags.internal = true;
-        } else if (Config.onoff.global_internal_static && internalStaticCheck(request->url.path())) {
-            debugs(33, 2, "internal URL found: " << request->url.getScheme() << "://" << request->url.authority(true) << " (global_internal_static on)");
-            request->url.setScheme(AnyP::PROTO_HTTP, "http");
-            request->url.host(internalHostname());
-            request->url.port(getMyPort());
-            request->flags.internal = true;
-            http->setLogUriToRequestUri();
-        } else
-            debugs(33, 2, "internal URL found: " << request->url.getScheme() << "://" << request->url.authority(true) << " (not this proxy)");
-
-        if (ForSomeCacheManager(request->url.path()))
-            request->flags.disableCacheUse("cache manager URL");
-    }
+    http->checkForInternalAccess();
 
     if (!isFtp) {
         // XXX: for non-HTTP messages instantiate a different Http::Message child type

diff --git a/src/client_side_request.cc b/src/client_side_request.cc
index 36d0aca11db29c05463f3b13ee9a15c48e820089..91a289bfc5cef282c90c9bc3c8a9bcb36402eed2 100644 (file)
--- a/src/client_side_request.cc
+++ b/src/client_side_request.cc
@@ -41,6 +41,7 @@
 #include "HttpHdrCc.h"
 #include "HttpReply.h"
 #include "HttpRequest.h"
+#include "internal.h"
 #include "ip/NfMarkConfig.h"
 #include "ip/QosConfig.h"
 #include "ipcache.h"
@@ -1222,9 +1223,6 @@ ClientRequestContext::clientRedirectDone(const Helper::Reply &reply)
                     new_request->url = tmpUrl;
                     debugs(61, 2, "URL-rewriter diverts URL from " << old_request->effectiveRequestUri() << " to " << new_request->effectiveRequestUri());
 
-                    // update the new request to flag the re-writing was done on it
-                    new_request->flags.redirected = true;
-
                     // unlink bodypipe from the old request. Not needed there any longer.
                     if (old_request->body_pipe != nullptr) {
                         old_request->body_pipe = nullptr;
@@ -1232,7 +1230,7 @@ ClientRequestContext::clientRedirectDone(const Helper::Reply &reply)
                                " from request " << old_request << " to " << new_request);
                     }
 
-                    http->resetRequest(new_request);
+                    http->resetRequestXXX(new_request, true);
                     old_request = nullptr;
                 } else {
                     debugs(85, DBG_CRITICAL, "ERROR: URL-rewrite produces invalid request: " <<
@@ -1626,12 +1624,48 @@ ClientHttpRequest::initRequest(HttpRequest *aRequest)
 
 void
 ClientHttpRequest::resetRequest(HttpRequest *newRequest)
+{
+    const auto uriChanged = request->effectiveRequestUri() != newRequest->effectiveRequestUri();
+    resetRequestXXX(newRequest, uriChanged);
+}
+
+void
+ClientHttpRequest::resetRequestXXX(HttpRequest *newRequest, const bool uriChanged)
 {
     assert(request != newRequest);
     clearRequest();
     assignRequest(newRequest);
     xfree(uri);
     uri = SBufToCstring(request->effectiveRequestUri());
+
+    if (uriChanged) {
+        request->flags.redirected = true;
+        checkForInternalAccess();
+    }
+}
+
+void
+ClientHttpRequest::checkForInternalAccess()
+{
+    if (!internalCheck(request->url.path()))
+        return;
+
+    if (internalHostnameIs(request->url.host()) && request->url.port() == getMyPort()) {
+        debugs(33, 3, "internal URL found: " << request->url.getScheme() << "://" << request->url.authority(true));
+        request->flags.internal = true;
+    } else if (Config.onoff.global_internal_static && internalStaticCheck(request->url.path())) {
+        debugs(33, 3, "internal URL found: " << request->url.getScheme() << "://" << request->url.authority(true) << " (global_internal_static on)");
+        request->url.setScheme(AnyP::PROTO_HTTP, "http");
+        request->url.host(internalHostname());
+        request->url.port(getMyPort());
+        request->flags.internal = true;
+        setLogUriToRequestUri();
+    } else {
+        debugs(33, 3, "internal URL found: " << request->url.getScheme() << "://" << request->url.authority(true) << " (not this proxy)");
+    }
+
+    if (ForSomeCacheManager(request->url.path()))
+        request->flags.disableCacheUse("cache manager URL");
 }
 
 void
@@ -1963,9 +1997,6 @@ ClientHttpRequest::handleAdaptedHeader(Http::Message *msg)
     assert(msg);
 
     if (HttpRequest *new_req = dynamic_cast<HttpRequest*>(msg)) {
-        // update the new message to flag whether URL re-writing was done on it
-        if (request->effectiveRequestUri() != new_req->effectiveRequestUri())
-            new_req->flags.redirected = true;
         resetRequest(new_req);
         assert(request->method.id());
     } else if (HttpReply *new_rep = dynamic_cast<HttpReply*>(msg)) {

diff --git a/src/client_side_request.h b/src/client_side_request.h
index 73ba5c8cbbe0a886cee7885cb9dd5b4f612bf862..f7c4eec632990f9699dda95441ca8d6e8e93333c 100644 (file)
--- a/src/client_side_request.h
+++ b/src/client_side_request.h
@@ -82,6 +82,14 @@ public:
     /// the request. To set the virgin request, use initRequest().
     void resetRequest(HttpRequest *);
 
+    // XXX: unify the uriChanged condition calculation with resetRequest() callers, removing this method
+    /// resetRequest() variation for callers with custom URI change detection logic
+    /// \param uriChanged whether the new request URI differs from the current request URI
+    void resetRequestXXX(HttpRequest *, bool uriChanged);
+
+    /// Checks whether the current request is internal and adjusts it accordingly.
+    void checkForInternalAccess();
+
     /// update the code in the transaction processing tags
     void updateLoggingTags(const LogTags_ot code) { al->cache.code.update(code); }