Compare commits
1 Commits
v0.5.6-dem
...
v0.5.7-dem
| Author | SHA1 | Date | |
|---|---|---|---|
| 0558344dc2 |
46
CLAUDE.md
46
CLAUDE.md
@@ -3653,3 +3653,49 @@ Reproduced end to end before fixing, against real wheels on Python 3.12: two
|
||||
builds of the same version, `--upgrade` leaves the old code in place and prints
|
||||
the same sentence the colleague's Mac printed, `--force-reinstall --no-deps`
|
||||
replaces it.
|
||||
|
||||
### `urllib` does not let `SSLCertVerificationError` out, and a fake said it did
|
||||
|
||||
The certificate fix above shipped and **failed on the machine it was written
|
||||
for, with the exact traceback it was meant to prevent**. The retry was written
|
||||
|
||||
```python
|
||||
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 whole lesson: a
|
||||
fake that agrees with the author is worse than no test, because it converts an
|
||||
untested path into a tested-looking one. The same sentence is already in this
|
||||
file about `UPDATE ... RETURNING` and about the in-memory API fake, and it was
|
||||
written again here anyway.
|
||||
|
||||
`_is_cert_failure` checks the exception **and** its `.reason`, and the tests
|
||||
now raise `URLError(SSLCertVerificationError(...))`, which is what his
|
||||
traceback shows. A plain `URLError` — "no route to host" — is re-raised
|
||||
untouched: retrying that with a different CA list changes nothing except how
|
||||
long the operator waits for the real message, and a test asserts the second
|
||||
attempt never happens.
|
||||
|
||||
Beside the stubs there is now a **real** reproduction,
|
||||
`test_against_a_real_machine_with_no_trust_store`, opt-in behind
|
||||
`BEHAVISION_NETWORK_TESTS=1` because it reaches github.com. Python's default
|
||||
context honours `SSL_CERT_FILE`, so pointing it at an empty file gives a
|
||||
default 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 rather than assumed:
|
||||
|
||||
```
|
||||
macOS Command Line Tools LibreSSL 2.8.3 128 CAs with SSL_CERT_FILE empty
|
||||
(reads the system keychain)
|
||||
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.
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -9,6 +9,7 @@ perfectly - numpy, onnxruntime, faiss, all of it - and then could not fetch a
|
||||
"""
|
||||
import io
|
||||
import logging
|
||||
import os
|
||||
import ssl
|
||||
import urllib.request
|
||||
|
||||
@@ -33,9 +34,18 @@ class _Resp(io.BytesIO):
|
||||
|
||||
|
||||
def _verify_error():
|
||||
return ssl.SSLCertVerificationError(
|
||||
"""What urlopen ACTUALLY raises, which is not what it looks like.
|
||||
|
||||
urllib catches ssl.SSLCertVerificationError and re-raises
|
||||
urllib.error.URLError(err), carrying the original on `.reason`. The first
|
||||
version of this test raised the bare SSL error - a shape real urllib never
|
||||
produces - so it passed against a fallback that could never fire, and the
|
||||
fix shipped and failed on the machine it was written for with the exact
|
||||
traceback it was meant to prevent.
|
||||
"""
|
||||
return urllib.error.URLError(ssl.SSLCertVerificationError(
|
||||
"[SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: "
|
||||
"unable to get local issuer certificate")
|
||||
"unable to get local issuer certificate"))
|
||||
|
||||
|
||||
def test_a_machine_with_no_trust_store_falls_back_to_the_bundled_one(monkeypatch):
|
||||
@@ -74,12 +84,22 @@ def test_a_working_trust_store_is_used_as_is(monkeypatch):
|
||||
|
||||
|
||||
def test_a_real_network_failure_is_not_disguised_as_a_certificate_problem(monkeypatch):
|
||||
"""A URLError is not automatically a certificate problem.
|
||||
|
||||
"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.
|
||||
"""
|
||||
calls = []
|
||||
|
||||
def fake(url, timeout=None, context=None):
|
||||
calls.append(context)
|
||||
raise urllib.error.URLError("no route to host")
|
||||
|
||||
monkeypatch.setattr(urllib.request, "urlopen", fake)
|
||||
with pytest.raises(urllib.error.URLError):
|
||||
model_assets._urlopen("https://example.invalid/m.onnx")
|
||||
assert calls == [None], f"a dead network was retried as a CA problem: {calls}"
|
||||
|
||||
|
||||
def test_progress_lines_survive_the_rewrite(tmp_path, monkeypatch, caplog):
|
||||
@@ -103,3 +123,42 @@ def test_progress_lines_survive_the_rewrite(tmp_path, monkeypatch, caplog):
|
||||
pct = [ln for ln in lines if ln.startswith("download: face detector ")]
|
||||
assert pct, f"no progress lines at all: {lines}"
|
||||
assert "download: face detector 100%" in pct, pct
|
||||
|
||||
|
||||
@pytest.mark.skipif(not os.environ.get("BEHAVISION_NETWORK_TESTS"),
|
||||
reason="set BEHAVISION_NETWORK_TESTS=1 to reach github.com")
|
||||
def test_against_a_real_machine_with_no_trust_store(tmp_path, monkeypatch):
|
||||
"""The whole thing, over the real network, with the real failure.
|
||||
|
||||
Every stub above is a statement about what I believe urllib does, and the
|
||||
first version of this file proved how much that is worth. Python's default
|
||||
context honours SSL_CERT_FILE, so pointing it at an EMPTY file reproduces
|
||||
a python.org macOS build exactly: a default context that trusts nobody.
|
||||
certifi is loaded by path and is unaffected by the variable, so if the
|
||||
fallback works here it works there.
|
||||
"""
|
||||
empty = tmp_path / "no-cas.pem"
|
||||
empty.write_text("")
|
||||
monkeypatch.setenv("SSL_CERT_FILE", str(empty))
|
||||
monkeypatch.setenv("SSL_CERT_DIR", str(tmp_path / "nothing"))
|
||||
|
||||
# Not every interpreter can be put into that state, and pretending
|
||||
# otherwise would turn this into a test that passes by not running. A
|
||||
# Python linked against LibreSSL - which is what macOS Command Line Tools
|
||||
# ships - reads the system keychain and ignores SSL_CERT_FILE entirely:
|
||||
# measured, 128 CAs loaded with the variable pointing at an empty file. A
|
||||
# python.org build links OpenSSL and honours it: 0 CAs, which is the
|
||||
# condition being reproduced.
|
||||
if ssl.create_default_context().get_ca_certs():
|
||||
pytest.skip("this interpreter ignores SSL_CERT_FILE (%s), so the "
|
||||
"no-trust-store condition cannot be reproduced here"
|
||||
% ssl.OPENSSL_VERSION)
|
||||
|
||||
# The premise: the default context really is broken in this process.
|
||||
with pytest.raises(urllib.error.URLError) as caught:
|
||||
urllib.request.urlopen(model_assets.YUNET_URL, timeout=30)
|
||||
assert model_assets._is_cert_failure(caught.value), caught.value
|
||||
|
||||
dest = tmp_path / "yunet.onnx"
|
||||
model_assets._fetch(model_assets.YUNET_URL, dest, "face detector")
|
||||
assert dest.stat().st_size > 200_000
|
||||
|
||||
Reference in New Issue
Block a user