From d8201c406fca4e8684ede2cbbcf8b9f058b94691 Mon Sep 17 00:00:00 2001 From: Felix Becker Date: Thu, 27 Jan 2022 01:42:18 +0100 Subject: [PATCH] Implement SchemeHostAndPortMatches in urlutil extract duplicate logic (target for refactoring) from the pkg/action/install.go and pkg/getter/httpgetter.go to the internal/urlutil/urlutil.go. Keep the logic as is and guard it by unit tests. Signed-off-by: Felix Becker --- internal/urlutil/urlutil.go | 8 ++++++++ internal/urlutil/urlutil_test.go | 32 +++++++++++++++++++++++++++++++- pkg/action/install.go | 6 ++---- pkg/getter/httpgetter.go | 5 +---- 4 files changed, 42 insertions(+), 9 deletions(-) diff --git a/internal/urlutil/urlutil.go b/internal/urlutil/urlutil.go index a8cf7398c..db482510d 100644 --- a/internal/urlutil/urlutil.go +++ b/internal/urlutil/urlutil.go @@ -71,3 +71,11 @@ func ExtractHostname(addr string) (string, error) { } return u.Hostname(), nil } + +// SchemeHostAndPortMatches returns if the scheme, port and hostname of the given url matches +func SchemeHostAndPortMatches(u1, u2 *url.URL) bool { + // Host on URL (returned from url.Parse) contains the port if present. + // This check ensures credentials are not passed between different + // services on different ports. + return u1.Scheme == u2.Scheme && u1.Host == u2.Host +} diff --git a/internal/urlutil/urlutil_test.go b/internal/urlutil/urlutil_test.go index 82acc40fe..56e4376b6 100644 --- a/internal/urlutil/urlutil_test.go +++ b/internal/urlutil/urlutil_test.go @@ -16,7 +16,10 @@ limitations under the License. package urlutil -import "testing" +import ( + "net/url" + "testing" +) func TestURLJoin(t *testing.T) { tests := []struct { @@ -39,6 +42,33 @@ func TestURLJoin(t *testing.T) { } } +func TestSchemeHostAndPortMatches(t *testing.T) { + for _, tt := range []struct { + a, b string + match bool + }{ + {"http://example.com", "http://example.com", true}, + {"https://example.com", "https://example.com", true}, + {"http://example.com", "https://example.com", false}, + {"https://example.com", "http://example.com", false}, + {"http://example.com:80", "http://example.com:80", true}, + {"https://example.com:443", "https://example.com:443", true}, + {"http://example.com:1234", "http://example.com:5678", false}, + {"https://example.com:1234", "https://example.com:5678", false}, + // The following lines are subject of change, currently only there + // to ensure that the existing logic works as expected and the + // upcoming fix / improvement works as wanted + {"http://example.com:80", "http://example.com", false}, + {"https://example.com:443", "https://example.com", false}, + } { + u1, _ := url.Parse(tt.a) + u2, _ := url.Parse(tt.b) + if tt.match != SchemeHostAndPortMatches(u1, u2) { + t.Errorf("Expected %q==%q to be %t", tt.a, tt.b, tt.match) + } + } +} + func TestEqual(t *testing.T) { for _, tt := range []struct { a, b string diff --git a/pkg/action/install.go b/pkg/action/install.go index 32be904b4..dc76126c7 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -38,6 +38,7 @@ import ( "k8s.io/cli-runtime/pkg/resource" "sigs.k8s.io/yaml" + "helm.sh/helm/v3/internal/urlutil" "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/chartutil" "helm.sh/helm/v3/pkg/cli" @@ -730,10 +731,7 @@ func (c *ChartPathOptions) LocateChart(name string, settings *cli.EnvSettings) ( return "", err } - // Host on URL (returned from url.Parse) contains the port if present. - // This check ensures credentials are not passed between different - // services on different ports. - if c.PassCredentialsAll || (u1.Scheme == u2.Scheme && u1.Host == u2.Host) { + if c.PassCredentialsAll || urlutil.SchemeHostAndPortMatches(u1, u2) { dl.Options = append(dl.Options, getter.WithBasicAuth(c.Username, c.Password)) } else { dl.Options = append(dl.Options, getter.WithBasicAuth("", "")) diff --git a/pkg/getter/httpgetter.go b/pkg/getter/httpgetter.go index 454eb6eb6..f167d89b6 100644 --- a/pkg/getter/httpgetter.go +++ b/pkg/getter/httpgetter.go @@ -66,10 +66,7 @@ func (g *HTTPGetter) get(href string) (*bytes.Buffer, error) { return nil, errors.Wrap(err, "Unable to parse URL getting from") } - // Host on URL (returned from url.Parse) contains the port if present. - // This check ensures credentials are not passed between different - // services on different ports. - if g.opts.passCredentialsAll || (u1.Scheme == u2.Scheme && u1.Host == u2.Host) { + if g.opts.passCredentialsAll || urlutil.SchemeHostAndPortMatches(u1, u2) { if g.opts.username != "" && g.opts.password != "" { req.SetBasicAuth(g.opts.username, g.opts.password) }