oauthex: build defaultDiscoveryTransport lazily instead of at import (#1278)
Fixes #1276 ## What's happening `oauthex` builds `defaultDiscoveryTransport` as a package-level var, so it runs at import time. That path calls `proxyConfigured`, which invokes `http.DefaultTransport`'s proxy func (`http.ProxyFromEnvironment`). net/http reads the proxy env vars only once and caches the result, so this import-time call freezes the proxy config before `main()` can set `HTTPS_PROXY`. New in v1.8.0, which is why it's a regression — just importing `.../mcp` (which pulls in `oauthex`) triggers it, even with no use of the SDK. ## Fix Wrap the setup in `sync.OnceValue` so it runs on first use instead of at import. Still built once and reused. ## Testing (removed) - Ran the issue's repro against this branch via a local `replace`: prints `proxy=http://127.0.0.1:9` now, `proxy=<nil>` before. - Added a regression test in `oauthex`. - `go test ./...` passes. Co-authored-by: Guglielmo Colombo <guglielmoc@google.com>
This commit is contained in:
parent
b58aa8d8d8
commit
3b917b466c
1 changed files with 11 additions and 2 deletions
|
|
@ -16,6 +16,7 @@ import (
|
||||||
"net/netip"
|
"net/netip"
|
||||||
"net/url"
|
"net/url"
|
||||||
"strings"
|
"strings"
|
||||||
|
"sync"
|
||||||
"syscall"
|
"syscall"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
|
|
@ -25,7 +26,15 @@ import (
|
||||||
|
|
||||||
const maxDiscoveryRedirects = 10
|
const maxDiscoveryRedirects = 10
|
||||||
|
|
||||||
var defaultDiscoveryTransport = newDiscoveryTransport(http.DefaultTransport)
|
// Build this on first use, not when the package loads. Building it checks the
|
||||||
|
// proxy via http.ProxyFromEnvironment, and net/http only reads the proxy
|
||||||
|
// environment once and then remembers it for the whole run. If that happened
|
||||||
|
// at import time, it would lock in the proxy setting before the app had a
|
||||||
|
// chance to set HTTPS_PROXY. sync.OnceValue waits until first use and still
|
||||||
|
// builds the transport just once. See #1276.
|
||||||
|
var defaultDiscoveryTransport = sync.OnceValue(func() http.RoundTripper {
|
||||||
|
return newDiscoveryTransport(http.DefaultTransport)
|
||||||
|
})
|
||||||
|
|
||||||
type httpStatusError struct {
|
type httpStatusError struct {
|
||||||
StatusCode int
|
StatusCode int
|
||||||
|
|
@ -103,7 +112,7 @@ func checkHTTPSOrLoopback(addr string) error {
|
||||||
}
|
}
|
||||||
|
|
||||||
func newDiscoveryClient(c *http.Client) *http.Client {
|
func newDiscoveryClient(c *http.Client) *http.Client {
|
||||||
transport := defaultDiscoveryTransport
|
transport := defaultDiscoveryTransport()
|
||||||
if c != nil && c.Transport != nil {
|
if c != nil && c.Transport != nil {
|
||||||
// a caller that has taken over dialing or TLS opts out of the checks
|
// a caller that has taken over dialing or TLS opts out of the checks
|
||||||
if t, ok := c.Transport.(*http.Transport); ok && t.DialContext == nil && t.DialTLSContext == nil {
|
if t, ok := c.Transport.(*http.Transport); ok && t.DialContext == nil && t.DialTLSContext == nil {
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue