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

chore(http): Delete "Digest" and "Want-Digest", improve digest pages #36879

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

bsmth
Copy link
Member

@bsmth bsmth commented Nov 20, 2024

Description

This PR removes the Digest and Want-Digest pages which are deprecated. They are also confusing, given the pages are mixed in with newer alternatives. I'm proposing we redirect to content-related digest headers.

Deletions:

  • Digest header, redirected to Content-Digest
  • Want-Digest header, redirected to Want-Content-Digest

Changes:

  • Language in Repr-Digest and Content-Digest pages for clarity
  • spec_urls front matter should be spec-urls

Additions:

  • Some more examples in Content-Digest based on specs

Motivation

The remaining pages were touched recently, but I think there's room for improvement in the language used so that it's easier to understand when/why to use Repr-Digest instead of Content-Digest.

Related issues and pull requests

@bsmth bsmth requested review from a team as code owners November 20, 2024 11:29
@bsmth bsmth requested review from hamishwillee and removed request for a team November 20, 2024 11:29
@github-actions github-actions bot added Content:HTTP HTTP docs Content:Glossary Glossary entries Content:Meta Content in the meta docs size/m [PR only] 51-500 LoC changed labels Nov 20, 2024
Copy link
Contributor

Preview URLs (7 pages)
External URLs (14)

URL: /en-US/docs/Glossary/Representation_header
Title: Representation header


URL: /en-US/docs/Web/HTTP/Headers
Title: HTTP headers


URL: /en-US/docs/Web/HTTP/Headers/Content-Digest
Title: Content-Digest

The HTTP **`Content-Digest`** header provides a {{Glossary("digest")}} of the message content in an HTTP message.
As such, `Content-Digest` is dependent on among other things {{HTTPHeader("Content-Encoding")}} and {{HTTPHeader("Content-Range")}}, but not dependent on, for example, HTTP/1.1's {{HTTPHeader("Transfer-Encoding")}}.
`Content-Digest` may coincide with {{HTTPHeader("Repr-Digest")}} if a representation was sent in a single message.
The HTTP **`Content-Digest`** {{Glossary("request header", "request")}} and {{Glossary("response header")}} provides a {{Glossary("digest")}} calculated using a hashing algorithm applied to the message content.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not going to block merging, but given this is request and response, perhaps we should have an example or explanation of the case where the browser sends a digest?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto for Repr

```plain
Content-Digest: unixcksum=916142062
Content-Digest: md5=:+thA//8pGVGk2VYuJkFNvA==:, unixsum=26869
### Client requests a SHA-256 Content-Digest
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Client? User-Agent, Browser?


### Identical Content-Digest and Repr-Digest values

A client requests a resource without a `Want-Content-Digest` field:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For client, I prefer browser if it is always a browser, and user-agent if it could reasonably mean a script. Obviously if you agree, change everywhere.

```

In this case, the server is configured to send unsolicited digest headers in responses.
Both the `Repr-Digest` and `Content-Digest` fields have matching values:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps

Suggested change
Both the `Repr-Digest` and `Content-Digest` fields have matching values:
The `Repr-Digest` and `Content-Digest` fields have matching values because they are using the same algorithm, and in this case the whole resource is sent in just one message.

@@ -8,12 +8,14 @@ spec-urls: https://datatracker.ietf.org/doc/html/rfc9530
{{HTTPSidebar}}

The HTTP **`Repr-Digest`** {{Glossary("Request header", "request")}} and {{Glossary("Response header", "response header")}} provides a {{Glossary("digest")}} of the selected representation of the target resource.
A `Repr-Digest` is most suited to validate the integrity of partial or multipart messages against the full selected representation.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, I wanted to change it like this:

Suggested change
A `Repr-Digest` is most suited to validate the integrity of partial or multipart messages against the full selected representation.
A `Repr-Digest` can be used to validate the integrity of partial or multipart messages against the full selected representation.

Now I'd like you to tell me what that means ? :-). How can you validate a partial message using this information. The answer is probably you can't. My take is that this is only really useful once you have the whole message, and then it tells you that all the parts were OK.

Copy link
Collaborator

@hamishwillee hamishwillee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good. Was tempted to merge, but perhaps worth leaving open for you to see if you want to address any of the comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:Glossary Glossary entries Content:HTTP HTTP docs Content:Meta Content in the meta docs size/m [PR only] 51-500 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants