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

x/net/websocket: add Config.DialContext #57953

Closed
Cyberax opened this issue Jan 22, 2023 · 9 comments
Closed

x/net/websocket: add Config.DialContext #57953

Cyberax opened this issue Jan 22, 2023 · 9 comments

Comments

@Cyberax
Copy link

Cyberax commented Jan 22, 2023

What version of Go are you using (go version)?

$ go version
go version go1.19.5 darwin/arm64

Does this issue reproduce with the latest release?

Yes.

What did you do?

websocket.DialConfig function can block indefinitely. While it's possible to specify the TLS connection timeout using the config parameter, the function calls the NewClient method that does read/write operations, and they can be blocked indefinitely.

Also because the code is so old, there is no way to provide context.Context to support cancellation.

What did you expect to see?

A way to not block the connection.

Cyberax added a commit to Cyberax/golang-x-net that referenced this issue Jan 22, 2023
Right now there is no way to pass context.Context to websocket.Dial. In
addition, this method can block indefinitely in the `NewClient` call.

Fixes golang/go#57953
@seankhliao seankhliao changed the title golang.org/x/net/websocket: websocket.DialConfig can block indefinitely x/net/websocket: websocket.DialConfig can block indefinitely Jan 22, 2023
@gopherbot gopherbot added this to the Unreleased milestone Jan 22, 2023
@gopherbot
Copy link

Change https://go.dev/cl/463097 mentions this issue: Add support for dialing with context to websockets

@seankhliao seankhliao changed the title x/net/websocket: websocket.DialConfig can block indefinitely proposal: x/net/websocket: add Config.DialContext Jan 28, 2023
@seankhliao seankhliao modified the milestones: Unreleased, Proposal Jan 28, 2023
@danielh2942
Copy link

Hey what's the status of this, kinda curious

@ianlancetaylor
Copy link
Contributor

It's on the list for the proposal review committee.

@rsc
Copy link
Contributor

rsc commented May 24, 2023

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 May 31, 2023

Have all concerns with this proposal been addressed?

@rsc
Copy link
Contributor

rsc commented Jun 7, 2023

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

@rsc
Copy link
Contributor

rsc commented Jun 14, 2023

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: x/net/websocket: add Config.DialContext x/net/websocket: add Config.DialContext Jun 14, 2023
@rsc rsc modified the milestones: Proposal, Backlog Jun 14, 2023
@Cyberax
Copy link
Author

Cyberax commented Jun 15, 2023

Cyberax added a commit to Cyberax/golang-x-net that referenced this issue Nov 26, 2023
Right now there is no way to pass context.Context to websocket.Dial. In
addition, this method can block indefinitely in the `NewClient` call.

Fixes golang/go#57953
@dmitshur dmitshur modified the milestones: Backlog, Unreleased Jan 6, 2024
Cyberax added a commit to Cyberax/golang-x-net that referenced this issue Jan 12, 2024
Right now there is no way to pass context.Context to websocket.Dial. In
addition, this method can block indefinitely in the `NewClient` call.

Fixes golang/go#57953
@gopherbot
Copy link

Change https://go.dev/cl/568198 mentions this issue: websocket: re-add documentation for DialConfig

gopherbot pushed a commit to golang/net that referenced this issue Mar 4, 2024
The comment of the DialConfig function was dropped during CL 463097.
There doesn't seem to be a good reason to do that, so bring it back.

For golang/go#57953.

Change-Id: I3e458b7d18cdab95763f003da5a644c8287b54ad
Reviewed-on: https://go-review.googlesource.com/c/net/+/568198
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Accepted
Development

Successfully merging a pull request may close this issue.

7 participants