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
|
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`
|
the same sentence the colleague's Mac printed, `--force-reinstall --no-deps`
|
||||||
replaces it.
|
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 logging
|
||||||
import shutil
|
import shutil
|
||||||
import ssl
|
import ssl
|
||||||
|
import urllib.error
|
||||||
import urllib.request
|
import urllib.request
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
@@ -65,6 +66,27 @@ def _https_context() -> "ssl.SSLContext | None":
|
|||||||
return ssl.create_default_context(cafile=certifi.where())
|
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):
|
def _urlopen(url: str, timeout: float = 60.0):
|
||||||
"""Open a URL, falling back to certifi's CA bundle on a verify failure.
|
"""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:
|
try:
|
||||||
return urllib.request.urlopen(url, timeout=timeout)
|
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()
|
ctx = _https_context()
|
||||||
if ctx is None:
|
if ctx is None:
|
||||||
raise
|
raise
|
||||||
|
|||||||
@@ -9,6 +9,7 @@ perfectly - numpy, onnxruntime, faiss, all of it - and then could not fetch a
|
|||||||
"""
|
"""
|
||||||
import io
|
import io
|
||||||
import logging
|
import logging
|
||||||
|
import os
|
||||||
import ssl
|
import ssl
|
||||||
import urllib.request
|
import urllib.request
|
||||||
|
|
||||||
@@ -33,9 +34,18 @@ class _Resp(io.BytesIO):
|
|||||||
|
|
||||||
|
|
||||||
def _verify_error():
|
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: "
|
"[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):
|
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):
|
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):
|
def fake(url, timeout=None, context=None):
|
||||||
|
calls.append(context)
|
||||||
raise urllib.error.URLError("no route to host")
|
raise urllib.error.URLError("no route to host")
|
||||||
|
|
||||||
monkeypatch.setattr(urllib.request, "urlopen", fake)
|
monkeypatch.setattr(urllib.request, "urlopen", fake)
|
||||||
with pytest.raises(urllib.error.URLError):
|
with pytest.raises(urllib.error.URLError):
|
||||||
model_assets._urlopen("https://example.invalid/m.onnx")
|
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):
|
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 ")]
|
pct = [ln for ln in lines if ln.startswith("download: face detector ")]
|
||||||
assert pct, f"no progress lines at all: {lines}"
|
assert pct, f"no progress lines at all: {lines}"
|
||||||
assert "download: face detector 100%" in pct, pct
|
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