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/crypto/ssh: support service names in Client.Dial with net.LookupPort #63771

Open
HouseK opened this issue Oct 27, 2023 · 5 comments
Open

x/crypto/ssh: support service names in Client.Dial with net.LookupPort #63771

HouseK opened this issue Oct 27, 2023 · 5 comments
Labels
FeatureRequest NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Proposal
Milestone
Unreleased

Comments

@HouseK
Copy link

HouseK commented Oct 27, 2023 •
edited

Add support for service names additional to port numbers to ssh.Client.Dial.

PR: https://go.dev/cl/443775 (golang/crypto#235)

net.Dial already supports this same, this proposal adds that same feature to ssh.Client.Dial.

Currently ssh.Client.Dial uses strconv.ParseUint to convert a port number as string into the int type it needs.
The proposal is to use net.LookupPort instead.

net.LookupPort is purpose build for interpreting strings as ports and has the additional feature that it also supports service names.
@gopherbot gopherbot added this to the Proposal milestone Oct 27, 2023
@drakkan
Copy link
Member

drakkan commented Oct 27, 2023

This addition seems reasonable to me

cc @golang/proposal-review

@seankhliao seankhliao changed the title proposal: affected/package: crypto/ssh x/crypto/ssh: support service names in Client.Dial with net.LookupPort Oct 28, 2023
@seankhliao
Copy link
Member

With no api change I don't think this requires a proposal

@seankhliao seankhliao modified the milestones: Proposal, Unreleased Oct 28, 2023
@seankhliao seankhliao added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. FeatureRequest and removed Proposal labels Oct 28, 2023
@hanwen
Copy link
Contributor

hanwen commented Oct 30, 2023

I have already approved the change, and didn't think this needs a proposal at all.

@dr2chase
Copy link
Contributor

dr2chase commented Nov 1, 2023

It needs the proposal tag, I was about to ok a CL after the "is this what it claims" Google-required supply-chain check, and realized that it might reach the "proposal" bar, pinged the rest of the release team, and consensus was "yes".

@HouseK
Copy link
Author

HouseK commented Nov 7, 2023

Thanks for all the input! What would be the next step now?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
FeatureRequest NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Proposal
Projects
Proposals
Status: Incoming
Milestone
Unreleased
Development

No branches or pull requests
6 participants
@hanwen @drakkan @dr2chase @gopherbot @HouseK @seankhliao