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.
Python: pin the validated address for OpenAPI plugin requests #14371
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Uh oh!
There was an error while loading. Please reload this page.
Python: pin the validated address for OpenAPI plugin requests #14371
Changes from all commits
5e537c2File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pinning is disabled here whenever any
http/https/allproxy variable is set,but the built-in
httpx.AsyncClient(timeout=timeout)usestrust_env=Trueandhonors
NO_PROXY. For a target matched byNO_PROXY(e.g.HTTPS_PROXYset withNO_PROXY=internal.exampleorNO_PROXY=*), httpx bypasses the proxy andconnects directly, re-resolving the hostname at connect time — yet
pinned_addresseswas already cleared, so the request goes out by name. Thisreopens the exact DNS check-time/use-time (rebinding) gap the change exists to
close, for precisely the hosts that take the vulnerable direct-connect path
(confirmed: with
HTTPS_PROXYset and the host inNO_PROXY,getproxies()reports the proxy while
httpx.get_environment_proxies()returns a bypass entryand
proxy_bypass(host)is True). The comment's rationale — that a proxy resolvesthe name itself — does not hold for the
NO_PROXYset. Decide whether to disablepinning based on whether the proxy actually applies to this request's host, e.g.
disable only when a proxy is configured and
urllib.request.proxy_bypass(host)isfalse (mirroring httpx's own
NO_PROXY-aware routing), rather than on the merepresence of a global proxy variable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fallback loop (openapi_runner.py:246-252) pins each attempt to one literal IP and iterates strictly sequentially, so httpx/anyio no longer races the vetted A/AAAA addresses (each
connect_tcpsees a single address, losing happy-eyeballs); combined with the built-in client being built ashttpx.AsyncClient(timeout=timeout)wheretimeoutisNoneby default (lines 204, 239), there is no application-level connect timeout, so an attempt against a blackholed/silently-dropping first vetted address stalls (until the OS TCP connect timeout, not truly "forever") before the loop advances to a reachable vetted address — a latency regression versus the prior by-hostname happy-eyeballs path. A safe fix must preserve the single-delivery invariant pinned bytest_run_operation_does_not_retry_a_request_that_may_already_have_been_delivered(onlyConnectError/ConnectTimeoutmay fall through, and the request must never be delivered to more than one address), so racing full requests concurrently is not acceptable; instead bound each attempt's connect phase (e.g., a per-attempt connect timeout inside the loop) so a stalled address cannot delay fallback.Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.