Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix(retry): Avoid panicking if responses come early
`linkerd-http-retry` and `linkerd-retry` provide generic `tower` middleware to allow services to retry requests that fail. part of this middleware hinges on a `ReplayBody<B>` that will lazily buffer request bodies' data for use in subsequent attempts. if a request fails, the retry middleware will attempt to send another request, assuming the original body was able to completely fit into this buffer. `ReplayBody<B>` makes a subtle assummption, most succinctly stated in this excerpt of an internal method's documentation: ```rust // linkerd/http/retry/src/replay.rs impl<B: Body> ReplayBody<B> { /// This panics if another clone has currently acquired the state, based on /// the assumption that a retry body will not be polled until the previous /// request has been dropped. fn acquire_state<'a>( state: &'a mut Option<BodyState<B>>, shared: &Mutex<Option<BodyState<B>>>, ) -> &'a mut BodyState<B> { // ... } } ``` this assumption is slightly at odds with the request/response lifecycle permitted within the HTTP/2 specification. see RFC 9113 § 8.1, "_HTTP Message Framing_" (emphasis added): > .. > An HTTP request/response exchange fully consumes a single stream. A > request starts with the HEADERS frame that puts the stream into the > "open" state. **The request ends with a frame with the END_STREAM flag > set**, which causes the stream to become "half-closed (local)" for the > client and "half-closed (remote)" for the server. A response stream > starts with zero or more interim responses in HEADERS frames, followed > by a HEADERS frame containing a final status code. > > An HTTP response is complete after the server sends -- or the client > receives -- a frame with the END_STREAM flag set (including any > CONTINUATION frames needed to complete a field block). **A server can > send a complete response prior to the client sending an entire request > if the response does not depend on any portion of the request that has > not been sent and received.** <https://www.rfc-editor.org/rfc/rfc9113.html#section-8.1-11> because of this, a retry may panic when checking if the previous request body was capped, if a server delivers a response before the request is complete. this has been observed when retrying [wire-grpc](https://github.com/square/wire) requests, manifesting in a panic with this message: ```text thread 'main' panicked at 'if our `state` was `None`, the shared state must be `Some`', /__w/linkerd2-proxy/linkerd2-proxy/linkerd/http-retry/src/replay.rs:152:22 ``` this commit refactors `ReplayBody::is_capped()` so that it will no longer panic if there is an outstanding body still being polled. rather, it will return `Some(true)` or `Some(false)` if the previous body was capped, or `None` if it has not finished streaming. the related logic in the `linkerd-http-retry` library is updated to refrain from attempting a retry if a response is received before the request stream was completed. Signed-off-by: katelyn martin <[email protected]>
- Loading branch information