Skip to content

Conversation

@kazuho
Copy link
Owner

@kazuho kazuho commented Nov 21, 2019

Forbid servers using the Priority request header to generate different responses, as suggested by @royfielding.

properties of an HTTP request it receives, the server is expected to control the
cacheability or the applicability of the cached response, by using header fields
that control the caching behavior (e.g., Cache-Control, Vary).
A server MUST NOT generate different response based on the value of the Priority
Copy link
Contributor

Choose a reason for hiding this comment

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

It can generate a different Priority header in response, correct?

Copy link
Owner Author

Choose a reason for hiding this comment

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

In 9fffd45, I've clarified that we are talking about "HTTP response" but not a response header field. Does that look better?

Copy link
Collaborator

@LPardue LPardue left a comment

Choose a reason for hiding this comment

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

lgtm

@rmarx
Copy link
Contributor

rmarx commented Nov 21, 2019

I'm afraid we're losing a bit of guidance on the caching footguns with this? This might be clear to you all, but not to me and probably not to some other eventual implementers either. Since this was a main point from Ian's, I wonder if we need to add some more guidance or details in here (not necessarily mentioning Vary etc.)

@ianswett
Copy link
Contributor

I agree with @rmarx that we shouldn't remove the other paragraph and only replace it with this.

@kazuho
Copy link
Owner Author

kazuho commented Nov 21, 2019

@rmarx @ianswett I can see the concern, therefore added a sentence explaining the negative consequence that people might face if they do not obey to the requirement. PTAL.

properties of an HTTP request it receives, the server is expected to control the
cacheability or the applicability of the cached response, by using header fields
that control the caching behavior (e.g., Cache-Control, Vary).
The sole intention of prioritization is to optimize how responses are being
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure it's the sole intention of prioritization. Don't we say you can use this to inform request behavior as well?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we are trying to say that the intention of prioritization is to optimise HOW responses are being provided, not WHAT is provided. I started to walk this path in the introduction but didn't want to get too much into HTTP-core leet speak. IIUC what we are trying to say, I think we can riff off the following text fragments from the intro:


   To provide meaningful representation of a document at the earliest
   moment, it is important for an HTTP server to prioritize the HTTP
   responses, or the chunks of those HTTP responses, that it sends.
   This document defines the Priority HTTP header field that can be used
   by both client and server to specify the precedence of HTTP responses

Copy link
Contributor

@ianswett ianswett left a comment

Choose a reason for hiding this comment

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

Some nits, but I think this LG now.

Copy link
Contributor

@ianswett ianswett left a comment

Choose a reason for hiding this comment

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

One suggestion

The sole intention of prioritization is to optimize how responses are being
provided, not to change what is provided as the response. Therefore, a server MUST
NOT generate a different HTTP response based on the value of the Priority request
header field. Sending a cacheable response that depends on the Priority request
Copy link
Contributor

@ianswett ianswett Nov 27, 2019

Choose a reason for hiding this comment

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

I'm finding this last sentence somewhat vague, how about:
"Sending a cacheable response including a Priority response header field that depends upon the Priority request header field is likely to lead to mis-prioritization when a cache reuses that response."

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.

5 participants