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

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

Closed
Cyberax opened this issue Jan 22, 2023 · 9 comments · Fixed by Unity-and-wireless-communications/net#6
Closed

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

Cyberax opened this issue Jan 22, 2023 · 9 comments · Fixed by Unity-and-wireless-communications/net#6
Labels
Proposal Proposal-Accepted
Milestone
Unreleased

Comments

@Cyberax
Copy link

Cyberax commented Jan 22, 2023 •
edited
Loading

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
@Cyberax
Add support for dialing with context to websockets
d208c33 
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
@Cyberax Cyberax mentioned this issue Jan 22, 2023
Closed
@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
Contributor

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
@ianlancetaylor ianlancetaylor moved this to Incoming in Proposals Feb 1, 2023
@notdanhan
Copy link

Hey what's the status of this, kinda curious

@ianlancetaylor
Copy link
Member

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 rsc moved this from Incoming to Active in Proposals May 24, 2023
@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 rsc moved this from Active to Likely Accept in Proposals Jun 7, 2023
@rsc rsc moved this from Likely Accept to Accepted in Proposals Jun 14, 2023
@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

I have a CL that implements this: https://go-review.googlesource.com/c/net/+/463097#message-31956dbc4b1568dbd323fd81a18b6a1fcb016c9e

Cyberax added a commit to Cyberax/golang-x-net that referenced this issue Nov 26, 2023
@Cyberax
Add support for dialing with context to websockets
3771fe4 
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
@Cyberax
Add support for dialing with context to websockets
5e8c3a7 
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
Contributor

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
@dmitshur @gopherbot
websocket: re-add documentation for DialConfig
c289c7a 
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 <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Damien Neil <[email protected]>
Auto-Submit: Dmitri Shuralyov <[email protected]>
@meslubi2021 meslubi2021 mentioned this issue Dec 13, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Proposal Proposal-Accepted
Projects
Proposals
Status: Accepted
Milestone
Unreleased
7 participants
@rsc @Cyberax @dmitshur @ianlancetaylor @gopherbot @seankhliao @notdanhan