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

Handling of If-Match: * is not RFC compliant #483

Open
Yonezawa-T2 opened this issue Sep 12, 2019 · 7 comments
Open

Handling of If-Match: * is not RFC compliant #483

Yonezawa-T2 opened this issue Sep 12, 2019 · 7 comments
Assignees

Comments

@Yonezawa-T2
Copy link

Yonezawa-T2 commented Sep 12, 2019

The documentation says if we omit If-Match header for WebDAV PUT method, it defaults to *.
https://personium.io/docs/en/apiref/current/312_Register_and_Update_WebDAV.html

This is not compliant to RFC 7232:

If the field-value is "*", the condition is false if the origin server does not have a current representation for the target resource.

https://tools.ietf.org/html/rfc7232#section-3.1

Omitting If-Match header and giving If-Match header with the value * must behave differently.

I have not checked actual behavior. Is this just a error of documentation, or bug of actual implementation?

@tochi-y
Copy link
Member

tochi-y commented Sep 13, 2019

Thank you for raising this issue.
When personium project started in 2012, RFC7232 did not exist. So early codes don't comply with it. We think it should be compliant to RFC as you commented. Now I'll check the RFC and the current implementation.

@tochi-y tochi-y self-assigned this Sep 13, 2019
@Yonezawa-T2
Copy link
Author

It is also specified in RFC 2616, issued at June 1999 and obsoleted by RFC 7232 later.

if "*" is given and no current entity exists, the server MUST NOT perform the requested method, and MUST return a 412 (Precondition Failed) response.

https://tools.ietf.org/html/rfc2616#section-14.24

RFC 2068, which is predecessor of RFC 2616 and released at January 1997, also states the same behavior.

https://tools.ietf.org/html/rfc2068#section-14.25

So, Personium had been violating the RFC from beginning.

@shimono
Copy link
Member

shimono commented Sep 13, 2019

It is a shame we had been violating RFC2616 from the beginning. Thank you for pointing it out and give us a chance to stop this situation.

Could you help us a bit more to make things clearer? My understanding is the following:


Conditions

When the following 3 conditions are met:

  1. PUT method
  2. Resource does not exist
  3. If-Match Header value is "*"

Current Personium:

  • accepts the request and create the resource.

To make it RFC compliant, it should:

  • return 412 error.

Is my understanding correct?

@Yonezawa-T2
Copy link
Author

Is my understanding correct?

Yes. It must return 412.

@Yonezawa-T2
Copy link
Author

DELETE also accepts If-Match header:
https://personium.io/docs/en/apiref/current/314_Delete_WebDAV.html

@Yonezawa-T2
Copy link
Author

In my opinion, the priority of this bug isn't high. Practical applications won't pass If-Match: *, that is, “update/delete the file if it exists but I don't care its version”.
I thought it is an error of documentation when I file this issue.

@shimono shimono added this to the 1.7.20 milestone Oct 20, 2019
@tochi-y tochi-y closed this as completed in 51c1cba Nov 1, 2019
tochi-y added a commit that referenced this issue Nov 1, 2019
Fix #483 #484 If-Match when resource does not exists and If-None-Match when it exists.
tochi-y added a commit that referenced this issue Nov 5, 2019
shimono added a commit that referenced this issue Nov 5, 2019
Revert "Fix #483 #484 If-Match when resource does not exists and If-None-Match when it exists."
@tochi-y tochi-y modified the milestones: 1.7.20, 1.7.21 Nov 6, 2019
@tochi-y
Copy link
Member

tochi-y commented Nov 6, 2019

Moved to 1.7.21 for the issue personium/personium-engine#119.

@tochi-y tochi-y reopened this Nov 6, 2019
@tochi-y tochi-y modified the milestones: 1.7.21, v1.7.22 Nov 13, 2019
@shimono shimono modified the milestones: 1.7.22, next-candidate Mar 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants