Restore SNI coverage with a local TLS fixture - #1085
Closed
jmtdev0 wants to merge 2 commits into
Closed
Conversation
jmtdev0
marked this pull request as ready for review
September 6, 2026 18:34
Member
|
This PR introduces too much infra directly into this repo that is overkill for the purposes of this test. The preferred approach is to extend https://github.com/RequestsPHP/test-server as needed to allow for testing this specific scenario. I'll update the related issue and contributing guide to clarify the test setup. |
Member
|
Thanks for the PR, @jmtdev0 , but this is not the type of approach we want for this particular issue. |
Author
|
Understandable. Thanks for reviewing the PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request Type
This is a:
Context
The existing
testSNISupporttest relied on a public third-party host whose certificate is now the same with and without SNI. As a result, the test no longer verifies the behavior it was intended to cover and had been skipped.This change restores meaningful SNI coverage for issue #1077 without depending on a public endpoint or its current TLS configuration.
Detailed Description
localhostcertificate, and an untrusted fallback certificate during the test.localhost.verifynameoption variants through the current data provider.The helper uses Python 3 from the test environment and has no third-party Python dependency.
Quality assurance
Validation performed:
git diff --checkpassed.Fixes #1077
AI Disclaimer
This pull request was primarily developed with assistance from OpenAI Codex, an AI coding agent. For this PR, Codex analyzed issue #1077, inspected the relevant Requests transport tests, implemented the local TLS/SNI fixture, and ran the reported validation commands under the supervision of
jmtdev0.Human involvement in this PR was very low.
If you do not agree with the use of AI assistance or with the level of human involvement in this PR, please feel free to disregard it, close it, or request changes. I will fully respect that decision.