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

Duplicative specification of precondition handling #665

Closed
mnot opened this issue Jan 8, 2021 · 8 comments · Fixed by #670
Closed

Duplicative specification of precondition handling #665

mnot opened this issue Jan 8, 2021 · 8 comments · Fixed by #670

Comments

@mnot
Copy link
Member

mnot commented Jan 8, 2021

Caching 4.3.2 re-specifies precondition processing from Semantics 13.

E.g.,

A request containing an If-None-Match header field (Section 13.1.2 of [Semantics]) indicates that the client wants to validate one or more of its own stored responses in comparison to whichever stored response is selected by the cache. If the field value is "*", or if the field value is a list of entity-tags and at least one of them matches the entity-tag of the selected stored response, a cache recipient should generate a 304 (Not Modified) response (using the metadata of the selected stored response) instead of sending that stored response.

If an If-None-Match header field is not present, a request containing an If-Modified-Since header field (Section 13.1.3 of [Semantics]) indicates that the client wants to validate one or more of its own stored responses by modification date. A cache recipient should generate a 304 (Not Modified) response (using the metadata of the selected stored response) if one of the following cases is true:

[...]

Do we want to try to fold these into Semantics so that it isn't duplicated? That would require some small adjustments to Semantics AFAICT (e.g., to make it clear what the selected representation is in each case, etc.).

@royfielding
Copy link
Member

royfielding commented Jan 8, 2021

Why not just end each first sentence with ", as defined in Section X.x of Semantics." and remove the rest?

@royfielding
Copy link
Member

Er, never mind ... having looked at the full section, I think this should stay as is because it is refining the requirements in terms of the cache's stored responses.

@royfielding
Copy link
Member

I think we can close this. It isn't duplicated: Semantics already refers to this section to define cache handling.

@mnot
Copy link
Member Author

mnot commented Jan 11, 2021

Re-reading the current text, I think I'm finding it most disturbing that semantics 13.1.3 If-Modified-Since doesn't say how to evaluate the condition, whereas caching 4.3.2 does (but just for caches). I think it needs to be removed from caching and made generic.

This is necessary because semantics 13.2 refers to evaluation in their algorithms. INM and if-match define how to evaluate, but IMS and IUS don't.

@royfielding
Copy link
Member

I think I'm finding it most disturbing that semantics 13.1.3 If-Modified-Since doesn't say how to evaluate the condition

It defines it near the end of the section because of the etag discussion. Anyway, we can improve that.

@mnot
Copy link
Member Author

mnot commented Jan 11, 2021

I proposed a fix for that, but now that #669 is proposed as well, some of the text in caching is clearly duplicative. Will adjust and push another commit or two; please review.

@royfielding
Copy link
Member

There is a larger problem here -- the section on evaluation was split into two sections (evaluation and precedence) but the conditionals only reference the first. This can be fixed by moving precedence back within evaluation with subsections for the other content in evaluation.

@royfielding
Copy link
Member

Okay, I have updated the PR to make evaluation one section (with two subsections), fixed a few typos, made the requirements self-descriptive, and updated If-Range to be consistent as well.

@mnot mnot closed this as completed in #670 Jan 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

2 participants