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.

Sign up for GitHub

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.OnProxyConnectResponse #54299

Closed
bradfitz opened this issue Aug 5, 2022 · 7 comments
Closed

net/http: add Transport.OnProxyConnectResponse #54299

bradfitz opened this issue Aug 5, 2022 · 7 comments
Labels
FrozenDueToAge Proposal Proposal-Accepted Proposal-FinalCommentPeriod
Milestone
Go1.20

Comments

@bradfitz
Copy link
Contributor

bradfitz commented Aug 5, 2022 •
edited
Loading

We've had variants of this addition in our tree for a number of releases now:

tailscale@e6895e8 (corp CLA; same license as Go)

It adds a Transport.OnProxyConnectResponse hook:

	// OnProxyConnectResponse is called when the Transport gets an HTTP response from
	// a proxy for a CONNECT request. It's called before the check for a 200 OK response.
	// If it returns an error, the request fails with that error.
	OnProxyConnectResponse func(ctx context.Context, proxyURL *url.URL, connectReq *Request, connectRes *Response) error

Otherwise there's no way to find out how a CONNECT request to a proxy failed. (HTTP 403, etc)

/cc @neild
@gopherbot gopherbot added this to the Proposal milestone Aug 5, 2022
@rsc
Copy link
Contributor

rsc commented Oct 6, 2022

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Oct 12, 2022

Does anyone object to adding this API?

@rsc
Copy link
Contributor

rsc commented Oct 20, 2022

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Oct 26, 2022

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

@rsc rsc changed the title proposal: net/http: add Transport.OnProxyConnectResponse net/http: add Transport.OnProxyConnectResponse Oct 26, 2022
@rsc rsc modified the milestones: Proposal, Backlog Oct 26, 2022
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/447216 mentions this issue: net/http: add Transport.OnProxyConnectResponse

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/449215 mentions this issue: api/next/54299: add missing newline

gopherbot pushed a commit that referenced this issue Nov 9, 2022
@gopherbot
api/next/54299: add missing newline
e70f74b 
Updates #54299.

Change-Id: I20a9191fa3c78810987ace69527d34091c4a42fb
Reviewed-on: https://go-review.googlesource.com/c/go/+/449215
Run-TryBot: Bryan Mills <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Auto-Submit: Bryan Mills <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/450515 mentions this issue: doc/go1.20: add release notes for net/http and net/http/httputil

gopherbot pushed a commit that referenced this issue Nov 15, 2022
@neild @gopherbot
doc/go1.20: add release notes for net/http and net/http/httputil
8cca427 
For #41773
For #41773
For #50465
For #51914
For #53002
For #53896
For #53960
For #54136
For #54299

Change-Id: I729d5eafc1940d5706f980882a08ece1f69bb42c
Reviewed-on: https://go-review.googlesource.com/c/go/+/450515
Auto-Submit: Damien Neil <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: Damien Neil <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
@prattmic prattmic mentioned this issue Dec 6, 2022
Closed
mihaip added a commit to tailscale/tailscale that referenced this issue Feb 4, 2023
@mihaip
net/tshttpproxy: more directly use Transport proxy CONNECT hooks
ef0ca42 
GetProxyConnectHeader (golang/go#41048) was upstreamed in Go 1.16 and
OnProxyConnectResponse (golang/go#54299) in Go 1.20, thus we no longer
need to guard their use by the tailscale_go build tag.

Updates #7123

Signed-off-by: Mihai Parparita <[email protected]>
@mihaip mihaip mentioned this issue Feb 4, 2023
Merged
mihaip added a commit to tailscale/tailscale that referenced this issue Feb 4, 2023
@mihaip
net/tshttpproxy: more directly use Transport proxy CONNECT hooks
b690818 
GetProxyConnectHeader (golang/go#41048) was upstreamed in Go 1.16 and
OnProxyConnectResponse (golang/go#54299) in Go 1.20, thus we no longer
need to guard their use by the tailscale_go build tag.

Updates #7123

Signed-off-by: Mihai Parparita <[email protected]>
tmm1 pushed a commit to fancybits/go that referenced this issue May 26, 2023
@cuiweixie @tmm1
net/http: add Transport.OnProxyConnectResponse
49b3c67 
Fixes golang#54299

Change-Id: I3a29527bde7ac71f3824e771982db4257234e9ef
Reviewed-on: https://go-review.googlesource.com/c/go/+/447216
Reviewed-by: Brad Fitzpatrick <[email protected]>
Run-TryBot: xie cui <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Bryan Mills <[email protected]>
Reviewed-by: Damien Neil <[email protected]>
(cherry picked from commit 2f4d5c3)
@dmitshur dmitshur modified the milestones: Backlog, Go1.20 Jun 4, 2023
@cmoresco-stripe cmoresco-stripe mentioned this issue Aug 2, 2023
Merged
@wm775825 wm775825 mentioned this issue Dec 19, 2023
Closed
@max-b max-b mentioned this issue Mar 28, 2024
Closed
@golang golang locked and limited conversation to collaborators Jun 3, 2024
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 Proposal-FinalCommentPeriod
Projects
None yet
Milestone
Go1.20
Development

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