-
Notifications
You must be signed in to change notification settings - Fork 16
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
Rename Content-{Length,Type,Range} → Patch-{Length,Type,Range} #97
Comments
The |
Yes! I like this change. It also has the (perhaps unintended) consequence of making parsing more robust: If there is a mistake made in the number of patches, for example ( |
Oh, since Edit: Nevermind, we still need to go into the actually length of the Patch, because \r\n inside a patch would mess up the parser. |
Thinking about the names a bit more:
So if we'd like to keep the semantics clear, I think we should have:
Changing your example:
|
Yeah I think I like that idea. A lot of the examples in the spec at the moment have The thing I like the most about this idea is that its clear about the separation between parts of the response. Right now as I mentioned in #96 there are 3 levels of HTTP headers in subscriptions:
I feel like the current spec is a little ambiguous about which headers go where, and when. With this change |
This change seems to have strong support (as it is mentioned as a dependency in other issues as well), and is probably ready for a PR to be drafted. However, @canadaduane notes that we might want to keep Option 1: keep Content-Range
Option 2: rename to Patch-Range
That's for responses. What about Requests?I am also noticing that we have only been examining responses, here, and we should consider if this change will be symmetric for requests. Requests have always used the Option 1: Range
Option 2: Patch-Range
Consider also that a Range-PUT request without the
But if we express that with the Braid
It might be desirable to always use If I remove the chair hat, I am personally leaning slightly in favor of preserving the existing names for Range, and only transitioning:
instead of:
But it would help to hear from other people on this. |
Perhaps we should petition HTTPbis to change the definition of Partial PUT, and require a
|
Oh... the most recent HTTP RFC 9110 has updated the definition of "content". I'm not sure my past interpretations of "content" are correct anymore. This deserves re-evaluation before making a choice. |
FYI, with See example response there:
The size of the content on the server is |
Yes, mitar is right. It turns out that "Content" actually refers to the content of the HTTP message, not the resource on the server. (It's a coincidence that "the content of a message" often happens to be "the thing on the server.") Also, it turns out that the word "Content" in "Content-Length" is not to be taken seriously according to rfc9110:
The definition for
|
I spent quite a few hours thinking through all the different naming options. It turns out to be fairly complicated, because these headers can appear at the top level, or within a block of patches.
I tried to strike a balance here, and came up with the following PR: #120 This gives us:
This probably is not the end of the discussion, but maybe it's enough for version 03 of the draft. Thoughts? |
I just discovered that the HTTP WG is noticing problems with the ambiguity of Content-Length, and has been considering designing a new header to fix them. Here's a draft proposal: https://datatracker.ietf.org/doc/html/draft-nottingham-bikeshed-length This indicates might be support in solving the Content-Length issue deeply within HTTP. Perhaps we want to borrow that deeper solution, or contribute to it, to solve our problem in message framing as well. Also, HTTP/2 has its own message framing system. Headers and are transmitted in a "headers frame", and content in a "data frame", that says how long it is in binary, and thus doesn't need a content-length or patch-length field to frame it. |
The headers we specify on patches currently have the
Content-
prefix:We borrowed these from HTTP's existing header names. But upon further reflection, the semantics of "content" might not be totally appropriate.
In HTTP, the term "content" refers to the contents of the resource. (See rfc7231 "HTTP/1.1 Semantics and Content".) However, we are using these terms to refer to a patch. Thus, they are not about the same "content" anymore.
I think it might be clearer if we renamed them to
Patch-Length
,Patch-Range
, andPatch-Type
, or perhaps even justLength
,Range
, andType
. Here is what it would look like with the former:And here's an example of the latter:
The text was updated successfully, but these errors were encountered: