Maintenance: Drop HttpStateData::handleMoreRequestBodyAvailable (#2286)

The method is unreachable: It was supposed to be a virtual method. It is
declared as such in HttpStateData class, but it should have been
declared virtual in Client (i.e. the _parent_ class). Without that
virtual declaration in Client, the caller calls a Client method with the
same name instead. This code became unreachable in 2007 commit 5f8252d2.

The current method implementation is unusable: There are two explicit
XXXs, excessive (and unclassified) level-1 debugging, and an infinite
recursion added in the same 2007 commit 5f8252d2.

It is possible to refactor this method implementation to address known
concerns. We decided to remove this method instead because proper
refactoring is costly and is likely to result in this method removal.

The infinite recursion concern was raised by Joshua Rogers
https://joshua.hu/

diff --git a/src/http.cc b/src/http.cc
index a43b2b504f4b14331402e93b36c2cf69c558a687..42ee7ec24a013f8847b7e9f3a846515bf863c8d8 100644 (file)
--- a/src/http.cc
+++ b/src/http.cc
@@ -2621,38 +2621,6 @@ HttpStateData::doneSendingRequestBody()
     sendComplete();
 }
 
-// more origin request body data is available
-void
-HttpStateData::handleMoreRequestBodyAvailable()
-{
-    if (eof || !Comm::IsConnOpen(serverConnection)) {
-        // XXX: we should check this condition in other callbacks then!
-        // TODO: Check whether this can actually happen: We should unsubscribe
-        // as a body consumer when the above condition(s) are detected.
-        debugs(11, DBG_IMPORTANT, "Transaction aborted while reading HTTP body");
-        return;
-    }
-
-    assert(requestBodySource != nullptr);
-
-    if (requestBodySource->buf().hasContent()) {
-        // XXX: why does not this trigger a debug message on every request?
-
-        if (flags.headers_parsed && !flags.abuse_detected) {
-            flags.abuse_detected = true;
-            debugs(11, DBG_IMPORTANT, "http handleMoreRequestBodyAvailable: Likely proxy abuse detected '" << request->client_addr << "' -> '" << entry->url() << "'" );
-
-            if (virginReply()->sline.status() == Http::scInvalidHeader) {
-                closeServer();
-                mustStop("HttpStateData::handleMoreRequestBodyAvailable");
-                return;
-            }
-        }
-    }
-
-    HttpStateData::handleMoreRequestBodyAvailable();
-}
-
 // premature end of the request body
 void
 HttpStateData::handleRequestBodyProducerAborted()

diff --git a/src/http.h b/src/http.h
index e5f8fdb3842d656bb8ed0d3484ab2aa1dcd5aaec..c5f14a9a6670877ee78f9350851413cf5c59fe16 100644 (file)
--- a/src/http.h
+++ b/src/http.h
@@ -121,8 +121,6 @@ private:
     std::optional<size_t> canBufferMoreReplyBytes() const;
     size_t maybeMakeSpaceAvailable(size_t maxReadSize);
 
-    // consuming request body
-    virtual void handleMoreRequestBodyAvailable();
     void handleRequestBodyProducerAborted() override;
 
     void writeReplyBody();

diff --git a/src/http/StateFlags.h b/src/http/StateFlags.h
index 52b86f5edacfa22bdc16d648c8cf0e4869a00695..8f4dd28ccc2155f0af1c077b310a4052b7108e54 100644 (file)
--- a/src/http/StateFlags.h
+++ b/src/http/StateFlags.h
@@ -56,7 +56,6 @@ public:
     bool toOriginPeer() const { return toOrigin && peering && !tunneling; }
 
     bool keepalive_broken = false;
-    bool abuse_detected = false;
     bool request_sent = false;
     bool chunked = false;           ///< reading a chunked response; TODO: rename
     bool chunked_request = false;   ///< writing a chunked request