Django

Code

Ticket #4148 (closed: fixed)

Opened 2 years ago

Last modified 5 months ago

Inclusion of Middleware to fix IE VARY bug

Reported by: Michael Axiak <axiak@mit.edu> Assigned to: nobody
Milestone: Component: Core framework
Version: SVN Keywords: pdf, ie, middleware
Cc: axiak@mit.edu, axisofentropy@gmail.com Triage Stage: Ready for checkin
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

Description

There is a bug in Internet Explorer, described in http://support.microsoft.com/kb/824847/en-us?spid=8722&sid=global, where Vary headers will break certain applications. One easy way to create this bug is to generate a !PDF and loading the page in IE. This can either go in the core or in contrib, but I'm afraid it will be lost if it stays on a site like djangosnippets, where it could really help people who plan to generate PDFs.

Attachments

5065-fix_ie_error_bug__for_4148.diff (2.3 kB) - added by Michael Axiak <axiak@mit.edu> on 04/24/07 23:15:06.
The middleware. It includes the documentation.
5078-fix_ie_vary_bug_core__for_4148.diff (2.9 kB) - added by Michael Axiak <axiak@mit.edu> on 04/25/07 11:53:02.
Take 2: This file alters the BaseHandler? to do it and adds a setting to disable it if one wants.
5078-fix_ie_vary_bug_core__for_4148_2.diff (3.4 kB) - added by Michael Axiak <axiak@mit.edu> on 04/25/07 11:55:48.
Take 2: This file alters the BaseHandler? to do it and adds a setting to disable it if one wants. This patch is better.
5079-fix_ie_cache_bugs.diff (2.4 kB) - added by Michael Axiak <axiak@mit.edu> on 04/25/07 21:37:42.
Cleaned the patch a little bit...added fix for content-disposition
5079-fix_IE_cache_bugs_2.diff (2.9 kB) - added by Michael Axiak <axiak@mit.edu> on 04/25/07 21:51:30.
Cleaned the patch a little bit...added fix for content-disposition...take two
7737-fix_ie_cache_bugs_3.diff (3.0 kB) - added by Axis_of_Entropy on 06/25/08 00:18:49.
It's been a year, and this bug persists. It prevents documented examples like http://www.djangoproject.com/documentation/request_response/#telling-the-browser-to-treat-the-response-as-a-file-attachment I've modified the last submitted patch, bringing it in line with the newer version of the HttpResponse class.
4148-fixed.diff (2.9 kB) - added by mtredinnick on 07/06/08 10:24:00.
Updated and corrected(?) patch

Change History

04/24/07 23:15:06 changed by Michael Axiak <axiak@mit.edu>

  • attachment 5065-fix_ie_error_bug__for_4148.diff added.

The middleware. It includes the documentation.

04/24/07 23:15:42 changed by Michael Axiak <axiak@mit.edu>

  • needs_better_patch changed.
  • needs_tests changed.
  • needs_docs set to 1.

04/24/07 23:15:46 changed by Michael Axiak <axiak@mit.edu>

  • needs_docs deleted.

04/24/07 23:16:00 changed by Michael Axiak <axiak@mit.edu>

  • summary changed from Inclusion of Middleware to fix IE VARY bug to [patch] Inclusion of Middleware to fix IE VARY bug.

04/25/07 00:52:46 changed by Collin Grady <cgrady@the-magi.us>

  • summary changed from [patch] Inclusion of Middleware to fix IE VARY bug to Inclusion of Middleware to fix IE VARY bug.

04/25/07 05:59:37 changed by Simon G. <dev@simon.net.nz>

  • stage changed from Unreviewed to Design decision needed.

04/25/07 06:30:47 changed by mtredinnick

  • needs_better_patch set to 1.
  • stage changed from Design decision needed to Accepted.

This probably shouldn't be middleware. It's something that has to always be done to the response, because it's broken all the time. Because of the impact, this fix needs to be included. We can also leave out the Microsoft bashing in the initial docstring (although the reference to the bug we are fixing should stay).

04/25/07 11:53:02 changed by Michael Axiak <axiak@mit.edu>

  • attachment 5078-fix_ie_vary_bug_core__for_4148.diff added.

Take 2: This file alters the BaseHandler? to do it and adds a setting to disable it if one wants.

04/25/07 11:55:48 changed by Michael Axiak <axiak@mit.edu>

  • attachment 5078-fix_ie_vary_bug_core__for_4148_2.diff added.

Take 2: This file alters the BaseHandler? to do it and adds a setting to disable it if one wants. This patch is better.

04/25/07 11:58:09 changed by Michael Axiak <axiak@mit.edu>

  • keywords changed from pdf, iesux, middleware to pdf, ie, middleware.

Okay, I put it in the BaseHandler in what it seems a fairly clean way. If you want to move the actual response processing out of that file, you're free to do so. Also, sorry for putting [patch] in the summary, I thought that was standard (!).

04/25/07 14:20:42 changed by Michael Axiak <axiak@mit.edu>

  • cc set to axiak@mit.edu.

04/25/07 17:57:13 changed by SmileyChris

  • needs_better_patch deleted.
  • stage changed from Accepted to Ready for checkin.

Thanks Michael, looks pretty clean to me.

04/25/07 21:07:11 changed by SmileyChris

  • stage changed from Ready for checkin to Accepted.

Discussed a slightly less odd way to implement this on IRC

04/25/07 21:37:42 changed by Michael Axiak <axiak@mit.edu>

  • attachment 5079-fix_ie_cache_bugs.diff added.

Cleaned the patch a little bit...added fix for content-disposition

04/25/07 21:51:30 changed by Michael Axiak <axiak@mit.edu>

  • attachment 5079-fix_IE_cache_bugs_2.diff added.

Cleaned the patch a little bit...added fix for content-disposition...take two

04/25/07 21:56:16 changed by Michael Axiak <axiak@mit.edu>

  • stage changed from Accepted to Design decision needed.

The new update is very much like the other version, without a settings variable. It also includes a fix for a bug in IE whereby if you use Content-Disposition and either Pragma: no-cache or Control-Cache with either no-cache or no-store IE will not let you download it.

06/25/08 00:18:49 changed by Axis_of_Entropy

  • attachment 7737-fix_ie_cache_bugs_3.diff added.

It's been a year, and this bug persists. It prevents documented examples like http://www.djangoproject.com/documentation/request_response/#telling-the-browser-to-treat-the-response-as-a-file-attachment I've modified the last submitted patch, bringing it in line with the newer version of the HttpResponse class.

06/25/08 04:56:04 changed by russellm

  • stage changed from Design decision needed to Accepted.

This has been accepted several times; I can't see anything that needs an additional design decision. However, I don't have ready access to IE to validate the fix, so someone else will need to test the patch before it is ready for checkin.

06/25/08 10:28:23 changed by Karen Tracey <kmtracey@gmail.com>

I have machines with IE (W2K with IE6 and XP with IE7) so I thought I could test the fix. However I cannot recreate the problem. Using trunk r7739 (development server) and following the example at:

http://www.djangoproject.com/documentation/request_response/#telling-the-browser-to-treat-the-response-as-a-file-attachment

either specifying no Vary header or including a Vary header matching the one at:

http://support.microsoft.com/kb/824847/en-us?spid=8722&sid=global

I see no problem on either of my Windows machines. I choose "Open" from the popup and Excel opens with the file supplied in the response. Also tried a PDF and ran into no errors opening that either. If someone could tell me exactly what I need to do to hit the error, I'll try again to test the fix.

06/26/08 21:00:22 changed by Axis_of_Entropy

  • cc changed from axiak@mit.edu to axiak@mit.edu, axisofentropy@gmail.com.

Ah, there's a little more needed to reproduce this bug. Django does not always attach a Vary header to every HttpResponse. In my case, the sessions middleware was automatically adding one to a generic.list_detail view (although it wasn't really appropriate, see also bug #3586). So to reproduce this easily, make sure that your HttpResponse does include a Vary header by using a decorator:

1. create a views.py like so:

from django.http import HttpResponse
from django.views.decorators.vary import vary_on_cookie

@vary_on_cookie #explicitly ensure this HttpResponse includes a Vary header
def django_ticket_4148_sample_with_Vary(request):
    fileHandle = open('/var/www/static/foo.xls', 'r')
    my_data = fileHandle.read()
    response = HttpResponse(my_data, mimetype='application/vnd.ms-excel')
    response['Content-Disposition'] = 'attachment; filename=foo.xls'
    return response

2. Point your urlpatterns at this view, maybe like

urlpatterns = patterns('',
    (r'^foo.xls', 'views.django_ticket_4148_sample_with_Vary'),
)

3. Make sure an Excel file exists at the path shown above.
4. Visit the URL in Internet Explorer (6 or 7)
5. My IE6 spits out garbage when trying to view a .xls file like this. For other Content-Types like .pdf IE can't open their temporary file, and .odt is even weirder. Firefox works fine.

It's possible that some apache or python handler configurations could cover this up for IE users, so I'd like to hear from people running other distros.

My implementation, running on mod_python on Gentoo, is viewable here, for anyone with IE: http://entropyindustries.com/foo.xls

06/27/08 10:47:43 changed by Karen Tracey <kmtracey@gmail.com>

  • stage changed from Accepted to Ready for checkin.

OK, the key difference between what I was trying and your setup seems to be the fact that I was using the development server and you are using Apache. There must be something else that Apache is doing that the development server does not do that triggers the error, since no matter what I tried I could not recreate with the development server. So I tried again, this time using Apache as my server and then I could recreate the problem with both IE6 & IE7. Applying the patch 7737-fix_ie_cache_bugs_3.diff fixed the issue for both browsers. Promoting to ready for checkin since Russell's last comment seems to indicate all that was needed was testing/verification, which I've now done.

07/06/08 10:24:00 changed by mtredinnick

  • attachment 4148-fixed.diff added.

Updated and corrected(?) patch

07/06/08 10:26:44 changed by mtredinnick

  • stage changed from Ready for checkin to Accepted.

I've cleaned up the patch a bit, added some extra robustness, and moved the code to the correct locations, since we already have some "compulsory middleware" now.

Can somebody give this a quick check (if you're using IE, actually run the code)? I'm a bit tired at the moment and may easily have made some kind of bozo error. It doesn't crash when I run things, but, then again, I don't have IE -- or even Windows -- either. Can be moved back to "ready for checkin" when it's been tested.

07/06/08 12:07:51 changed by Karen Tracey <kmtracey@gmail.com>

  • stage changed from Accepted to Ready for checkin.

Verified 4148-fixed.diff using IE6 & IE7. Verified failures on r7852 without the patch, fixed by applying the patch. Verified headers from Firefox were unaffected by the patch (Vary still present after patch whereas it's gone for IE6/IE7). Didn't attempt to track changes to the Cache-Control header though.

07/06/08 20:45:19 changed by mtredinnick

  • status changed from new to closed.
  • resolution set to fixed.

(In [7856]) Fixed #4148 -- Changed the way attachments are served to IE to avoid some caching and header-related bugs there. Only comes into play when Internet Explorer is the user-agent.

Patch from Michael Axiak, with testing from Axis_of_Entropy and Karen Tracey.


Add/Change #4148 (Inclusion of Middleware to fix IE VARY bug)




Change Properties
Action