[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