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

Add server-content-type-payload #524

Merged
merged 1 commit into from
May 18, 2023
Merged

Conversation

csarven
Copy link
Member

@csarven csarven commented May 11, 2023

This change adds a new feature as per https://www.w3.org/2021/Process-20211102/#class-4 :

Add requirement for server to include Content-Type in messages with payload in Solid Protocol.

Server including Content-Type in the response of a message including a payload is not guaranteed as per RFC 7231 since the inclusion is a SHOULD. It follows that the response may trigger the client to do MIME type sniffing. This requirement will ensure that server will have awareness of the media type of the message with a MUST, and clients to follow the provided Content-Type.

Moreover, the Solid Protocol requires that clients MUST include the Content-Type in PUT, POST, PATCH requests. It does not however necessarily follow that the server will preserve the media type (or its alternatives). Lastly, while a storage may potentially include resources that were not initially provided by a client - in which case the interaction is technically out of scope of Solid Protocol - this server requirement will ensure that HTTP messages will indeed include a Content-Type irrespective to however resources became available from a storage.


Preview | Diff

Copy link
Member

@matthieubosquet matthieubosquet left a comment

Choose a reason for hiding this comment

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

This is a great addition :)

@damooo
Copy link

damooo commented May 11, 2023

Yes, it would allow to not depend on many insecure ways to mime-guess.

@elf-pavlik
Copy link
Member

I understand that this only applies to GET. I would like to consider extending the requirement to also require it for HEAD. I work on implementing support for Non-RDFSources and would like to be able to find the Content-Type without doing full GET. The use case would be for example in the list of file attachments to show relevant icons based on the MIME type before the user decides to download any of the attachments.

@csarven
Copy link
Member Author

csarven commented May 11, 2023

With the proposed requirement, HEAD will be covered:

The HEAD method is identical to GET except that the server MUST NOT
send a message body in the response (i.e., the response terminates at
the end of the header section).

@elf-pavlik
Copy link
Member

What do you think about following RFC 9110 terminology?

https://datatracker.ietf.org/doc/html/rfc9110#name-changes-from-rfc-7231

The terms "payload" and "payload body" have been replaced with "content", to better align with its usage elsewhere (e.g., in field names) and to avoid confusion with frame payloads in HTTP/2 and HTTP/3. (Section 6.4)

https://datatracker.ietf.org/doc/html/rfc9110#name-head

The HEAD method is identical to GET except that the server MUST NOT send content in the response. HEAD is used to obtain metadata about the selected representation without transferring its representation data, often for the sake of testing hypertext links or finding recent modifications.

https://datatracker.ietf.org/doc/html/rfc9110#section-6.4.1

Responses to the HEAD request method (Section 9.3.2) never include content; the associated response header fields indicate only what their values would have been if the request method had been GET (Section 9.3.1).

Copy link
Member

@kjetilk kjetilk left a comment

Choose a reason for hiding this comment

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

Yes, we like the metadata to be assistive, so I think we should have this as a MUST.

@@ -645,6 +645,8 @@ <h3 property="schema:name">HTTP Server</h3>

<p><span about="" id="server-authentication" rel="spec:requirement" resource="#server-authentication"><span property="spec:statement"><span rel="spec:requirementSubject" resource="#Server">Servers</span> <span rel="spec:requirementLevel" resource="spec:MUST">MUST</span> conform to <cite>HTTP/1.1 Authentication</cite> [<cite><a class="bibref" href="#bib-rfc7235">RFC7235</a></cite>].</span></span> <span about="" id="server-unauthenticated" rel="spec:requirement" resource="#server-unauthenticated"><span property="spec:statement">When a client does not provide valid credentials when requesting a resource that requires it (see <a href="#webid">WebID</a>), <span rel="spec:requirementSubject" resource="#Server">servers</span> <span rel="spec:requirementLevel" resource="spec:MUST">MUST</span> send a response with a <code>401</code> status code (unless <code>404</code> is preferred for security reasons).</span></span></p>

<p><span about="" id="server-content-type-payload" rel="spec:requirement" resource="#server-content-type-payload"><span property="spec:statement"><span rel="spec:requirementSubject" resource="#Server">Server</span> <span rel="spec:requirementLevel" resource="spec:MUST">MUST</span> generate a <code>Content-Type</code> header field in a message that contains a payload body.</span></span></p>
Copy link
Member

@elf-pavlik elf-pavlik May 11, 2023

Choose a reason for hiding this comment

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

Suggested change
<p><span about="" id="server-content-type-payload" rel="spec:requirement" resource="#server-content-type-payload"><span property="spec:statement"><span rel="spec:requirementSubject" resource="#Server">Server</span> <span rel="spec:requirementLevel" resource="spec:MUST">MUST</span> generate a <code>Content-Type</code> header field in a message that contains a payload body.</span></span></p>
<p><span about="" id="server-content-type-payload" rel="spec:requirement" resource="#server-content-type-payload"><span property="spec:statement"><span rel="spec:requirementSubject" resource="#Server">Server</span> <span rel="spec:requirementLevel" resource="spec:MUST">MUST</span> generate a <code>Content-Type</code> header field in a message with content.</span></span></p>

based on my comments in #524 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not opposed to this but let me ask this. Can we do this in a separate PR?

I've assigned milestone 0.11.0 to #471 where we should look into changing references to 911x and corresponding terminology throughout the spec. For the time being, at least with this PR, the terminology aligns with current reference to 7231.

(Also, is it "with a content" or just "with content"?)

Copy link
Member

Choose a reason for hiding this comment

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

Ok to handle it as a separate PR, yeah probably just "with content" will update my suggestion for the future record

@csarven csarven requested a review from TallTed May 17, 2023 14:59
@csarven
Copy link
Member Author

csarven commented May 18, 2023

@csarven csarven merged commit 61573ed into main May 18, 2023
@csarven csarven deleted the feature/server-content-type-payload branch May 18, 2023 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

6 participants