it looks like someone broke http:// in dev

[Marius Kruger]

2009/2/7 John Arbash Meinel <john at arbash-meinel.com>
...

> It seems that by default file-like objects support readline(size). I
> don't believe we ever use that functionality.
>
> However, in httplib, the SSLFile that they use to wrap the ssl
> connection doesn't have a size parameter.
>
> We don't ever use the size argument, but if we want to be fully capable,
> then the fix would be:
>
> === modified file 'bzrlib/transport/http/_urllib2_wrappers.py'
> - --- bzrlib/transport/http/_urllib2_wrappers.py  2009-01-29 14:27:28 +0000
> +++ bzrlib/transport/http/_urllib2_wrappers.py  2009-02-07 21:38:59 +0000
> @@ -80,7 +80,16 @@
>          return s
>
>     def readline(self, size=-1):
> - -        s = self.filesock.readline(size)
> +        if size < 0:
> +            # Unfortunately in python 2.5 (and others?)
> sslsocket.makefile()
> +            # returns an SSLFile wrapper which does not support the size
> +            # argument to readline. Since most clients don't need it
> anyway, we
> +            # workaround that fact by only supplying it if the client
> actually
> +            # requested a specific size. (Which would still fail for
> SSLFile,
> +            # but that isn't our problem, as we don't use it.)
> +            s = self.filesock.readline()
> +        else:
> +            s = self.filesock.readline(size)
>          self._report_activity(len(s), 'read')
>         return s
>
> So I'm positive that bzrlib doesn't ever use "readline(size)" because
> otherwise https would never have worked, and it is only because
> Vincent's recent wrapper always supplies that value that it started
> breaking.
>
> Technically it is a bug in httplib's SSLFile implementation, but it is
> one that we can just work around. Personally, I would be fine just
> removing the size parameter altogether.


Are you planning to submit something?

I'm quite surprised that we don't have tests for this.
Maybe we should add a http blackbox test (extending
http_utils.TestCaseWithWebserver or something else?)
testing some basic workflow actions like
info, init, co, up, ci, push, pull, log  to make sure that it doesn't break
all of a sunday when nobody is looking.

regards
marius
-------------- next part --------------
An HTML attachment was scrubbed...
URL: https://lists.ubuntu.com/archives/bazaar/attachments/20090208/4ddde441/attachment.htm