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 13b3432bae64dd801d55646d607618c2ab7e7602..5030b7bfe395b18b7414a1244904a298a575ec35 100644 (file)
--- a/src/http.cc
+++ b/src/http.cc
@@ -2615,38 +2615,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 cd5e26f9df3d8b26b2e3996c030a54d8a97bb0d1..989625ab18023e695b403187463f4abdbac48abe 100644 (file)
--- a/src/http.h
+++ b/src/http.h
@@ -120,8 +120,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