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

Support 'Precondition Required' #102

Open
SebastianEdwards opened this issue May 7, 2013 · 12 comments
Open

Support 'Precondition Required' #102

SebastianEdwards opened this issue May 7, 2013 · 12 comments
Labels

Comments

@SebastianEdwards
Copy link

Is there any interest in supporting the Precondition Required response in the decision tree? The status code definition can be found at http://tools.ietf.org/html/rfc6585#section-3.

The change would add the following:

  • A new callback on resource called precondition_required? which would default to false. This would commonly be used to check for the presence of an If-Match header.
  • A new decision node between G7 and G8 which will return code 428 if the precondition_required? callback returns true.

Very happy to prepare a pull request if there is interest in this.

@SebastianEdwards
Copy link
Author

Hmm, on second thought, the proposed semantics aren't quite right. It really requires a pair of callbacks precondition_required? AND precondition_present?

precondition_present? could helpfully default to checking the If-Match header as this is the common use case.

@SebastianEdwards
Copy link
Author

First run at this: SebastianEdwards@4faa45b

Happy to take a stab at amending the HTTP flowchart if we have consensus on decision node placement.

Thoughts?

@ghost
Copy link

ghost commented May 7, 2013

Summoning @justinsheehy as he also might have some thoughts regarding the decision tree

@justinsheehy
Copy link

My initial thought would be to put it in the leftmost column with other simple tests, as it seems that it can be determined statically without depending on anything else in the decision flow.

A quick reading of the RFC section indicates to me that only the "required" callback is needed. The presence of any conditional request header ("If-*") would satisfy the requirement.

I would also support adding this to the original Webmachine if a PR happens to appear there.

@seancribbs
Copy link
Member

I concur with @justinsheehy, it seems independent of other decisions, so the earlier the better.

@SebastianEdwards
Copy link
Author

Thanks for the input @justinsheehy.

It had occurred to me to use the leftmost column. However, since conditional headers check against existing resources, I thought it made more sense to insert it after it's ascertained that the resource exists.

The reasoning for a precondition_present? callback is that in practice most preconditions are not strong enough to guarantee a conflict will not occur. For example: an If-None-Match header, while useful for caching, is completely useless for conflict avoidance. Users usually wish to define a subset of acceptable preconditions depending on their use case.

@samwgoldman
Copy link
Contributor

@SebastianEdwards Somewhat off-topic, but can you explain why If-None-Match is useless for conflict avoidance? It seems to me that if two clients are trying to update a resource, checking ETags would ensure nobody writes what they haven't read.

@SebastianEdwards
Copy link
Author

@samwgoldman an If-None-Match header would allow the write to occur if the client provides any ETag which DOES NOT match the current one. On the other hand, an If-Match header checks that the ETag DOES match the current one, ensuring the client has at least read the most current version of the resource.

@seancribbs
Copy link
Member

For what it's worth, If-None-Match is generally only used with "*" as the value, preventing blind overwites of resources that have already been created. It's a similar problem to doing a PUT with If-Modified-Since. Who clobbers things intentionally?

@seancribbs
Copy link
Member

Sorry, the other usage of If-None-Match is obviously conditional GET, where you want a 304 Not Modified if the ETag(s) still match.

@SebastianEdwards
Copy link
Author

Ah, I haven't seen the use of If-None-Match: "*" in the wild before. Makes sense though.

@seancribbs
Copy link
Member

@SebastianEdwards Have you made any headway on code/tests for this feature?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants