Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs(http/upgrade): document linkerd-http-upgrade #3531

Merged
merged 7 commits into from
Jan 16, 2025

Conversation

cratelyn
Copy link
Collaborator

@cratelyn cratelyn commented Jan 15, 2025

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 comment affixed to the linkerd_http_upgrade::upgrade::Http11Upgrade request/response extension:

// 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. 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.

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]>
this function has since been renamed `halves()`.

Signed-off-by: katelyn martin <[email protected]>
this adds a comment additionally clarifying that HTTP/2 does not support
upgrades.

Signed-off-by: katelyn martin <[email protected]>
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]>
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]>
@cratelyn cratelyn marked this pull request as ready for review January 15, 2025 21:42
@cratelyn cratelyn requested a review from a team as a code owner January 15, 2025 21:42
@cratelyn cratelyn merged commit a4a55fa into main Jan 16, 2025
15 checks passed
@cratelyn cratelyn deleted the kate/http-upgrade.2-docs-and-comments branch January 16, 2025 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants