Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/middleware.md
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,8 @@ The following arguments are supported:

* `minimum_size` - Do not GZip responses that are smaller than this minimum size in bytes. Defaults to `500`.

The middleware won't GZip responses that already have a `Content-Encoding` set, to prevent them from being encoded twice.

## BaseHTTPMiddleware

An abstract class that allows you to write ASGI middleware against a request/response
Expand Down
8 changes: 8 additions & 0 deletions starlette/middleware/gzip.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ def __init__(self, app: ASGIApp, minimum_size: int, compresslevel: int = 9) -> N
self.send: Send = unattached_send
self.initial_message: Message = {}
self.started = False
self.content_encoding_set = False
self.gzip_buffer = io.BytesIO()
self.gzip_file = gzip.GzipFile(
mode="wb", fileobj=self.gzip_buffer, compresslevel=compresslevel
Expand All @@ -48,6 +49,13 @@ async def send_with_gzip(self, message: Message) -> None:
# Don't send the initial message until we've determined how to
# modify the outgoing headers correctly.
self.initial_message = message
headers = Headers(raw=self.initial_message["headers"])
self.content_encoding_set = "content-encoding" in headers
elif message_type == "http.response.body" and self.content_encoding_set:
if not self.started:
self.started = True
await self.send(self.initial_message)
await self.send(message)
elif message_type == "http.response.body" and not self.started:
Comment thread
kklingenberg marked this conversation as resolved.
self.started = True
body = message.get("body", b"")
Expand Down
24 changes: 24 additions & 0 deletions tests/middleware/test_gzip.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,3 +76,27 @@ async def generator(bytes, count):
assert response.text == "x" * 4000
assert response.headers["Content-Encoding"] == "gzip"
assert "Content-Length" not in response.headers


def test_gzip_ignored_for_responses_with_encoding_set(test_client_factory):
def homepage(request):
async def generator(bytes, count):
for index in range(count):
yield bytes

streaming = generator(bytes=b"x" * 400, count=10)
return StreamingResponse(
streaming, status_code=200, headers={"Content-Encoding": "br"}
)

app = Starlette(
routes=[Route("/", endpoint=homepage)],
middleware=[Middleware(GZipMiddleware)],
)

client = test_client_factory(app)
response = client.get("/", headers={"accept-encoding": "gzip, br"})
assert response.status_code == 200
assert response.text == "x" * 4000
assert response.headers["Content-Encoding"] == "br"
assert "Content-Length" not in response.headers
Comment on lines +81 to +102
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, this test creates a StreamingResponse that claims to be encoded as Brotli but actually isn't, since its body is just b"x" * 4000, right? How is it possible that this test passes? Doesn't the client try to process brotli automatically on the response and fails?

>>> import brotli
>>> decompressor = brotli.Decompressor()
>>> decompressor.process(b"x" * 4000)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
brotli.error: BrotliDecoderDecompressStream failed while processing the stream

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've created what I think is a better test here. @Kludex If you're interested, I can submit a PR updating the test here as well.

Copy link
Copy Markdown
Owner

@Kludex Kludex Nov 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah... It was my bad. This test was modified, you can see how it is on master.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh indeed, I found it