Skip to content

Commit ec441e5

Browse files
authored
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
1 parent 384aa21 commit ec441e5

File tree

1 file changed

+55
-10
lines changed

1 file changed

+55
-10
lines changed

linkerd/proxy/http/src/orig_proto.rs

Lines changed: 55 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -128,21 +128,66 @@ where
128128
/// Handles HTTP/2 client errors for HTTP/1.1 requests by wrapping the error type. This
129129
/// simplifies error handling elsewhere so that HTTP/2 errors can only be encountered when the
130130
/// original request was HTTP/2.
131-
fn downgrade_h2_error(error: hyper::Error) -> Error {
132-
use std::error::Error;
133-
134-
let mut cause = error.source();
135-
while let Some(e) = cause {
136-
if let Some(e) = e.downcast_ref::<h2::H2Error>() {
137-
if let Some(reason) = e.reason() {
138-
return DowngradedH2Error(reason).into();
139-
}
131+
fn downgrade_h2_error<E: std::error::Error + Send + Sync + 'static>(orig: E) -> Error {
132+
#[inline]
133+
fn reason(e: &(dyn std::error::Error + 'static)) -> Option<h2::Reason> {
134+
e.downcast_ref::<h2::H2Error>()?.reason()
135+
}
136+
137+
// If the provided error was an H2 error, wrap it as a downgraded error.
138+
if let Some(reason) = reason(&orig) {
139+
return DowngradedH2Error(reason).into();
140+
}
141+
142+
// Otherwise, check the source chain to see if its original error was an H2 error.
143+
let mut cause = orig.source();
144+
while let Some(error) = cause {
145+
if let Some(reason) = reason(error) {
146+
return DowngradedH2Error(reason).into();
140147
}
141148

142149
cause = error.source();
143150
}
144151

145-
error.into()
152+
// If the error was not an H2 error, return the original error (boxed).
153+
orig.into()
154+
}
155+
156+
#[cfg(test)]
157+
#[test]
158+
fn test_downgrade_h2_error() {
159+
assert!(
160+
downgrade_h2_error(h2::H2Error::from(h2::Reason::PROTOCOL_ERROR)).is::<DowngradedH2Error>(),
161+
"h2 errors must be downgraded"
162+
);
163+
164+
#[derive(Debug, Error)]
165+
#[error("wrapped h2 error: {0}")]
166+
struct WrapError(#[source] Error);
167+
assert!(
168+
downgrade_h2_error(WrapError(
169+
h2::H2Error::from(h2::Reason::PROTOCOL_ERROR).into()
170+
))
171+
.is::<DowngradedH2Error>(),
172+
"wrapped h2 errors must be downgraded"
173+
);
174+
175+
assert!(
176+
downgrade_h2_error(WrapError(
177+
WrapError(h2::H2Error::from(h2::Reason::PROTOCOL_ERROR).into()).into()
178+
))
179+
.is::<DowngradedH2Error>(),
180+
"double-wrapped h2 errors must be downgraded"
181+
);
182+
183+
assert!(
184+
!downgrade_h2_error(std::io::Error::new(
185+
std::io::ErrorKind::Other,
186+
"non h2 error"
187+
))
188+
.is::<DowngradedH2Error>(),
189+
"other h2 errors must not be downgraded"
190+
);
146191
}
147192

148193
// === impl UpgradeResponseBody ===

0 commit comments

Comments
 (0)