Skip to content

Commit

Permalink
docs(http/upgrade): document linkerd-http-upgrade (#3531)
Browse files Browse the repository at this point in the history
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 <kate@buoyant.io>

* 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 <kate@buoyant.io>

* nit(http/upgrade): update incorrect comment

this function has since been renamed `halves()`.

Signed-off-by: katelyn martin <kate@buoyant.io>

* 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 <kate@buoyant.io>

* 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 <kate@buoyant.io>

* nit(http/upgrade): fix typo in `Http11Upgrade` comment

Signed-off-by: katelyn martin <kate@buoyant.io>

* 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 <kate@buoyant.io>

---------

Signed-off-by: katelyn martin <kate@buoyant.io>
  • Loading branch information
cratelyn authored Jan 16, 2025
1 parent fed1e89 commit a4a55fa
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 12 deletions.
3 changes: 1 addition & 2 deletions linkerd/http/upgrade/src/glue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@ use tracing::debug;
#[pin_project(PinnedDrop)]
#[derive(Debug)]
pub struct UpgradeBody<B = BoxBody> {
/// In UpgradeBody::drop, if this was an HTTP upgrade, the body is taken
/// to be inserted into the Http11Upgrade half.
/// The inner [`Body`] being wrapped.
#[pin]
body: B,
upgrade: Option<(Http11Upgrade, hyper::upgrade::OnUpgrade)>,
Expand Down
48 changes: 47 additions & 1 deletion linkerd/http/upgrade/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,56 @@
//! Facilities for HTTP/1 upgrades.
//! Facilities for HTTP/1.1 upgrades.
//!
//! HTTP/1.1 specifies an `Upgrade` header field that may be used in tandem with the `Connection`
//! header field as a simple mechanism to transition from HTTP/1.1 to another protocol. This crate
//! provides [`tower`] middleware that enable upgrades to HTTP/2 for services running within a
//! [`tokio`] runtime.
//!
//! Use [`Service::new()`] to add upgrade support to a [`tower::Service`].
//!
//! [RFC 9110 § 7.6.1][rfc9110-connection] for more information about the `Connection` header
//! field, [RFC 9110 § 7.8][rfc9110-upgrade] for more information about HTTP/1.1's `Upgrade`
//! header field, and [RFC 9110 § 15.2.2][rfc9110-101] for more information about the
//! `101 (Switching Protocols)` response status code.
//!
//! Note that HTTP/2 does *NOT* provide support for the `Upgrade` header field, per
//! [RFC 9113 § 8.6][rfc9113]. HTTP/2 is a multiplexed protocol, and connection upgrades are
//! thus inapplicable.
//!
//! [rfc9110-connection]: https://www.rfc-editor.org/rfc/rfc9110#name-connection
//! [rfc9110-upgrade]: https://www.rfc-editor.org/rfc/rfc9110#field.upgrade
//! [rfc9110-101]: https://www.rfc-editor.org/rfc/rfc9110#name-101-switching-protocols
//! [rfc9113]: https://www.rfc-editor.org/rfc/rfc9113.html#name-the-upgrade-header-field
pub use self::upgrade::Service;

pub mod glue;
pub mod upgrade;

/// Removes connection headers from the given [`HeaderMap`][http::HeaderMap].
///
/// An HTTP proxy is required to do this, according to [RFC 9110 § 7.6.1 ¶ 5][rfc9110-761-5]:
///
/// > Intermediaries MUST parse a received Connection header field before a message is forwarded
/// > and, for each connection-option in this field, remove any header or trailer field(s) from the
/// > message with the same name as the connection-option, and then remove the Connection header
/// > field itself (or replace it with the intermediary's own control options for the forwarded
/// > message).
///
/// This function additionally removes some headers mentioned in
/// [RFC 9110 § 7.6.1 ¶ 7-8.5][rfc9110-761-7]
///
/// > Furthermore, intermediaries SHOULD remove or replace fields that are known to require removal
/// > before forwarding, whether or not they appear as a connection-option, after applying those
/// > fields' semantics. This includes but is not limited to:
/// >
/// > - `Proxy-Connection` (Appendix C.2.2 of [HTTP/1.1])
/// > - `Keep-Alive` (Section 19.7.1 of [RFC2068])
/// > - `TE` (Section 10.1.4)
/// > - `Transfer-Encoding` (Section 6.1 of [HTTP/1.1])
/// > - `Upgrade` (Section 7.8)
///
/// [rfc9110-761-5]: https://www.rfc-editor.org/rfc/rfc9110#section-7.6.1-5
/// [rfc9110-761-7]: https://www.rfc-editor.org/rfc/rfc9110#section-7.6.1-7
pub fn strip_connection_headers(headers: &mut http::HeaderMap) {
use http::header;
if let Some(val) = headers.remove(header::CONNECTION) {
Expand Down
10 changes: 6 additions & 4 deletions linkerd/http/upgrade/src/upgrade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,14 @@ use try_lock::TryLock;
/// inserted into the `Request::extensions()`. If the HTTP1 client service
/// also detects an upgrade, the two `OnUpgrade` futures will be joined
/// together with the glue in this type.
// Note: this relies on their only having been 2 Inner clones, so don't
// Note: this relies on there only having been 2 Inner clones, so don't
// implement `Clone` for this type.
pub struct Http11Upgrade {
half: Half,
inner: Arc<Inner>,
}

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

/// Checks requests to determine if they want to perform an HTTP upgrade.
/// Returns true if the given [Request<B>][http::Request] is attempting an HTTP/1.1 upgrade.
fn wants_upgrade<B>(req: &http::Request<B>) -> bool {
// HTTP upgrades were added in 1.1, not 1.0.
// Upgrades are specific to HTTP/1.1. They are not included in HTTP/1.0, nor are they supported
// in HTTP/2. If this request is associated with any protocol version besides HTTP/1.1, we can
// dismiss it immediately as not being applicable to an upgrade.
if req.version() != http::Version::HTTP_11 {
return false;
}
Expand Down
13 changes: 8 additions & 5 deletions linkerd/proxy/http/src/h1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,17 +138,20 @@ where

Box::pin(rsp_fut.err_into().map_ok(move |mut rsp| {
if is_http_connect {
// Add an extension to indicate that this a response to a CONNECT request.
debug_assert!(
upgrade.is_some(),
"Upgrade extension must be set on CONNECT requests"
);
rsp.extensions_mut().insert(HttpConnect);

// Strip headers that may not be transmitted to the server, per
// https://tools.ietf.org/html/rfc7231#section-4.3.6:
// Strip headers that may not be transmitted to the server, per RFC 9110:
//
// > A server MUST NOT send any `Transfer-Encoding` or `Content-Length` header
// > fields in a 2xx (Successful) response to `CONNECT`. A client MUST ignore any
// > `Content-Length` or `Transfer-Encoding` header fields received in a successful
// > response to `CONNECT`.
//
// A client MUST ignore any Content-Length or Transfer-Encoding
// header fields received in a successful response to CONNECT.
// see: https://www.rfc-editor.org/rfc/rfc9110#section-9.3.6-12
if rsp.status().is_success() {
rsp.headers_mut().remove(CONTENT_LENGTH);
rsp.headers_mut().remove(TRANSFER_ENCODING);
Expand Down

0 comments on commit a4a55fa

Please sign in to comment.