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 PUT Location header test #14

Merged
merged 2 commits into from
Feb 8, 2024
Merged

add PUT Location header test #14

merged 2 commits into from
Feb 8, 2024

Conversation

notroj
Copy link
Owner

@notroj notroj commented Feb 8, 2024

No description provided.

* src/props.c (do_invalid_pfind, propfind_returns_wellformed, do_patch):
  Set content-type (fixes #5).
@notroj notroj merged commit 9f0247b into master Feb 8, 2024
13 checks passed
@notroj notroj deleted the put-location branch February 8, 2024 13:29
@gstrauss
Copy link
Contributor

A PUT which creates a resource is required to have the server return HTTP status 201 Created, and the new test verifies this.

Is the server also required to provide Location in the response headers? This new test issues a warning if Location is not provided. If the resource is located at the same URI to which the PUT was sent, is Location required?

RFC9110

15.3.2. 201 Created

The 201 (Created) status code indicates that the request has been fulfilled and has resulted in one or more new resources being created. The primary resource created by the request is identified by either a Location header field in the response or, if no Location header field is received, by the target URI.

I do not think Location is required for PUT when 201 Created is returned and the target URI for the resource is the same URI as was sent in the PUT.

Should I submit a PR to remove the warning if Location is not included in response to the new put_location test?

@notroj
Copy link
Owner Author

notroj commented Feb 16, 2024

Oh good catch, thanks for reviewing @gstrauss (I messed up the commit message for that commit too... annoying)

Yes please a PR to remove the warning would be great!

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.

2 participants