Opened 5 years ago

Closed 2 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: master
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)

middleware_gzip_patch_with_docs.diff (6.2 KB) - added by Aaron Cannon 5 years ago.
Patch, includes docs and tests

Download all attachments as: .zip

Change History (12)

Changed 5 years ago by Aaron Cannon

Patch, includes docs and tests

comment:1 Changed 5 years ago by Aaron Cannon

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset

The patch does the following:

  1. adds a check for IE5, IE6 and Netscape 4, and if the browser matches, doesn't compress the content.
  2. Rearranges the code a bit so that more common cases to bail out are tested for first.
  3. Adds a few tests for the new code.
  4. Updates the docs to clarify what browsers are tested for.

comment:2 Changed 5 years ago by Aaron Cannon

Has patch: set

comment:3 Changed 5 years ago by Aymeric Augustin

Owner: changed from nobody to Aymeric Augustin
Triage Stage: UnreviewedAccepted

This looks interesting.

comment:4 Changed 5 years ago by Aymeric Augustin

Owner: changed from Aymeric Augustin to nobody
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?

comment:5 Changed 2 years ago by Aymeric Augustin

Resolution: wontfix
Status: newclosed

I think usage of IE 6 and Netscape 4 has fallen sufficiently to make this unnecessary. Sorry.

comment:6 in reply to:  5 Changed 2 years ago by Aaron Cannon

Resolution: wontfix
Status: closednew

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:7 Changed 2 years ago by Aymeric Augustin

Oops. Thank you for reopening.

comment:8 Changed 2 years ago by Aaron Cannon

Summary: The GZip Middleware's browser checking is too broad and misses versions of Netscape which have similar bugsThe GZip Middleware's browser checking is buggy, as it does not send compressed content to modern versions of IE that support it.

comment:9 Changed 2 years ago by Aymeric Augustin

Patch needs improvement: unset

comment:10 Changed 2 years ago by Tim Graham

Triage Stage: AcceptedReady for checkin

comment:11 Changed 2 years ago by Aymeric Augustin <aymeric.augustin@…>

Resolution: fixed
Status: newclosed

In df09d854828bcff56eb72f48ff1ba8fce7e90c90:

Fixed #17552 -- Removed a hack for IE6 and earlier.

It prevented the GZipMiddleware from compressing some data types even on
more recent version of IE where the corresponding bug was fixed.

Thanks Aaron Cannon for the report and Tim Graham for the review.

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