Opened 17 years ago

Closed 17 years ago

#3206 closed defect (fixed)

USE_ETAGS returns status 304 even if response.status_code=400

Reported by: mihai.preda@… Owned by: Adrian Holovaty
Component: Core (Other) Version: dev
Severity: normal Keywords: rev 4235
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

When USE_ETAGS=True,
a page which wants to return a 400 status code (by setting response.status_code=400),
may return a 304 status code on reloads (because the content of the page didn't change).

I don't know if this is the correct behavior or not, but I find it surprising that a 400-status is transformed into a 304-status.
I would expect USE_ETAGS to only affect pages with 200 status code (or something like it).

Attachments (2)

etag_304.diff (625 bytes ) - added by Vinay Sajip <vinay_sajip@…> 17 years ago.
Patch for HTTP 304 being returned inappropriately
common.py.patch (682 bytes ) - added by Manuel Saelices <msaelices@…> 17 years ago.
Patch that fixes previous patch

Download all attachments as: .zip

Change History (13)

comment:1 by mihai_preda@…, 17 years ago

sorry, I've mispelled the email in the initial ticket.

comment:2 by Michael Radziej <mir@…>, 17 years ago

Can you please refer me to any rfc or similar standard that backs your opinion?

comment:3 by mihai_preda@…, 17 years ago

In RFC 2616, section 10.3.5, 304 Not Modified:
"If the client has performed a conditional GET request and access is allowed, but the document has not been modified, the server SHOULD respond with this status code (304)."

I see two conditions for 304: GET, and "access is allowed". I interpret "access is allowed" as meaning: 2xx status code.

Also it is obvious (IMO) that not any status code (e.g. 4xx, 5xx) may be replaced with 304, as this would effectively erase this significant information (the status code).

I would suggest to play it conservativelly and only rewrite 200 to 304, unless some more authoritative understanding of RFC 2616 is expressed.

comment:4 by Michael Radziej <mir@…>, 17 years ago

Triage Stage: UnreviewedAccepted

Makes sense to me. Thank you, Mihai Preda, for looking this up!

comment:5 by cactus, 17 years ago

I also get this behavior.

The client, in my case firefox, upon recieving an etag on a 404 response, will subsequently send an If-None-Match header for the next request of the same 404 page.

The rfc for If-None-Match states:

If the request would, without the If-None-Match header field, result in anything other than a 2xx or 304 status, then the If-None-Match header MUST be ignored. (See section 13.3.4 for a discussion of server behavior when both If-Modified-Since and If-None-Match appear in the same request.)

http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.26 

Django is returning a 304 not modified header, instead of a 404.

In duplicating the headers sent to an apache server serving a 404 page, I get a 404 on subsequent requests, and the if-none-match is responded to with a 404 as expected.

by Vinay Sajip <vinay_sajip@…>, 17 years ago

Attachment: etag_304.diff added

Patch for HTTP 304 being returned inappropriately

comment:6 by Vinay Sajip <vinay_sajip@…>, 17 years ago

Has patch: set

Patch added to only return a 304 when the original status was 200 and the ETag matches. Possibly any 2xx return should be treated this way, but I've gone with just 200 for now.

comment:7 by Vinay Sajip <vinay_sajip@…>, 17 years ago

Triage Stage: AcceptedReady for checkin

comment:8 by Malcolm Tredinnick, 17 years ago

Resolution: fixed
Status: newclosed

(In [5407]) Fixed #3206 -- Changed ETag comparison to only return 304 when the normal
status code would be in the range 200 - 299. This matches RFC 2616
requirements. Based on a patch from Vinay Sajip.

comment:9 by Manuel Saelices <msaelices@…>, 17 years ago

Resolution: fixed
Status: closedreopened

Patch is buggy.

by Manuel Saelices <msaelices@…>, 17 years ago

Attachment: common.py.patch added

Patch that fixes previous patch

comment:10 by Malcolm Tredinnick, 17 years ago

Wow.. that's a little humiliating (for me). Thanks for the extra pair of eyes.

comment:11 by Malcolm Tredinnick, 17 years ago

Resolution: fixed
Status: reopenedclosed

(In [5417]) Fixed #3206 -- Fixed typo in [5407]. This time with bonus testing. Thanks,
Manuel Saelices.

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