Skip to content

Commit

Permalink
Do not strip entity-headers with HTTP status 304 or 412 (#2824)
Browse files Browse the repository at this point in the history
* Add test for issue #2823

* Do not strip entity-headers.

* Remove unused import

* Remove remove_entity_headers()

* Add workflow_dispatch release

* RTD build fix

* RTD build fix (another)

* RTD build fix (yet another)

* Update Crowdin configuration file

* Fix Docker publish (#2887)

* Fix Docker publish

* Remove workflow dispatch

The actions uses data from the release object itself, so workflow dispatch doesn't work anyway

* More fixes

* Remove test for removed functionality.

---------

Co-authored-by: L. Kärkkäinen <Tronic@users.noreply.github.com>
Co-authored-by: Adam Hopkins <adam@amhopkins.com>
Co-authored-by: Néstor Pérez <25409753+prryplatypus@users.noreply.github.com>
  • Loading branch information
4 people committed Jan 9, 2024
1 parent 57d44f2 commit cf71fc7
Show file tree
Hide file tree
Showing 7 changed files with 18 additions and 49 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/publish-release.yml
Expand Up @@ -69,7 +69,7 @@ jobs:
tags="${tag_year}.${tag_month}"
if [[ "${tag_month}" == "12" ]]; then
tags+=",LTS"
tags+=",lts"
echo "::notice::Tag ${tag} is LTS version"
else
echo "::notice::Tag ${tag} is not LTS version"
Expand Down Expand Up @@ -122,7 +122,7 @@ jobs:
name: Publish Docker / Python ${{ matrix.python-version }}
needs: [generate_info, publish_package]
runs-on: ubuntu-latest
if: ${{ needs.generate_info.IS_TEST == 'false' }}
if: ${{ needs.generate_info.outputs.is-test == 'false' }}
strategy:
fail-fast: true
matrix:
Expand Down
3 changes: 3 additions & 0 deletions crowdin.yml
@@ -0,0 +1,3 @@
files:
- source: /guide/content/en/**/*.md
translation: /guide/content/%two_letters_code%/**/%original_file_name%
7 changes: 5 additions & 2 deletions readthedocs.yml
@@ -1,9 +1,12 @@
version: 2
python:
version: 3.8
install:
- method: pip
path: .
extra_requirements:
- docs
system_packages: true

build:
os: "ubuntu-22.04"
tools:
python: "3.8"
19 changes: 0 additions & 19 deletions sanic/helpers.py
Expand Up @@ -122,25 +122,6 @@ def is_hop_by_hop_header(header):
return header.lower() in _HOP_BY_HOP_HEADERS


def remove_entity_headers(headers, allowed=("content-location", "expires")):
"""
Removes all the entity headers present in the headers given.
According to RFC 2616 Section 10.3.5,
Content-Location and Expires are allowed as for the
"strong cache validator".
https://tools.ietf.org/html/rfc2616#section-10.3.5
returns the headers without the entity headers
"""
allowed = set([h.lower() for h in allowed])
headers = {
header: value
for header, value in headers.items()
if not is_entity_header(header) or header.lower() in allowed
}
return headers


def import_string(module_name, package=None):
"""
import a module or class by string path.
Expand Down
4 changes: 0 additions & 4 deletions sanic/response/types.py
Expand Up @@ -24,7 +24,6 @@
Default,
_default,
has_message_body,
remove_entity_headers,
)
from sanic.http import Http

Expand Down Expand Up @@ -104,9 +103,6 @@ def processed_headers(self) -> Iterator[Tuple[bytes, bytes]]:
Returns:
Iterator[Tuple[bytes, bytes]]: A list of header tuples encoded in bytes for sending
""" # noqa: E501
# TODO: Make a blacklist set of header names and then filter with that
if self.status in (304, 412): # Not Modified, Precondition Failed
self.headers = remove_entity_headers(self.headers)
if has_message_body(self.status):
self.headers.setdefault("content-type", self.content_type)
# Encode headers into bytes
Expand Down
22 changes: 0 additions & 22 deletions tests/test_helpers.py
Expand Up @@ -41,28 +41,6 @@ def test_is_hop_by_hop_header():
assert helpers.is_hop_by_hop_header(header) is expected


def test_remove_entity_headers():
tests = (
({}, {}),
({"Allow": "GET, POST, HEAD"}, {}),
(
{
"Content-Type": "application/json",
"Expires": "Wed, 21 Oct 2015 07:28:00 GMT",
"Foo": "Bar",
},
{"Expires": "Wed, 21 Oct 2015 07:28:00 GMT", "Foo": "Bar"},
),
(
{"Allow": "GET, POST, HEAD", "Content-Location": "/test"},
{"Content-Location": "/test"},
),
)

for header, expected in tests:
assert helpers.remove_entity_headers(header) == expected


def test_import_string_class():
obj = helpers.import_string("sanic.config.Config")
assert isinstance(obj, Config)
Expand Down
8 changes: 8 additions & 0 deletions tests/test_response.py
Expand Up @@ -178,6 +178,10 @@ async def no_content_unmodified_handler(request: Request):
async def unmodified_handler(request: Request):
return json(JSON_DATA, status=304)

@app.get("/precondition")
async def precondition_handler(request: Request):
return json(JSON_DATA, status=412)

@app.delete("/")
async def delete_handler(request: Request):
return json(None, status=204)
Expand All @@ -193,6 +197,10 @@ def test_json_response(json_app):
assert response.text == json_dumps(JSON_DATA)
assert response.json == JSON_DATA

request, response = json_app.test_client.get("/precondition")
assert response.status == 412
assert response.json == JSON_DATA


def test_no_content(json_app):
request, response = json_app.test_client.get("/no-content")
Expand Down

0 comments on commit cf71fc7

Please sign in to comment.