Merge pull request #6034 from thaJeztah/connhelper_cleanups_step2

cli/connhelper/ssh: add NewSpec utility to prevent parsing URL twice
This commit is contained in:
Sebastiaan van Stijn 2025-04-23 16:00:07 +02:00 committed by GitHub
commit aadd7879c9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 22 additions and 12 deletions

View File

@ -41,7 +41,7 @@ func getConnectionHelper(daemonURL string, sshFlags []string) (*ConnectionHelper
return nil, err return nil, err
} }
if u.Scheme == "ssh" { if u.Scheme == "ssh" {
sp, err := ssh.ParseURL(daemonURL) sp, err := ssh.NewSpec(u)
if err != nil { if err != nil {
return nil, fmt.Errorf("ssh host connection is not valid: %w", err) return nil, fmt.Errorf("ssh host connection is not valid: %w", err)
} }

View File

@ -17,16 +17,26 @@ func ParseURL(daemonURL string) (*Spec, error) {
if errors.As(err, &urlErr) { if errors.As(err, &urlErr) {
err = urlErr.Unwrap() err = urlErr.Unwrap()
} }
return nil, fmt.Errorf("invalid ssh URL: %w", err) return nil, fmt.Errorf("invalid SSH URL: %w", err)
} }
s, err := newSpec(u) return NewSpec(u)
}
// NewSpec creates a [Spec] from the given ssh URL's properties. It returns
// an error if the URL is using the wrong scheme, contains fragments,
// query-parameters, or contains a password.
func NewSpec(sshURL *url.URL) (*Spec, error) {
s, err := newSpec(sshURL)
if err != nil { if err != nil {
return nil, fmt.Errorf("invalid ssh URL: %w", err) return nil, fmt.Errorf("invalid SSH URL: %w", err)
} }
return s, nil return s, nil
} }
func newSpec(u *url.URL) (*Spec, error) { func newSpec(u *url.URL) (*Spec, error) {
if u == nil {
return nil, errors.New("URL is nil")
}
if u.Scheme == "" { if u.Scheme == "" {
return nil, errors.New("no scheme provided") return nil, errors.New("no scheme provided")
} }

View File

@ -87,41 +87,41 @@ func TestParseURL(t *testing.T) {
{ {
doc: "malformed URL", doc: "malformed URL",
url: "malformed %%url", url: "malformed %%url",
expectedError: `invalid ssh URL: invalid URL escape "%%u"`, expectedError: `invalid SSH URL: invalid URL escape "%%u"`,
}, },
{ {
doc: "URL missing scheme", doc: "URL missing scheme",
url: "no-scheme.example.com", url: "no-scheme.example.com",
expectedError: "invalid ssh URL: no scheme provided", expectedError: "invalid SSH URL: no scheme provided",
}, },
{ {
doc: "invalid URL with password", doc: "invalid URL with password",
url: "ssh://me:passw0rd@example.com", url: "ssh://me:passw0rd@example.com",
expectedError: "invalid ssh URL: plain-text password is not supported", expectedError: "invalid SSH URL: plain-text password is not supported",
}, },
{ {
doc: "invalid URL with query parameter", doc: "invalid URL with query parameter",
url: "ssh://example.com?foo=bar&bar=baz", url: "ssh://example.com?foo=bar&bar=baz",
expectedError: `invalid ssh URL: query parameters are not allowed: "foo=bar&bar=baz"`, expectedError: `invalid SSH URL: query parameters are not allowed: "foo=bar&bar=baz"`,
}, },
{ {
doc: "invalid URL with fragment", doc: "invalid URL with fragment",
url: "ssh://example.com#bar", url: "ssh://example.com#bar",
expectedError: `invalid ssh URL: fragments are not allowed: "bar"`, expectedError: `invalid SSH URL: fragments are not allowed: "bar"`,
}, },
{ {
doc: "invalid URL without hostname", doc: "invalid URL without hostname",
url: "ssh://", url: "ssh://",
expectedError: "invalid ssh URL: hostname is empty", expectedError: "invalid SSH URL: hostname is empty",
}, },
{ {
url: "ssh:///no-hostname", url: "ssh:///no-hostname",
expectedError: "invalid ssh URL: hostname is empty", expectedError: "invalid SSH URL: hostname is empty",
}, },
{ {
doc: "invalid URL with unsupported scheme", doc: "invalid URL with unsupported scheme",
url: "https://example.com", url: "https://example.com",
expectedError: `invalid ssh URL: incorrect scheme: https`, expectedError: `invalid SSH URL: incorrect scheme: https`,
}, },
} }
for _, tc := range testCases { for _, tc := range testCases {