Skip to content

Commit a4a55fa

Browse files
authored
docs(http/upgrade): document linkerd-http-upgrade (#3531)
some aspects of `linkerd-http-upgrade` are incompatible with the 1.0 interface of the `http` crate (_see: hyperium/http#395, linkerd/linkerd2#8733_). this new bound requiring that extensions must now be cloneable motivated me to read through this library's internals to gain a lucid understanding of how it works, in order to understand how to gracefully address [this](https://github.com/linkerd/linkerd2-proxy/blob/main/linkerd/http/upgrade/src/upgrade.rs#L25-L26) comment affixed to the `linkerd_http_upgrade::upgrade::Http11Upgrade` request/response extension: ```rust // Note: this relies on their only having been 2 Inner clones, so don't // implement `Clone` for this type. [sic] pub struct Http11Upgrade { half: Half, inner: Arc<Inner>, } ``` broadly, this library deals with some moderately arcane corners of the HTTP protocol family. the `Upgrade` header is not supported in HTTP/2, and was not yet introduced in HTTP/1.0, so it is a feature specific to HTTP/1.1. moreover, some behavior provided by this library falls into parts of the spec(s) that we `MUST` uphold, and isn't currently well documented. this branch includes a sequence of commits adding documentation and additional comments linking to, and quoting, the relevant parts of [RFC 9110](https://www.rfc-editor.org/rfc/rfc9110). some links to RFC 7231, which was obsoleted by RFC 9110 since the original time of writing, are additionally updated. some comments also did not accurately describe internal logic, or included typos, and are also updated. --- * docs(http/upgrade): add crate-level docs, rfc link Signed-off-by: katelyn martin <[email protected]> * docs(http/upgrade): update link to obsolete rfc rfc 9110 obsoletes the following rfc's: 2818, 7230, 7231, 7232, 7233, 7235, 7538, 7615, and 7694. this updates a comment related to connection upgrade logic, linking to the current rfc, 9110. this information now lives in section 9.3.6, paragraph 12. Signed-off-by: katelyn martin <[email protected]> * nit(http/upgrade): update incorrect comment this function has since been renamed `halves()`. Signed-off-by: katelyn martin <[email protected]> * docs(http/upgrade): add comments to `wants_upgrade` this adds a comment additionally clarifying that HTTP/2 does not support upgrades. Signed-off-by: katelyn martin <[email protected]> * docs(http/upgrade): document `strip_connection_headers()` this function performs some important behavior that we MUST implement, as a proxy/intermediary. to help elucidate the mandated behavior expected of us by the HTTP/1 specification, add documentation comments noting the related passages from rfc 9110 § 7.6.1. Signed-off-by: katelyn martin <[email protected]> * nit(http/upgrade): fix typo in `Http11Upgrade` comment Signed-off-by: katelyn martin <[email protected]> * docs(http/upgrade): update incorrect comment this comment is not true. this commit updates it, reflecting the current state of the upgrade body's `Drop` logic. Signed-off-by: katelyn martin <[email protected]> --------- Signed-off-by: katelyn martin <[email protected]>
1 parent fed1e89 commit a4a55fa

File tree

4 files changed

+62
-12
lines changed

4 files changed

+62
-12
lines changed

linkerd/http/upgrade/src/glue.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,7 @@ use tracing::debug;
1818
#[pin_project(PinnedDrop)]
1919
#[derive(Debug)]
2020
pub struct UpgradeBody<B = BoxBody> {
21-
/// In UpgradeBody::drop, if this was an HTTP upgrade, the body is taken
22-
/// to be inserted into the Http11Upgrade half.
21+
/// The inner [`Body`] being wrapped.
2322
#[pin]
2423
body: B,
2524
upgrade: Option<(Http11Upgrade, hyper::upgrade::OnUpgrade)>,

linkerd/http/upgrade/src/lib.rs

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,56 @@
1-
//! Facilities for HTTP/1 upgrades.
1+
//! Facilities for HTTP/1.1 upgrades.
2+
//!
3+
//! HTTP/1.1 specifies an `Upgrade` header field that may be used in tandem with the `Connection`
4+
//! header field as a simple mechanism to transition from HTTP/1.1 to another protocol. This crate
5+
//! provides [`tower`] middleware that enable upgrades to HTTP/2 for services running within a
6+
//! [`tokio`] runtime.
7+
//!
8+
//! Use [`Service::new()`] to add upgrade support to a [`tower::Service`].
9+
//!
10+
//! [RFC 9110 § 7.6.1][rfc9110-connection] for more information about the `Connection` header
11+
//! field, [RFC 9110 § 7.8][rfc9110-upgrade] for more information about HTTP/1.1's `Upgrade`
12+
//! header field, and [RFC 9110 § 15.2.2][rfc9110-101] for more information about the
13+
//! `101 (Switching Protocols)` response status code.
14+
//!
15+
//! Note that HTTP/2 does *NOT* provide support for the `Upgrade` header field, per
16+
//! [RFC 9113 § 8.6][rfc9113]. HTTP/2 is a multiplexed protocol, and connection upgrades are
17+
//! thus inapplicable.
18+
//!
19+
//! [rfc9110-connection]: https://www.rfc-editor.org/rfc/rfc9110#name-connection
20+
//! [rfc9110-upgrade]: https://www.rfc-editor.org/rfc/rfc9110#field.upgrade
21+
//! [rfc9110-101]: https://www.rfc-editor.org/rfc/rfc9110#name-101-switching-protocols
22+
//! [rfc9113]: https://www.rfc-editor.org/rfc/rfc9113.html#name-the-upgrade-header-field
223
324
pub use self::upgrade::Service;
425

526
pub mod glue;
627
pub mod upgrade;
728

29+
/// Removes connection headers from the given [`HeaderMap`][http::HeaderMap].
30+
///
31+
/// An HTTP proxy is required to do this, according to [RFC 9110 § 7.6.1 ¶ 5][rfc9110-761-5]:
32+
///
33+
/// > Intermediaries MUST parse a received Connection header field before a message is forwarded
34+
/// > and, for each connection-option in this field, remove any header or trailer field(s) from the
35+
/// > message with the same name as the connection-option, and then remove the Connection header
36+
/// > field itself (or replace it with the intermediary's own control options for the forwarded
37+
/// > message).
38+
///
39+
/// This function additionally removes some headers mentioned in
40+
/// [RFC 9110 § 7.6.1 ¶ 7-8.5][rfc9110-761-7]
41+
///
42+
/// > Furthermore, intermediaries SHOULD remove or replace fields that are known to require removal
43+
/// > before forwarding, whether or not they appear as a connection-option, after applying those
44+
/// > fields' semantics. This includes but is not limited to:
45+
/// >
46+
/// > - `Proxy-Connection` (Appendix C.2.2 of [HTTP/1.1])
47+
/// > - `Keep-Alive` (Section 19.7.1 of [RFC2068])
48+
/// > - `TE` (Section 10.1.4)
49+
/// > - `Transfer-Encoding` (Section 6.1 of [HTTP/1.1])
50+
/// > - `Upgrade` (Section 7.8)
51+
///
52+
/// [rfc9110-761-5]: https://www.rfc-editor.org/rfc/rfc9110#section-7.6.1-5
53+
/// [rfc9110-761-7]: https://www.rfc-editor.org/rfc/rfc9110#section-7.6.1-7
854
pub fn strip_connection_headers(headers: &mut http::HeaderMap) {
955
use http::header;
1056
if let Some(val) = headers.remove(header::CONNECTION) {

linkerd/http/upgrade/src/upgrade.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,14 @@ use try_lock::TryLock;
2222
/// inserted into the `Request::extensions()`. If the HTTP1 client service
2323
/// also detects an upgrade, the two `OnUpgrade` futures will be joined
2424
/// together with the glue in this type.
25-
// Note: this relies on their only having been 2 Inner clones, so don't
25+
// Note: this relies on there only having been 2 Inner clones, so don't
2626
// implement `Clone` for this type.
2727
pub struct Http11Upgrade {
2828
half: Half,
2929
inner: Arc<Inner>,
3030
}
3131

32-
/// A named "tuple" returned by `Http11Upgade::new()` of the two halves of
32+
/// A named "tuple" returned by [`Http11Upgade::halves()`] of the two halves of
3333
/// an upgrade.
3434
#[derive(Debug)]
3535
struct Http11UpgradeHalves {
@@ -257,9 +257,11 @@ fn is_origin_form(uri: &http::uri::Uri) -> bool {
257257
uri.scheme().is_none() && uri.path_and_query().is_none()
258258
}
259259

260-
/// Checks requests to determine if they want to perform an HTTP upgrade.
260+
/// Returns true if the given [Request<B>][http::Request] is attempting an HTTP/1.1 upgrade.
261261
fn wants_upgrade<B>(req: &http::Request<B>) -> bool {
262-
// HTTP upgrades were added in 1.1, not 1.0.
262+
// Upgrades are specific to HTTP/1.1. They are not included in HTTP/1.0, nor are they supported
263+
// in HTTP/2. If this request is associated with any protocol version besides HTTP/1.1, we can
264+
// dismiss it immediately as not being applicable to an upgrade.
263265
if req.version() != http::Version::HTTP_11 {
264266
return false;
265267
}

linkerd/proxy/http/src/h1.rs

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -138,17 +138,20 @@ where
138138

139139
Box::pin(rsp_fut.err_into().map_ok(move |mut rsp| {
140140
if is_http_connect {
141+
// Add an extension to indicate that this a response to a CONNECT request.
141142
debug_assert!(
142143
upgrade.is_some(),
143144
"Upgrade extension must be set on CONNECT requests"
144145
);
145146
rsp.extensions_mut().insert(HttpConnect);
146-
147-
// Strip headers that may not be transmitted to the server, per
148-
// https://tools.ietf.org/html/rfc7231#section-4.3.6:
147+
// Strip headers that may not be transmitted to the server, per RFC 9110:
148+
//
149+
// > A server MUST NOT send any `Transfer-Encoding` or `Content-Length` header
150+
// > fields in a 2xx (Successful) response to `CONNECT`. A client MUST ignore any
151+
// > `Content-Length` or `Transfer-Encoding` header fields received in a successful
152+
// > response to `CONNECT`.
149153
//
150-
// A client MUST ignore any Content-Length or Transfer-Encoding
151-
// header fields received in a successful response to CONNECT.
154+
// see: https://www.rfc-editor.org/rfc/rfc9110#section-9.3.6-12
152155
if rsp.status().is_success() {
153156
rsp.headers_mut().remove(CONTENT_LENGTH);
154157
rsp.headers_mut().remove(TRANSFER_ENCODING);

0 commit comments

Comments
 (0)