304 responses should not have Content-Length header

[Jens Vagelpohl]

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

[Jens Vagelpohl (dataflake-deactivatedaccount-deactivatedaccount)]

Uploaded: zserver_response.patch

Attaching patch

[Michael Dunstan (michael-elyt)]

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.)

[Jens Vagelpohl (dataflake-deactivatedaccount-deactivatedaccount)]

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)]

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.)

[Jens Vagelpohl (dataflake-deactivatedaccount-deactivatedaccount)]

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

[Jens Vagelpohl (dataflake-deactivatedaccount-deactivatedaccount)]

Just to have all the relevant info in in one place, here are links to the discussion on zope-dev:

http://mail.zope.org/pipermail/zope-dev/2005-August/025297.html

http://mail.zope.org/pipermail/zope-dev/2005-August/025303.html

http://mail.zope.org/pipermail/zope-dev/2005-August/025309.html

[Sascha Welter (betabug)]

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

[Martijn Pieters (mjpieters)]

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)]

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