Skip to content

fix(schema): drop bare 'ssl' substring match in TLS error classifier - #50

Closed
Danny-Dasilva wants to merge 2 commits into
mainfrom
fix/classifier-substring-leak
Closed

Danny-Dasilva wants to merge 2 commits into
mainfrom
fix/classifier-substring-leak

Conversation

@Danny-Dasilva

Copy link
Copy Markdown
Owner

Summary

The keyword set ('certificate', 'x509:', 'tls: failed to verify', 'ssl') in cycletls/schema.py:567-576 was substring-matched against the entire error message including the request URL. Any host containing ssl in its name (e.g. self-signed.badssl.com) caused every non-TLS error (EOF, idle-close) to be misclassified as TLSError instead of ConnectionError/CycleTLSError.

Fix: drop the bare 'ssl' term — Go's actual TLS error vocabulary is fully covered by the other three markers. The status==495 + 'handshake' branch above remains unchanged.

Why

Confirmed by the source-tracer investigator while triaging the recurring badssl flake. Exact reproduction: `'Get "https://self-signed.badssl.com\": http: server closed idle connection'` was being raised as TLSError when it's a transport idle-close.

Test plan

  • `uv run pytest tests/test_error_classification.py -v` (5 new regression cases — bug repro + existing-behavior preservation)
  • No other tests touch this code path

Part of the badssl-flake remediation series (#48 marks tests as @LiVe; this is the root-cause Python fix).

… match

The keyword set ('certificate', 'x509:', 'tls: failed to verify', 'ssl') was matched
against the entire error message including the request URL. Any host containing 'ssl'
in its name (e.g. self-signed.bad**ssl**.com) caused every non-TLS error (EOF,
idle-close) to be tagged TLSError instead of ConnectionError/CycleTLSError.

Drop the bare 'ssl' term — Go's actual TLS error vocabulary is covered by the other
three markers ('certificate', 'x509:', 'tls: failed to verify'). The status==495 +
'handshake' branch above remains unchanged.

Includes 5 regression tests in tests/test_error_classification.py covering both
the bug case and existing-behavior preservation.
@Danny-Dasilva
Danny-Dasilva force-pushed the fix/classifier-substring-leak branch from 76f31fa to c398d21 Compare April 27, 2026 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant