ConditionalGetMiddleware behavior improvement.

[penzoil]

This patch is intended to improve the behavior of the django.middleware.http.ConditionalGetMiddleware? middleware. When a particular django view has at least two decorators that work with redirecting behavior, Firefox will send a ETag that seems to be valid since the response is nothing more than a Location redirect, thus the content-length is 0 and the ETag is valid for the two steps created by the two decorators that redirect the user to different pages (Example decorator : @login_required and another that acts very similarly). The bug is that the browser will receive a Not Modified header once the users passes the first decorator when he gets redirected by to the original view.

The FIX: The fix changes the behavior of the django.middleware.http.ConditionalGetMiddleware? to not send a 304 response code if the content-length is 0 or undefined.

I'm not sure if it's proper, but it seems good of a fix and it actually fixes my issue. So, I just wanted to shared it, hopefully it can make it in the code-base.

This fix will require a little bit of documentation change to explain the extra condition.

[Russell Keith-Magee]

I don't deny that this fixes the problem you describe, but it feels like there is something else going on. Firefox should only be sending an etag if the server provided one. Before I start making exceptions in ConditionalGetMiddleware, I'd like to rule out the possiblity that the ETags are getting set incorrectly in the first place.

Can you provide a specific test case where this happens i.e., an example view, decorated in a way that exhibits the problem? You should also provide your Firefox version, in case it is version dependent.

[penzoil]

You probably have a point there. Maybe we could return no ETag when a Location header is sent with the response. I'll have to test this when I have time.

For what is of the test case views, I'll get on that as soon as I can. For what is of the Firefox version it was 3.5.7 at the time of implementation (On Windows).

[Forest Bond]

Hi,

The problem is that patch_response_headers sets an ETag header regardless of the response status_code. This differs from the behavior implemented by CommonMiddleware, which only sets ETags when 200 <= response.status_code < 300. Making it consistent with CommonMiddleware behavior fixes things for me.

I'll attach a new patch.

Thanks,
Forest

[Forest Bond]

Patch fixing patch_response_headers.

[Forest Bond]

Resetting some flags on the ticket given new patch.

[Forest Bond]

Hi,

I'm sorry, it looks like CommonMiddleware actually does set an ETag for non-2XX responses, but it never returns HTTP 304 unless the response generated by the view has a 2XX status code.

Now I'm unsure about what the proper fix should be. I think we should do one or both of the following:

1. Don't set ETag on non-2XX responses (CommonMiddleware currently does *not* do this, my patch implements it for patch_response_headers).
2. Don't replace non-2XX responses with HTTP 304 (CommonMiddleware currently does this, ConditionalGetMiddleware currently does *not* do this).

I suspect that CommonMiddleware should be fixed to not set an ETag on non-2XX responses.

Thoughts?

Thanks,
Forest

[Russell Keith-Magee]

Reverting to DDN. The problem seems real, but we need to work out the right solution. Some programatic test cases would help considerably.

[Aymeric Augustin]

Change UI/UX from NULL to False.

[Aymeric Augustin]

Change Easy pickings from NULL to False.

[Aymeric Augustin]

As said by Russell early in the thread, this is actually a problem with ETag generation, which is tracked in #17834.