response.appendHeader leaves carriage return cruft behind

[newbery]

In ZPublixher.HTTPResponse.appendHeader, we seem to be inserting some carriage return cruft in the header value which doesn't get cleaned up after.

Possible patch...

--- HTTPResponse.py
+++ (clipboard)
@@ -592,14 +592,16 @@
         set for the header. '''
         name, value = _scrubHeader(name, value)
         name = name.lower()
+ scrubbed = True

         headers = self.headers
         if headers.has_key(name):
             h = headers[name]
- h = "%s%s\r\n\t%s" % (h,delimiter,value)
+ h = "%s%s%s" % (h,delimiter,value)
+ scrubbed = False
         else:
             h = value
- self.setHeader(name,h, scrubbed=True)
+ self.setHeader(name,h, scrubbed=scrubbed)

     def isHTML(self, s):
         s = s.lstrip()

[newbery (ric-digitalmarbles)]

Hmm... i'm pretty sure I filed this as Zope2 bug. Now how do I fix this...

[newbery (ric-digitalmarbles)]

Accidentally filed as grok bug... fixed now

[Tres Seaver (tseaver)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Launchpad Bug Tracker wrote:
> You have been subscribed to a public bug:
>
> In ZPublixher.HTTPResponse.appendHeader, we seem to be inserting some
> carriage return cruft in the header value which doesn't get cleaned up
> after.
>
> Possible patch...
>
> --- HTTPResponse.py
> +++ (clipboard)
> @@ -592,14 +592,16 @@
> set for the header. '''
> name, value = _scrubHeader(name, value)
> name = name.lower()
> + scrubbed = True
>
> headers = self.headers
> if headers.has_key(name):
> h = headers[name]
> - h = "%s%s\r\n\t%s" % (h,delimiter,value)
> + h = "%s%s%s" % (h,delimiter,value)
> + scrubbed = False
> else:
> h = value
> - self.setHeader(name,h, scrubbed=True)
> + self.setHeader(name,h, scrubbed=scrubbed)
>
> def isHTML(self, s):
> s = s.lstrip()
>
> ** Affects: zope2
> Importance: Undecided
> Status: New

You don't specify why you think Zope's current behavior is broken. RFC
2616 specifies[1]:

  Header fields can be extended over multiple lines by preceding each
  extra line with at least one SP or HT.

Your patch also introduces a bug (any previous value for the header is
jammed directly against the new value), and you don't seem to get what
the 'scrubbed' argument is for (previous and new values are already
scrubbbed, so you don't need to scrub again).

[1] http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2

Tres.
- --
===================================================================
Tres Seaver [phone] <email address hidden>
Palladion Software "Excellence by Design" http://palladion.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAktXL8IACgkQ+gerLs4ltQ4NHQCeJOmBHLel67LFHjCHmbCW57Sz
wWkAoJeNUjmf9YBjkTQ+rZXkXyvyscSh
=DQt+
-----END PGP SIGNATURE-----

[newbery (ric-digitalmarbles)]

Hmm... maybe I pulled the trigger on this too fast. I was originally trying to figure some odd result with addHeader and put in appendHeader to see the difference. Got myself confused, the issue I'm seeing was with addHeader, not appendHeader. I blame the late hour.

Anyway, maybe I'm pointing at the wrong culprit here? Can't seem to do integration testing with the addHeader. Maybe it's the testbrowser? Example follows:

Using Plone with the front page adding a couple of test headers using addHeader.

First via command line...
% wget -O- -S "http://localhost:8080/Plone"

HTTP/1.0 200 OK
Server: Zope/(2.12.3, python 2.6.4, darwin) ZServer/1.1
Date: Thu, 21 Jan 2010 08:17:40 GMT
Content-Length: 14887
Content-Language: en
Expires: Mon, 24 Jan 2000 08:17:40 GMT
Connection: Keep-Alive
Content-Type: text/html;charset=utf-8
X-Testheader: value1
X-Testheader: value2
...

So wget gets me the expected result. Now via an integration test in my Plone product...

... (a bunch of test framework setup) ...

>>> from Products.Five.testbrowser import Browser
>>> browser = Browser()
>>> browser.open(self.portal.absolute_url())
>>> print browser.headers

Content-Language: en
Content-Length: 14556
Content-Type: text/html;charset=utf-8
Expires: Mon, 24 Jan 2000 08:19:28 GMTContent-Type: text/html; charset=utf-8
X-UA-Compatible: IE=edge

So the integration test is missing a few headers. I'm guessing several missing headers (Server, Date, Connection) are because the testbrowser isn't going through the ZServer so I'm ignoring those. But I'm also missing the X-Testheaders. But if I instead only set one Testheader, it shows up -- multiple ones and they disappear to the test browser. A bug in the test browser then? Should I file another bug report?

[Hanno Schlichting (hannosch)]

It's no clear from the description if there is a problem in Zope 2 or if the testing environment (zope.testbrowser) is at fault here.

[newbery (ric-digitalmarbles)]

Sorry, meant to refile the bug. It's an problem with Five.testbrowser which chokes on multiple header lines.

[Colin Watson (cjwatson)]

The zope2 project on Launchpad has been archived at the request of the Zope developers (see https://answers.launchpad.net/launchpad/+question/683589 and https://answers.launchpad.net/launchpad/+question/685285). If this bug is still relevant, please refile it at https://github.com/zopefoundation/zope2.