Skip to content

Commit

Permalink
fix(helm): improve URL comparison logic
Browse files Browse the repository at this point in the history
Normalize URLs before comparing them. This deviates slightly from the
URL spec, but in order to accomodate the predominant use pattern for
Helm. Specifically, './', '../', and '/' are all "interpreted" to be
filepath-like.

Closes #1588
  • Loading branch information
technosophos committed Nov 30, 2016
1 parent 8fd3f09 commit fa462a6
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 0 deletions.
9 changes: 9 additions & 0 deletions cmd/helm/downloader/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -278,13 +278,22 @@ func (m *Manager) parallelRepoUpdate(repos []*repo.Entry) {
func urlsAreEqual(a, b string) bool {
au, err := url.Parse(a)
if err != nil {
a = filepath.Clean(a)
b = filepath.Clean(b)
// If urls are paths, return true only if they are an exact match
return a == b
}
bu, err := url.Parse(b)
if err != nil {
return false
}

for _, u := range []*url.URL{au, bu} {
if u.Path == "" {
u.Path = "/"
}
u.Path = filepath.Clean(u.Path)
}
return au.String() == bu.String()
}

Expand Down
24 changes: 24 additions & 0 deletions cmd/helm/downloader/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,3 +84,27 @@ func TestFindChartURL(t *testing.T) {
}

}

func TestUrlsAreEqual(t *testing.T) {
for _, tt := range []struct {
a, b string
match bool
}{
{"http://example.com", "http://example.com", true},
{"http://example.com", "http://another.example.com", false},
{"https://example.com", "https://example.com", true},
{"http://example.com/", "http://example.com", true},
{"https://example.com", "http://example.com", false},
{"http://example.com/foo", "http://example.com/foo/", true},
{"http://example.com/foo//", "http://example.com/foo/", true},
{"http://example.com/./foo/", "http://example.com/foo/", true},
{"http://example.com/bar/../foo/", "http://example.com/foo/", true},
{"/foo", "/foo", true},
{"/foo", "/foo/", true},
{"/foo/.", "/foo/", true},
} {
if tt.match != urlsAreEqual(tt.a, tt.b) {
t.Errorf("Expected %q==%q to be %t", tt.a, tt.b, tt.match)
}
}
}

0 comments on commit fa462a6

Please sign in to comment.