http: log a warning when a CONNECT response has the wrong version (#1827)

* http: log a warning when a CONNECT response has the wrong version

The issue linkerd/linkerd2#6035 describes a situation where Linkerd
rejects HTTP/1.1 CONNECT requests because the remote (non-Linkerd)
forward proxy returns a successful response that erroneously has the
wrong HTTP version (HTTP/1.0). Since the CONNECT method does not exist
in the HTTP/1.0 protocol, Linkerd does not treat this response as a
CONNECT.

It turns out that in the accursed Real World, some HTTP forward proxies
actually do return the wrong protocol version when successfully handling
CONNECT requests. We don't want to remove the check that the protocol
version is correct, because it could lead to us incorrectly treating
responses to CONNECTs as establishing a tunnel when they don't actually
do that. However, in order to make it easier for future users who
encounter issues where other proxies return wrong HTTP versions for
CONNECT responses, it might be nice to log a warning, so that users can
determine *why* their CONNECT requests aren't working.

This branch changes the proxy to log a warning if a successful response
to a CONNECT request had the wrong HTTP version. We won't log the
warning if the response is not a success or if the request was not a
CONNECT, so it should only cover this specific case.

* format version with debug

Signed-off-by: Eliza Weisman <[email protected]>

Author: hawkw

linkerd/proxy/http/src/h1.rs

@@ -241,8 +241,20 @@ pub(crate) fn wants_upgrade<B>(req: &http::Request<B>) -> bool {
 
 /// Checks responses to determine if they are successful HTTP upgrades.
 pub(crate) fn is_upgrade<B>(res: &http::Response<B>) -> bool {
+    #[inline]
+    fn is_connect_success<B>(res: &http::Response<B>) -> bool {
+        res.extensions().get::<HttpConnect>().is_some() && res.status().is_success()
+    }
+
     // Upgrades were introduced in HTTP/1.1
     if res.version() != http::Version::HTTP_11 {
+        if is_connect_success(res) {
+            tracing::warn!(
+                "A successful response to a CONNECT request had an incorrect HTTP version \
+                (expected HTTP/1.1, got {:?})",
+                res.version()
+            );
+        }
         return false;
     }
 
@@ -252,7 +264,7 @@ pub(crate) fn is_upgrade<B>(res: &http::Response<B>) -> bool {
     }
 
     // CONNECT requests are complete if status code is 2xx.
-    if res.extensions().get::<HttpConnect>().is_some() && res.status().is_success() {
+    if is_connect_success(res) {
         return true;
     }