Opened 15 years ago

Closed 12 years ago

#10762 closed Cleanup/optimization (fixed)

Gzip Middleware Should Compress 201 (Created) Responses

Reported by: rwagner@… Owned by: Aymeric Augustin
Component: HTTP handling Version: dev
Severity: Normal Keywords: middleware gzip
Cc: Georges Dubus, Aaron Cannon Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

Hi,

RESTful web services that use POST for creation of resources often respond with a representation of the newly created resource, which can be lengthy. However, the appropriate status code is 201, which the Gzip middleware ignores. There's a trivial patch attached that contains these lines:

-        if response.status_code != 200 or len(response.content) < 200:
+        if response.status_code not in (200, 201) or len(response.content) < 200:

This is not a particularly big deal, just a convenience.

Thanks,
Rick

Attachments (3)

gzip.patch (536 bytes ) - added by rwagner@… 15 years ago.
10762.patch (646 bytes ) - added by Aymeric Augustin 13 years ago.
10762_with_tests.patch (2.3 KB ) - added by Georges Dubus 12 years ago.
The previous patch, with added tests

Download all attachments as: .zip

Change History (14)

by rwagner@…, 15 years ago

Attachment: gzip.patch added

comment:1 by Julian Bez, 15 years ago

While we are at it: What about 404 or 403 responses?

in reply to:  1 comment:2 by Rick Wagner, 15 years ago

Replying to julianb:

While we are at it: What about 404 or 403 responses?

I don't believe that was what was intended by the original condition. The 2XX status codes indicate successful operations, whereas 4XX are client errors. In practice, I think it is a good idea to render errors in the simplest formats possible, to make interpreting them easier on the client side. But, I can see the argument for compressing a custom 404 response that is rendered from a template, and could get large.

--Rick

comment:3 by Alex Gaynor, 15 years ago

Triage Stage: UnreviewedDesign decision needed

comment:4 by Aymeric Augustin, 13 years ago

The comment in gzip.py say: "It's not worth compressing non-OK or really short responses."

If non-OK responses are not worth compressing because they are expected to be short, testing their length is enough, which means the check on response.status_code should be removed.

If there is another reason why non-OK responses should not be compressed, the proper check is 200 <= response.status_code < 300, and the reason should be explained in a comment.

comment:5 by Chris Beaven, 13 years ago

Severity: Normal
Type: Cleanup/optimization

by Aymeric Augustin, 13 years ago

Attachment: 10762.patch added

comment:6 by Aymeric Augustin, 13 years ago

Easy pickings: unset

Regarding Rick's comment above: the content will only be gzipped if the user agent sent an Accept-Encoding: gzip header. It seems reasonable to assume it will be able to decode gzip, even if the server returns an error page.

comment:7 by Aymeric Augustin, 12 years ago

Easy pickings: set
Needs tests: set
Triage Stage: Design decision neededAccepted
UI/UX: unset

by Georges Dubus, 12 years ago

Attachment: 10762_with_tests.patch added

The previous patch, with added tests

comment:8 by Georges Dubus, 12 years ago

Cc: Georges Dubus added
Needs tests: unset

Here are simple tests to check that long responses are compressed and short responses are not, regardless of the return type. Are they ok ?

comment:9 by Aaron Cannon, 12 years ago

Cc: Aaron Cannon added

comment:10 by Aymeric Augustin, 12 years ago

Owner: changed from nobody to Aymeric Augustin

See also #17514.

comment:11 by Aymeric Augustin, 12 years ago

Resolution: fixed
Status: newclosed

In [17365]:

Fixed #10762, #17514 -- Prevented the GZip middleware from returning a response longer than the original content, allowed compression of non-200 responses, and added tests (there were none). Thanks cannona for the initial patch.

Note: See TracTickets for help on using tickets.
Back to Top