Merge lp:~vila/bzr/2.2-568421-http-error-length into lp:bzr/2.2

[Vincent Ladeuil]

Set a Content-Length header on errors for HTTP/1.1

Targeting 2.2 since this concerns maverick and using the right basis this time.

This fixes bug #568421 which was only occurring on gentoo when pycurl was used with gnutls
so far (which made it harder to reproduce, at least for me).

This is due to a long standing bug in the python BaseHTTPServer: some http/1.1 clients requires a length even if the connection is closed. pycurl/gnutls seems to be the only combination choking on this one (but pycurl is also pretty pedantic about connection closes).

It looks like maverick changed the ssl implementation used for pycurl (which made the bug easier to reproduce) so the diagnosis was easier.

There is no easy way to reuse the base send_error so I went for a simplified implementation less likely to fail in the future.

[John A Meinel (jameinel)]

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

Vincent Ladeuil wrote:
> Vincent Ladeuil has proposed merging lp:~vila/bzr/2.2-568421-http-error-length into lp:bzr/2.2.
>
> Requested reviews:
> bzr-core (bzr-core)
> Related bugs:
> #568421 test_get_unknown_file fails for pycurl if curl was compiled against gnutls
> https://bugs.launchpad.net/bugs/568421
>
>
> Targeting 2.2 since this concerns maverick and using the right basis this time.
>
> This fixes bug #568421 which was only occurring on gentoo when pycurl was used with gnutls
> so far (which made it harder to reproduce, at least for me).
>
> This is due to a long standing bug in the python BaseHTTPServer: some http/1.1 clients requires a length even if the connection is closed. pycurl/gnutls seems to be the only combination choking on this one (but pycurl is also pretty pedantic about connection closes).
>
> It looks like maverick changed the ssl implementation used for pycurl (which made the bug easier to reproduce) so the diagnosis was easier.
>
> There is no easy way to reuse the base send_error so I went for a simplified implementation less likely to fail in the future.
>

Consider my comments on the bzr.dev proposal to apply here.

 review: needsfixing

(fine to land on 2.2, as long as we don't send content-length
inappropriately, and the class attributes are put somewhere discoverable.)

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkxFwBYACgkQJdeBCYSNAAP+ZwCgr3MtoaWbABdeHHpKqJNi2gbi
8mYAoMOrHt209ZoozBWnjxMjrO3yLXjd
=0X6m
-----END PGP SIGNATURE-----

review: Needs Fixing

[Vincent Ladeuil (vila)]

sent to pqm by email

=== modified file 'NEWS'
--- NEWS	2010-07-19 10:44:20 +0000
+++ NEWS	2010-07-20 15:21:19 +0000
@@ -38,6 +38,9 @@
 * Don't traceback trying to unversion children files of an already
   unversioned directory.  (Vincent Ladeuil, #494221)
 
+* ``HTTP/1.1` test servers now set a ``Content-Length`` header to comply
+  with pedantic ``HTTP/1.1`` clients. (Vincent Ladeuil, #568421)
+
 * Progress bars prefer to truncate the text message rather than the
   counters.  The spinner is shown between the network transfer indicator
   and the progress message.  (Martin Pool)
 
=== modified file 'bzrlib/tests/http_server.py'
--- bzrlib/tests/http_server.py	2010-02-23 07:43:11 +0000
+++ bzrlib/tests/http_server.py	2010-07-20 15:21:19 +0000
@@ -89,6 +89,41 @@
                                      errno.ECONNABORTED, errno.EBADF)):
                 raise
 
+    error_content_type = 'text/plain'
+    error_message_format = '''\
+Error code: %(code)s.
+Message: %(message)s.
+'''
+
+    def send_error(self, code, message=None):
+        """Send and log an error reply.
+
+        We redefine the python-provided version to be able to set a 
+        ``Content-Length`` header as some http/1.1 clients complain otherwise
+        (see bug #568421).
+
+        :param code: The HTTP error code.
+
+        :param message: The explanation of the error code, Defaults to a short
+             entry.
+        """
+
+        if message is None:
+            try:
+                message = self.responses[code][0]
+            except KeyError:
+                message = '???'
+        self.log_error("code %d, message %s", code, message)
+        content = (self.error_message_format %
+                   {'code': code, 'message': message})
+        self.send_response(code, message)
+        self.send_header("Content-Type", self.error_content_type)
+        self.send_header("Content-Length", "%d" % len(content))
+        self.send_header('Connection', 'close')
+        self.end_headers()
+        if self.command != 'HEAD' and code >= 200 and code not in (204, 304):
+            self.wfile.write(content)
+
     _range_regexp = re.compile(r'^(?P<start>\d+)-(?P<end>\d+)$')
     _tail_regexp = re.compile(r'^-(?P<tail>\d+)$')