Skip to content

create_ssl_context: raise when verify is a str and cert is provided#990

Merged
Kludex merged 8 commits into
pydantic:mainfrom
mbeijen:ssl-context-tuple
Jun 1, 2026
Merged

create_ssl_context: raise when verify is a str and cert is provided#990
Kludex merged 8 commits into
pydantic:mainfrom
mbeijen:ssl-context-tuple

Conversation

@mbeijen
Copy link
Copy Markdown
Contributor

@mbeijen mbeijen commented May 25, 2026 •
edited
Loading

Raise when verify is str and we have a cert. This has been deprecated for a while and also it does not work!

Closes #989

scollinson and others added 2 commits May 25, 2026 11:22
@scollinson @mbeijen
create_ssl_context: Don't skip ctx.load_cert_chain call when verify i…
d8c4b30 
…s a str
@mbeijen @claude
Add regression test for create_ssl_context verify=str with cert
e8a93b3 
Covers the case where `verify` is a string path and `cert` is a tuple:
the cert chain must still be loaded. Before the previous commit, the
function returned early and silently dropped the client cert.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 25, 2026 •
edited
Loading

Merging this PR will not alter performance

✅ 15 untouched benchmarks
⏩ 7 skipped benchmarks1

Comparing mbeijen:ssl-context-tuple (a75b1cd) with main (9866dec)

Open in CodSpeed

Footnotes

  1. 7 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@mbeijen mbeijen mentioned this pull request May 25, 2026
Closed
Kludex
Kludex reviewed May 25, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@Kludex Kludex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this is the right path, I think it's slightly better to raise an error and point the user to pass ctx themselves on verify, since verify as str is deprecated.

Comment thread tests/httpx2/test_config.py Outdated
@mbeijen @claude
create_ssl_context: raise on verify=<str> combined with cert=...
ce761fc 
The combination of two deprecated parameters silently dropped the
client cert chain. Rather than fixing the silent skip, refuse the
combination and direct users to build an `ssl.SSLContext` themselves,
matching the path the existing deprecation warnings already suggest.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@mbeijen mbeijen force-pushed the ssl-context-tuple branch from f5aa1c1 to ce761fc Compare May 25, 2026 12:32
@mbeijen mbeijen changed the title create_ssl_context: Don't skip ctx.load_cert_chain call when verify is a str create_ssl_context: raise when verify is a str and cert is provided May 25, 2026
@mbeijen
Copy link
Copy Markdown
Contributor Author

mbeijen commented May 25, 2026

I'm not sure this is the right path, I think it's slightly better to raise an error and point the user to pass ctx themselves on verify, since verify as str is deprecated.

Sure! I've changed the PR to now raise ;-)

cubic-dev-ai[bot]
cubic-dev-ai Bot reviewed Jun 1, 2026
View reviewed changes
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 2 files

Re-trigger cubic

Kludex
Kludex approved these changes Jun 1, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@Kludex Kludex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is the solution for the wrong problem.

The problem here is that Client/AsyncClient don't point to the user that cert= is deprecated at that point (so type checkers can point out), and also, that we emit DeprecationWarning - see https://sethmlarson.dev/deprecations-via-warnings-dont-work-for-python-libraries for more details.

All that said, let's merge this for the time being.

@Kludex Kludex merged commit 91af735 into pydantic:main Jun 1, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cubic-dev-ai cubic-dev-ai[bot] cubic-dev-ai[bot] left review comments

@Kludex Kludex Kludex approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Post Handshake Auth works with an SSLContext but not a cert tuple

3 participants

@mbeijen @Kludex @scollinson