Opened 13 years ago
Closed 11 years ago
#17552 closed Bug (fixed)
The GZip Middleware's browser checking is buggy, as it does not send compressed content to modern versions of IE that support it.
| Reported by: | Aaron Cannon | Owned by: | nobody |
|---|---|---|---|
| Component: | HTTP handling | Version: | dev |
| Severity: | Normal | Keywords: | gzip middleware |
| Cc: | Triage Stage: | Ready for checkin | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
The GZip middleware currently only tests if the browser is IE. However, there are only three browser versions that I was able to identify with bugs in how they handle GZipped content. They are IE5, IE6 and Netscape4. However, the current code assumes that all IE versions have such bugs, and doesn't test at all for Netscape. This means that a lot of pages that could be compressed aren't being compressed because the browser is IE. Furthermore, the broken versions of IE and Netscape came out over 10 years ago, so they are a small minority.
Attachments (1)
Change History (12)
by , 13 years ago
| Attachment: | middleware_gzip_patch_with_docs.diff added |
|---|
comment:1 by , 13 years ago
The patch does the following:
- adds a check for IE5, IE6 and Netscape 4, and if the browser matches, doesn't compress the content.
- Rearranges the code a bit so that more common cases to bail out are tested for first.
- Adds a few tests for the new code.
- Updates the docs to clarify what browsers are tested for.
comment:2 by , 13 years ago
| Has patch: | set |
|---|
comment:3 by , 13 years ago
| Owner: | changed from to |
|---|---|
| Triage Stage: | Unreviewed → Accepted |
This looks interesting.
comment:4 by , 13 years ago
| Owner: | changed from to |
|---|---|
| Patch needs improvement: | set |
The patch is quite good, thanks!
patch_vary_headers(response, ('Accept-Encoding',)) must be done before ae = request.META.get('HTTP_ACCEPT_ENCODING', ''), because the result depends on the Accept-Encoding header from the next line on.
How did you determine the regexps for the various user agents? Is there a reference of user agents strings where I can check them?
follow-up: 6 comment:5 by , 11 years ago
| Resolution: | → wontfix |
|---|---|
| Status: | new → closed |
I think usage of IE 6 and Netscape 4 has fallen sufficiently to make this unnecessary. Sorry.
comment:6 by , 11 years ago
| Resolution: | wontfix |
|---|---|
| Status: | closed → new |
Replying to aaugustin:
I think usage of IE 6 and Netscape 4 has fallen sufficiently to make this unnecessary. Sorry.
Hi. I certainly can't argue with this. However, I suspect you might be misunderstanding the nature of the bug this ticket calls out. As I understand it, the current code does not do any compression on certain types of content meant for IE, no matter the version. This is the bug I was reporting.
So, IMHO, the proper solution here, if we don't wish to support the older browsers, is to disable any checking of the user agent string, and just rely on whether or not they request compressed content. That way, we're not refusing to send compressed content to modern versions of IE when it's requested.
comment:8 by , 11 years ago
| Summary: | The GZip Middleware's browser checking is too broad and misses versions of Netscape which have similar bugs → The GZip Middleware's browser checking is buggy, as it does not send compressed content to modern versions of IE that support it. |
|---|
comment:9 by , 11 years ago
| Patch needs improvement: | unset |
|---|
New patch: https://github.com/django/django/pull/2781
comment:10 by , 11 years ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
comment:11 by , 11 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
Patch, includes docs and tests