delete HTTPSink::safeToUpgrade

Summary:
* continuining efforts to clean up the codebase and deprecating http/1.1 -> http/2 plaintext upgrades
* removed the only usage of ::safeToUpgrade in D76834276, we can now remove all instances of the overriden virtual fn

Reviewed By: kwaugh

Differential Revision: D77604276

fbshipit-source-id: efea5c00a5fb1301555f36b2f191fb4fd4096c8c

Author: hanidamlaj

third-party/proxygen/src/proxygen/lib/http/sink/HTTPConnectSink.h

@@ -159,10 +159,6 @@ class HTTPConnectSink
   void setIdleTimeout(std::chrono::milliseconds timeout) override;
 
   // Capabilities
-  bool safeToUpgrade(HTTPMessage*) const override {
-    return false;
-  }
-
   [[nodiscard]] bool supportsPush() const override {
     return false;
   }

third-party/proxygen/src/proxygen/lib/http/sink/HTTPSink.h

@@ -91,7 +91,6 @@ class HTTPSink {
   virtual void timeoutExpired() = 0;
   virtual void setIdleTimeout(std::chrono::milliseconds timeout) = 0;
   // Capabilities
-  virtual bool safeToUpgrade(HTTPMessage* req) const = 0;
   [[nodiscard]] virtual bool supportsPush() const = 0;
   // Logging
   virtual void describe(std::ostream& os) = 0;

third-party/proxygen/src/proxygen/lib/http/sink/HTTPTransactionSink.cpp

@@ -47,29 +47,4 @@ quic::QuicSocket* HTTPTransactionSink::getQUICTransport() const {
   return nullptr;
 }
 
-bool HTTPTransactionSink::safeToUpgrade(HTTPMessage* req) const {
-  // There's a race condition if we haven't seen the end of a POST
-  // terminated by content-length.  If that's the case, don't request an upgrade
-  // on this connection.  We can live with a little HTTP/1.1 for now.
-
-  // Because HTTP/2 and HTTP/3 reported "chunked" messages for requests that
-  // will see DATA frames, the only unsafe type of request is HTTP/1.1 with
-  // Content-Length terminated body.  We could force this to use chunked
-  // encoding to make it safe, but for now we won't.
-
-  // One subtle case here is a pure HTTP/1.1 GET request with no body.  It is
-  // safeToUpgrade per this function, but it relies on the fact that the
-  // resumeIngress call after sendingHeaders will trigger onClientEOM
-  // immediately, causing the sendEOM to happen before the 101 response arrives
-
-  // And because we force chunk this request, it can't be a MUST not have body
-  if (req && RFC2616::isRequestBodyAllowed(req->getMethod()) ==
-                 RFC2616::BodyAllowed::NOT_ALLOWED) {
-    return false;
-  }
-  return (httpTransaction_ && httpTransaction_->isIngressEOMSeen()) ||
-         (req && (!req->getHeaders().exists(HTTP_HEADER_CONTENT_LENGTH) ||
-                  req->getIsChunked()));
-}
-
 } // namespace proxygen

third-party/proxygen/src/proxygen/lib/http/sink/HTTPTransactionSink.h

@@ -156,7 +156,6 @@ class HTTPTransactionSink : public HTTPSink {
     httpTransaction_->setIdleTimeout(timeout);
   }
   // Capabilities
-  bool safeToUpgrade(HTTPMessage* req) const override;
   [[nodiscard]] bool supportsPush() const override {
     return true;
   }

third-party/proxygen/src/proxygen/lib/http/sink/HTTPTunnelSink.h

@@ -159,10 +159,6 @@ class HTTPTunnelSink
   void setIdleTimeout(std::chrono::milliseconds timeout) override;
 
   // Capabilities
-  bool safeToUpgrade(HTTPMessage*) const override {
-    return false;
-  }
-
   [[nodiscard]] bool supportsPush() const override {
     return false;
   }