[MERGE][bug #120768] https+urllib + proxy issues a CONNECT Request

[Aaron Bentley]

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

Vincent Ladeuil wrote:
> It appears that m2crypto
> http://chandlerproject.org/bin/view/Projects/MeTooCrypto is the
> best candidate to build upon. It's available for python >= 2.3,
> OpenSSL >= 0.9.7 on unix, mac and windows. 
> 
> I think that fits our needs, but feedback welcome. 
> 
> Also the 'Cryptography Export Notice' on the url above seems
> strange, I thought OpenSSL had solved these problems sometimes
> ago, what do I miss ?

The chandler project appears to have a US presence.  So US export bans
would apply to that software.  There was a general encryption export ban
in the US.  That general ban was lifted, but a few countries still have
special bans.

> * m2crypto:
> 
>   - the current version is 0.17 while both edgy and feisty
>     propose 0.16 in universe. What is the best way to get that
>     updated ?

Is the 0.17 version important for your work?

> ------------------------------------------------------------------------
> 
> # Bazaar revision bundle v0.9
> #
> # message:
> #   Fix #120678 by issuing a CONNECT request when https is used via a proxy.

I have not enough experience with https to know whether this is
required.  I take it on faith that it is.

> --- bzrlib/transport/http/_urllib2_wrappers.py
> +++ bzrlib/transport/http/_urllib2_wrappers.py
> @@ -108,6 +108,13 @@
>                  if self.debuglevel > 0:
>                      print "Consumed body: [%s]" % body
>              self.close()
> +        elif self.status == 200:
> +            # Whatever the request is, it went ok, so we surely don't want to
> +            # close the connection. Some cases are not correctly detected by
> +            # httplib.HTTPConnection.getresponse (called by
> +            # httplib.HTTPResponse.begin). The CONNECT response for the https
> +            # through proxy case is one.
> +            self.will_close = False
>  
>  
>  # Not inheriting from 'object' because httplib.HTTPConnection doesn't.
> @@ -125,16 +132,36 @@
>          # Preserve our preciousss
>          sock = self.sock
>          self.sock = None
> +        # Let httplib.HTTPConnection do its housekeeping 
>          self.close()
> +        # Restore our preciousss
>          self.sock = sock

^^^ This is pretty gross.  I'm looking forward to when you create
urllib3 ;-)

Technically, it would be better to do
self.sock = None
try:
    self.close()
finally:
    self.sock = sock

In case self.close throws an exception that you can recover from.

> +    def connect(self):
> +        httplib.HTTPConnection.connect(self)
> +        if self.proxied_host is None:
> +            self.connect_to_origin()

I don't understand why you're only doing something if proxied_host is
not None.

> +    Note: the proxy handling *may* modify the protocol used; the request may be
> +    against an https server proxied through an http proxy. So, https_request
> +    will be called, but later it's really http_open that will be called. This
> +    explain why we don't have to call self.parent.open as the urllib2 did.

^^^ "explains"

>      def update_auth(self, auth, key, value):
>          """Update a value in auth marking the auth as modified if needed"""
> @@ -805,6 +908,16 @@
>          :param headers: The headers for the authentication error response.
>          :return: None or the response for the authenticated request.
>          """
> +        # Don't try  to authenticate endlessly
> +        if self._retry_count is None:
> +            # The retry being recusrsive calls, None identify the first try
> +            self._retry_count = 1

Why not start the retry count at 0?

I don't understand urllib2 internals, nor do I understand https proxying
requirements.  So I'll give you a +1, but it's entirely based on your
track record, not this code.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGilge0F+nu1YWqI0RAoPcAJ94PYEoN7QnumJGZWGq02//ZGYFkwCdE7j6
q+li7ANHUUTNIX/rtecg5s0=
=kOyA
-----END PGP SIGNATURE-----