From ec441e5819eeb187dc6da2907fbdabb72c753c87 Mon Sep 17 00:00:00 2001 From: Oliver Gould Date: Tue, 19 Oct 2021 15:29:53 -0700 Subject: [PATCH] Fix an infinite loop when downgrading HTTP/2 errors (#1318) 578d979 introduced a bug: when the proxy handles errors on HTTP/2-upgraded connections, it can get stuck in an infinite loop when searching the error chain for an HTTP/2 error. This change fixes this inifite loop and adds a test that exercises this behavior to ensure that `downgrade_h2_error` behaves as expected. Fixes linkerd/linkerd2#7103 --- linkerd/proxy/http/src/orig_proto.rs | 65 +++++++++++++++++++++++----- 1 file changed, 55 insertions(+), 10 deletions(-) diff --git a/linkerd/proxy/http/src/orig_proto.rs b/linkerd/proxy/http/src/orig_proto.rs index da21218eff..57e1ec1db5 100644 --- a/linkerd/proxy/http/src/orig_proto.rs +++ b/linkerd/proxy/http/src/orig_proto.rs @@ -128,21 +128,66 @@ where /// Handles HTTP/2 client errors for HTTP/1.1 requests by wrapping the error type. This /// simplifies error handling elsewhere so that HTTP/2 errors can only be encountered when the /// original request was HTTP/2. -fn downgrade_h2_error(error: hyper::Error) -> Error { - use std::error::Error; - - let mut cause = error.source(); - while let Some(e) = cause { - if let Some(e) = e.downcast_ref::() { - if let Some(reason) = e.reason() { - return DowngradedH2Error(reason).into(); - } +fn downgrade_h2_error(orig: E) -> Error { + #[inline] + fn reason(e: &(dyn std::error::Error + 'static)) -> Option { + e.downcast_ref::()?.reason() + } + + // If the provided error was an H2 error, wrap it as a downgraded error. + if let Some(reason) = reason(&orig) { + return DowngradedH2Error(reason).into(); + } + + // Otherwise, check the source chain to see if its original error was an H2 error. + let mut cause = orig.source(); + while let Some(error) = cause { + if let Some(reason) = reason(error) { + return DowngradedH2Error(reason).into(); } cause = error.source(); } - error.into() + // If the error was not an H2 error, return the original error (boxed). + orig.into() +} + +#[cfg(test)] +#[test] +fn test_downgrade_h2_error() { + assert!( + downgrade_h2_error(h2::H2Error::from(h2::Reason::PROTOCOL_ERROR)).is::(), + "h2 errors must be downgraded" + ); + + #[derive(Debug, Error)] + #[error("wrapped h2 error: {0}")] + struct WrapError(#[source] Error); + assert!( + downgrade_h2_error(WrapError( + h2::H2Error::from(h2::Reason::PROTOCOL_ERROR).into() + )) + .is::(), + "wrapped h2 errors must be downgraded" + ); + + assert!( + downgrade_h2_error(WrapError( + WrapError(h2::H2Error::from(h2::Reason::PROTOCOL_ERROR).into()).into() + )) + .is::(), + "double-wrapped h2 errors must be downgraded" + ); + + assert!( + !downgrade_h2_error(std::io::Error::new( + std::io::ErrorKind::Other, + "non h2 error" + )) + .is::(), + "other h2 errors must not be downgraded" + ); } // === impl UpgradeResponseBody ===