urllib does not let SSLCertVerificationError out, and a fake said it did
The certificate fix shipped and failed on the machine it was written for, with
the exact traceback it was meant to prevent. The retry was written
except ssl.SSLCertVerificationError:
and urllib never raises that from urlopen. It catches it and re-raises
urllib.error.URLError(err), carrying the original on .reason. So the except
matched nothing, ever, and the fallback could not fire.
The unit test passed throughout, because the stub it used raised the bare SSL
error - a shape real urllib never produces. That is the lesson: a fake that
agrees with the author is worse than no test, because it converts an untested
path into a tested-looking one. This file already says that about
UPDATE ... RETURNING and about the in-memory API fake, and it got written
again anyway.
_is_cert_failure checks the exception and its .reason, and the tests now raise
URLError(SSLCertVerificationError(...)) - what the traceback actually shows. A
plain URLError is re-raised untouched, and a test asserts no second attempt is
made for one.
Beside the stubs there is now a real reproduction, opt-in behind
BEHAVISION_NETWORK_TESTS=1. Python's default context honours SSL_CERT_FILE, so
an empty file gives a context that trusts nobody - the python.org condition
exactly - while certifi is loaded by path and is unaffected. It skips rather
than passes where it cannot reproduce that, and the difference is measured:
macOS Command Line Tools LibreSSL 2.8.3 128 CAs with an empty CA file
python.org / pyenv build OpenSSL 3.5.8 0 CAs -> reproduces it
Checked for teeth by putting the shipped except back: both the corrected stub
test and the live one fail, and pass again when it is restored.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KGcjxF1cNLcuwc3DAPcnfj
This commit is contained in:
@@ -5,6 +5,7 @@ from __future__ import annotations
|
||||
import logging
|
||||
import shutil
|
||||
import ssl
|
||||
import urllib.error
|
||||
import urllib.request
|
||||
from pathlib import Path
|
||||
|
||||
@@ -65,6 +66,27 @@ def _https_context() -> "ssl.SSLContext | None":
|
||||
return ssl.create_default_context(cafile=certifi.where())
|
||||
|
||||
|
||||
def _is_cert_failure(err: BaseException) -> bool:
|
||||
"""Is this a certificate-verification failure, however it is wrapped?
|
||||
|
||||
`urllib` does NOT let `ssl.SSLCertVerificationError` out. It catches it and
|
||||
re-raises `urllib.error.URLError(err)`, carrying the original on `.reason`
|
||||
- so `except ssl.SSLCertVerificationError` around `urlopen` matches
|
||||
nothing, ever.
|
||||
|
||||
That is not a subtlety this file gets to record academically: the first
|
||||
version of the fallback below was written exactly that way, shipped, and
|
||||
failed on the machine it was written for with the very traceback it was
|
||||
meant to prevent. The unit test passed throughout, because the fake it
|
||||
used raised the bare SSL error - a shape real urllib never produces. A
|
||||
stub that agrees with the author is worse than no test, and the test now
|
||||
raises what urllib raises.
|
||||
"""
|
||||
reason = getattr(err, "reason", None)
|
||||
return isinstance(err, ssl.SSLCertVerificationError) or \
|
||||
isinstance(reason, ssl.SSLCertVerificationError)
|
||||
|
||||
|
||||
def _urlopen(url: str, timeout: float = 60.0):
|
||||
"""Open a URL, falling back to certifi's CA bundle on a verify failure.
|
||||
|
||||
@@ -75,7 +97,13 @@ def _urlopen(url: str, timeout: float = 60.0):
|
||||
"""
|
||||
try:
|
||||
return urllib.request.urlopen(url, timeout=timeout)
|
||||
except ssl.SSLCertVerificationError:
|
||||
except (urllib.error.URLError, ssl.SSLCertVerificationError) as err:
|
||||
# Only a certificate problem is worth a second attempt. "No route to
|
||||
# host" and "connection refused" arrive as URLError too, and retrying
|
||||
# those with a different CA list changes nothing except how long the
|
||||
# operator waits for the real message.
|
||||
if not _is_cert_failure(err):
|
||||
raise
|
||||
ctx = _https_context()
|
||||
if ctx is None:
|
||||
raise
|
||||
|
||||
Reference in New Issue
Block a user