Skip to content
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

Jump to bottom

net/http: add Transport.GetProxyConnectHeader #41048

Closed
bradfitz opened this issue Aug 26, 2020 · 7 comments
Closed

net/http: add Transport.GetProxyConnectHeader #41048

bradfitz opened this issue Aug 26, 2020 · 7 comments
Labels
FrozenDueToAge Proposal Proposal-Accepted
Projects
Proposals (old)
Milestone
Backlog

Comments

@bradfitz
Copy link
Contributor

bradfitz commented Aug 26, 2020 •
edited

net/http.Transport currently has:

https://golang.org/pkg/net/http/#Transport.ProxyConnectHeader

    // ProxyConnectHeader optionally specifies headers to send to
    // proxies during CONNECT requests.
    ProxyConnectHeader Header // Go 1.8

... but it's static for the life of the Transport.

Some authentication modes like NTLM/Kerberos/Negotiate require a dynamic header value, which currently requires making a new Transport for each request going through a proxy. That's heavy, as the Transport is the unit of connection reuse.

Proposal:

    // GetProxyConnectHeader is called to return headers to send to
    // proxuURL during a CONNECT request to the target ip:port. If it returns an error, the Transport
    // RoundTrip fails with that error. It can return (nil, nil) to not add headers.
    // If GetProxyConnectHeader is non-nil, ProxyConnectHeader is ignored.
    GetProxyConnectHeader func(ctx context.Context, proxyURL *url.URL, target string) (Header, error)

    // ProxyConnectHeader optionally specifies headers to send to
    // proxies during CONNECT requests. For dynamic use, see GetProxyConnectHeader.
    ProxyConnectHeader Header // Go 1.8
@gopherbot gopherbot added this to the Proposal milestone Aug 26, 2020
bradfitz added a commit to tailscale/go that referenced this issue Aug 27, 2020
@bradfitz
net/http: add Transport.GetProxyConnectHeader hook
6db6825 
Updates golang#41048
@bradfitz
Copy link
Contributor Author

bradfitz commented Aug 27, 2020 •
edited

Potential implementation: tailscale@6db6825 (licensed+CLA'd for use by Go)

(Lacking tests for now.)

@rsc rsc added this to Active in Proposals (old) Sep 2, 2020
@rsc rsc mentioned this issue Sep 2, 2020
Open
@rsc
Copy link
Contributor

rsc commented Sep 16, 2020

The Get is a bit non-standard but it does follow our pattern of having GetFoo func() T override Foo T in a struct when we realize too late that Foo T might not be initializable from the start. (For example, Request.GetBody vs Request.Body.)

Does anyone object to this?

@rsc
Copy link
Contributor

rsc commented Sep 23, 2020

Based on the discussion above, this seems like a likely accept.

@rsc rsc moved this from Active to Likely Accept in Proposals (old) Sep 23, 2020
@networkimprov
Copy link

cc @fraenkel @odeke-em for any input...

@rsc
Copy link
Contributor

rsc commented Sep 30, 2020

No change in consensus, so accepted.

@rsc rsc moved this from Likely Accept to Accepted in Proposals (old) Sep 30, 2020
@rsc rsc modified the milestones: Proposal, Backlog Sep 30, 2020
@rsc rsc changed the title proposal: net/http: add Transport.GetProxyConnectHeader net/http: add Transport.GetProxyConnectHeader Sep 30, 2020
@gopherbot
Copy link

Change https://golang.org/cl/259917 mentions this issue: net/http: add Transport.GetProxyConnectHeader

@dmitshur dmitshur mentioned this issue Dec 28, 2020
Closed
@gopherbot
Copy link

Change https://golang.org/cl/285594 mentions this issue: doc/go1.16: mention net/http.Transport.GetProxyConnectHeader

gopherbot pushed a commit that referenced this issue Jan 22, 2021
@ianlancetaylor
doc/go1.16: mention net/http.Transport.GetProxyConnectHeader
cf10e69 
For #40700
For #41048

Change-Id: Ida6bcaaf5edaa2bba9ba2b8e02ec9959481f8302
Reviewed-on: https://go-review.googlesource.com/c/go/+/285594
Trust: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@golang golang locked and limited conversation to collaborators Jan 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge Proposal Proposal-Accepted
Projects
No open projects
Proposals (old)
Accepted
Milestone
Backlog
Development

No branches or pull requests
4 participants
@bradfitz @rsc @networkimprov @gopherbot