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

Vincent Ladeuil v.ladeuil+lp at free.fr
Wed Jul 4 12:47:40 BST 2007


>>>>> "aaron" == Aaron Bentley <aaron.bentley at utoronto.ca> writes:

<snip/>


    >> >> 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
    >> 
    aaron> Why not start the retry count at 0?
    >> 
    >> Indeed.

    aaron> Actually, what I meant was to set the retry count to 0 in
    aaron> AbstractAuthHandler, and then the code in auth_required would be.

    aaron> self._retry_count += 1
    aaron> if self._retry_count > self._max_retry:
    aaron>     # Let's be ready for next round
    aaron>     self._retry_count = 0
    aaron>     return None

    aaron> No need to check for None at all, so you get rid of this bit:

    aaron>         if self._retry_count is None:
    aaron>             self._retry_count = 1

    aaron> Not a big deal, so I've sent it in without.  But I thought it would make
    aaron> the code simpler.

No, I replied too quickly, auth_required is called only when
either no credentials have been provided initially (say, from the
command line) or when the credentials provided are wrong (say,
when we prompt the user for a password).

So I really want to initialize _retry_count to None (we have never
failed to authenticate) and set it to 1 when we make our first
retry and finally reset it to None when the authentication is
successful. Due to the urllib2 layering, I can't do a simple loop
and I prefer to cleanly separate the different states.

This will be modified in the future when we fix bug #121146, but
I needed something along that way because the CONNECT request
activated a different code path than the other requests that
leads to death-by-recursion-too-deep.

So thanks, for the merge, I'll open a cleanup branch to remind me
of the point.

   Vincent



More information about the bazaar mailing list