Skip to content

Commit 8c45110

Browse files
committed
gh-53907: Reset the digest auth retry count when authentication fails
HTTPDigestAuthHandler.http_error_401() resets the retry count only when http_error_auth_reqed() returns normally. It doesn't when it raises -- and it raises once the count passes 5. The count stays set, so every later request through the same handler fails straight away with "digest auth failed", even with valid credentials. Reset it in a finally block so it runs on every exit path. ProxyDigestAuthHandler.http_error_407() had the same bug.
1 parent ed71655 commit 8c45110

3 files changed

Lines changed: 56 additions & 8 deletions

File tree

Lib/test/test_urllib2.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1751,6 +1751,48 @@ def test_unsupported_auth_digest_handler(self):
17511751
opener.add_handler(http_handler)
17521752
self.assertRaises(ValueError, opener.open, "http://www.example.com")
17531753

1754+
def test_digest_auth_retry_count_reset(self):
1755+
self._test_digest_auth_retry_count_reset(
1756+
urllib.request.HTTPDigestAuthHandler(), 401, "WWW-Authenticate")
1757+
1758+
def test_proxy_digest_auth_retry_count_reset(self):
1759+
self._test_digest_auth_retry_count_reset(
1760+
urllib.request.ProxyDigestAuthHandler(), 407, "Proxy-Authenticate")
1761+
1762+
def _test_digest_auth_retry_count_reset(self, auth_handler, code, header):
1763+
# A failed authentication must not leave the retry count set, or the
1764+
# handler rejects valid credentials on every later request.
1765+
challenge = ('%s: Digest realm="ACME Networks", nonce="%%s", '
1766+
'qop="auth"\r\n\r\n' % header)
1767+
1768+
class MockDigestHTTPHandler(urllib.request.BaseHandler):
1769+
# Answers with a fresh nonce every time, so the handler keeps
1770+
# retrying until it gives up.
1771+
def __init__(self):
1772+
self.count = 0
1773+
1774+
def http_open(self, req):
1775+
import email
1776+
self.count += 1
1777+
msg = email.message_from_string(challenge % self.count)
1778+
return self.parent.error(
1779+
"http", req, MockFile(), code, "Unauthorized", msg)
1780+
1781+
auth_handler.add_password("ACME Networks", "http://acme.example.com/",
1782+
"joe", "password")
1783+
opener = OpenerDirector()
1784+
opener.add_handler(auth_handler)
1785+
opener.add_handler(MockDigestHTTPHandler())
1786+
with self.assertRaises(urllib.error.HTTPError):
1787+
opener.open("http://acme.example.com/protected")
1788+
1789+
# The same handler must still authenticate a later request.
1790+
opener = OpenerDirector()
1791+
opener.add_handler(auth_handler)
1792+
opener.add_handler(MockHTTPHandlerRedirect(code, challenge % "nonce"))
1793+
response = opener.open("http://acme.example.com/protected")
1794+
self.assertEqual(response.code, 200)
1795+
17541796
def test_unsupported_auth_basic_handler(self):
17551797
# While using BasicAuthHandler
17561798
opener = OpenerDirector()

Lib/urllib/request.py

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1210,10 +1210,11 @@ class HTTPDigestAuthHandler(BaseHandler, AbstractDigestAuthHandler):
12101210

12111211
def http_error_401(self, req, fp, code, msg, headers):
12121212
host = urlparse(req.full_url)[1]
1213-
retry = self.http_error_auth_reqed('www-authenticate',
1214-
host, req, headers)
1215-
self.reset_retry_count()
1216-
return retry
1213+
try:
1214+
return self.http_error_auth_reqed('www-authenticate',
1215+
host, req, headers)
1216+
finally:
1217+
self.reset_retry_count()
12171218

12181219

12191220
class ProxyDigestAuthHandler(BaseHandler, AbstractDigestAuthHandler):
@@ -1223,10 +1224,11 @@ class ProxyDigestAuthHandler(BaseHandler, AbstractDigestAuthHandler):
12231224

12241225
def http_error_407(self, req, fp, code, msg, headers):
12251226
host = req.host
1226-
retry = self.http_error_auth_reqed('proxy-authenticate',
1227-
host, req, headers)
1228-
self.reset_retry_count()
1229-
return retry
1227+
try:
1228+
return self.http_error_auth_reqed('proxy-authenticate',
1229+
host, req, headers)
1230+
finally:
1231+
self.reset_retry_count()
12301232

12311233
class AbstractHTTPHandler(BaseHandler):
12321234

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Fix :class:`urllib.request.HTTPDigestAuthHandler` and
2+
:class:`~urllib.request.ProxyDigestAuthHandler` not resetting their retry count
3+
when authentication fails. The stale count made the handler reject valid
4+
credentials on all subsequent requests.

0 commit comments

Comments
 (0)