304 responses should not have Content-Length header

Bug #143486 reported by Jens Vagelpohl on 2005-08-14
2
Affects Status Importance Assigned to Milestone
Zope 2
Medium
Martijn Pieters

Bug Description

According to RFC 2616, 304 responses should not have a Content-Length header (and a few others Zope is sending, but this is strictly about content-length). While it is possible to prevent setting a content-length header in application code (such as in OFS.Image etc), ZServer.HTTPResponse is overly helpful and forces one if it does not exist.

The included patch and test (against the Zope 2.7 branch as of August 14th, 2005) fixes this and should apply cleanly against the Zope 2.8 branch and HEAD. Cannot test that right now because I am on dialup and svn.zope.org just hangs when I am trying to get it.

This report is an extension of a CMF collector report:

http://www.zope.org/Collectors/CMF/372

Uploaded: zserver_response.patch

Attaching patch

Michael Dunstan (michael-elyt) wrote :

Some thoughts about the patch:

Probably want to check the handling of the Connection header. Don't need to use "Connection: close" for a 304 for example?

Might as well handle the other status that also don't want a content length: 100, 101, 102, 204?

Should the approach be more forceful and explicitly remove any Content-Length header? (At the moment the patch avoids adding Content-Length but that does not stop application code from getting it wrong.)

Well, if you have the time to check every single response status and see if it needs a content-length header set, or if you want to make 100% sure none of the other "unwanted" headers are set for a 304 response, feel free to submit an improved patch. I don't have that time so I was concentrating on the specific problem at hand, the content-length header for 304 responses.

Michael Dunstan (michael-elyt) wrote :

Uploaded: zserver-content-length.patch

The thing I was more concerned about was a side effect of the original patch where a 304 would now force a connection close.

Uploading a patch that has a go at trying to avoid that. (Using Zope-2_8-branch.)

Thanks for taking the time to flesh that out. That patch looks great.

Sascha Welter (betabug) wrote :

Also: Current versions of Safari have a problem with content-type 'text/plain' being present on 304 replies.

Martijn Pieters (mjpieters) wrote :

Status: Pending => Accepted

 Supporters added: mj

Thanks for bringing this bug to my attention, betabug. I'll work in content-type as well, and merge the patch.

Martijn Pieters (mjpieters) wrote :

Status: Accepted => Resolved

I merged the patch into Zope 2.9, 2.10 and trunk, with the following changes:

- Split test method into several individual ones and adapted them
  to an existing HTTPResponse testcase.
- Added removal of content-type header on no-content responses,
  including a test

Thanks for the ground work and the great patch! Merging this was long overdue.

Checkins:

2.9: http://svn.zope.org/Zope/?rev=73801&view=rev
2.10: http://svn.zope.org/Zope/?rev=73802&view=rev
Trunk: http://svn.zope.org/Zope/?rev=73803&view=rev

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers