-
-
Notifications
You must be signed in to change notification settings - Fork 295
Make sure 204 status responses have no body in accordance with HTTP spec #2094
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
Make sure 204 status responses have no body in accordance with HTTP spec #2094
Conversation
Thanks @C-Loftus. As a first step, can you provide steps to reproduce the issue that this PR aims to address? |
Thanks for your reply @tomkralidis . To reproduce, make a custom provider that can potentially return no data, run pygeoapi with starlette, create a condition in the provider that throws a I quickly made up a repo with a minimal example for you here: https://github.com/cgs-earth/pygeoapi-starlette-issue-reproduction |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the test case @C-Loftus. Please see change requests. Thanks!
# NOTE: that gzip currently doesn't work in starlette | ||
# https://github.com/geopython/pygeoapi/issues/1591 | ||
content = apply_gzip(headers, content) | ||
# 204 responses must have an empty body, but gzip | ||
# encoding would add gzip metadata, thus we skip | ||
if status != HTTPStatus.NO_CONTENT: | ||
content = apply_gzip(headers, content) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have removed a comment linking to an issue that was resolved and then added a clarifying comment on why we need to skip applying gzip when it is a 204 response. Without this, the apply_gzip
function zips empty content, and empty content still will include a gzip header and thus raise an error
Overview
This PR makes it so http responses with a
204
status have no body in accordance with the http spec.Background
Currently if you raise a
ProviderNoDataError
it will return a response with status204
.However, currently it will include data in the response. This deviates from the spec which states that the response must not include any content
To fix this I make it return early and not add any data into the body.
Issue this solves
Currently in the starlette backend, if you raise a 204 but have data in the body, it will raise an error and fail.
Additional information
Dependency policy (RFC2)
Updates to public demo
Contributions and licensing
(as per https://github.com/geopython/pygeoapi/blob/master/CONTRIBUTING.md#contributions-and-licensing)