From 55073c404c0d44b9b63d1dbc033e9f6e0a1f3d2f Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 23 Apr 2025 14:27:49 +0200 Subject: [PATCH 1/3] cli/connhelper/ssh: tweak error-message (capitalize SSH) Signed-off-by: Sebastiaan van Stijn --- cli/connhelper/ssh/ssh.go | 4 ++-- cli/connhelper/ssh/ssh_test.go | 16 ++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/cli/connhelper/ssh/ssh.go b/cli/connhelper/ssh/ssh.go index a2552b82b5..94a46d5348 100644 --- a/cli/connhelper/ssh/ssh.go +++ b/cli/connhelper/ssh/ssh.go @@ -17,11 +17,11 @@ func ParseURL(daemonURL string) (*Spec, error) { if errors.As(err, &urlErr) { 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) if err != nil { - return nil, fmt.Errorf("invalid ssh URL: %w", err) + return nil, fmt.Errorf("invalid SSH URL: %w", err) } return s, nil } diff --git a/cli/connhelper/ssh/ssh_test.go b/cli/connhelper/ssh/ssh_test.go index 198d5a0802..492408f116 100644 --- a/cli/connhelper/ssh/ssh_test.go +++ b/cli/connhelper/ssh/ssh_test.go @@ -87,41 +87,41 @@ func TestParseURL(t *testing.T) { { doc: "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", url: "no-scheme.example.com", - expectedError: "invalid ssh URL: no scheme provided", + expectedError: "invalid SSH URL: no scheme provided", }, { doc: "invalid URL with password", 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", 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", 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", url: "ssh://", - expectedError: "invalid ssh URL: hostname is empty", + expectedError: "invalid SSH URL: hostname is empty", }, { url: "ssh:///no-hostname", - expectedError: "invalid ssh URL: hostname is empty", + expectedError: "invalid SSH URL: hostname is empty", }, { doc: "invalid URL with unsupported scheme", url: "https://example.com", - expectedError: `invalid ssh URL: incorrect scheme: https`, + expectedError: `invalid SSH URL: incorrect scheme: https`, }, } for _, tc := range testCases { From 11b53dabc603400d3f6ef0f18095ec2c5bc37ae4 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 4 Feb 2025 12:04:56 +0100 Subject: [PATCH 2/3] cli/connhelper/ssh: add NewSpec utility This allows creating a spec from an existing url.URL Signed-off-by: Sebastiaan van Stijn --- cli/connhelper/ssh/ssh.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/cli/connhelper/ssh/ssh.go b/cli/connhelper/ssh/ssh.go index 94a46d5348..2c9d0d61b1 100644 --- a/cli/connhelper/ssh/ssh.go +++ b/cli/connhelper/ssh/ssh.go @@ -19,7 +19,14 @@ func ParseURL(daemonURL string) (*Spec, error) { } 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 { return nil, fmt.Errorf("invalid SSH URL: %w", err) } @@ -27,6 +34,9 @@ func ParseURL(daemonURL string) (*Spec, error) { } func newSpec(u *url.URL) (*Spec, error) { + if u == nil { + return nil, errors.New("URL is nil") + } if u.Scheme == "" { return nil, errors.New("no scheme provided") } From f105e964daed35507da07bd9db5bc31b118b402d Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 4 Feb 2025 12:14:06 +0100 Subject: [PATCH 3/3] cli/connhelper: don't parse URL twice This function was parsing the same URL twice; first to detect the scheme, then again (through ssh.ParseURL) to construct a ssh.Spec. Change the function to use the URL that's parsed, and use ssh.NewSpec instead. Signed-off-by: Sebastiaan van Stijn --- cli/connhelper/connhelper.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/connhelper/connhelper.go b/cli/connhelper/connhelper.go index 9e3347c84c..f4b9388599 100644 --- a/cli/connhelper/connhelper.go +++ b/cli/connhelper/connhelper.go @@ -41,7 +41,7 @@ func getConnectionHelper(daemonURL string, sshFlags []string) (*ConnectionHelper return nil, err } if u.Scheme == "ssh" { - sp, err := ssh.ParseURL(daemonURL) + sp, err := ssh.NewSpec(u) if err != nil { return nil, fmt.Errorf("ssh host connection is not valid: %w", err) }