diff --git a/internal/plugin/installer/http_installer_test.go b/internal/plugin/installer/http_installer_test.go index be40b1b90..16ff0e305 100644 --- a/internal/plugin/installer/http_installer_test.go +++ b/internal/plugin/installer/http_installer_test.go @@ -27,6 +27,7 @@ import ( "net/http/httptest" "os" "path/filepath" + "sort" "strings" "syscall" "testing" @@ -66,22 +67,37 @@ func TestStripName(t *testing.T) { } } -func mockArchiveServer() *httptest.Server { +func mockArchiveServer(extensionToContentType map[string]string) *httptest.Server { + // Extract and sort keys by length in descending order + extensions := make([]string, 0, len(extensionToContentType)) + for ext := range extensionToContentType { + extensions = append(extensions, ext) + } + sort.Slice(extensions, func(i, j int) bool { + return len(extensions[i]) > len(extensions[j]) + }) + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if !strings.HasSuffix(r.URL.Path, ".tar.gz") { - w.Header().Add("Content-Type", "text/html") - fmt.Fprintln(w, "broken") - return + for _, ext := range extensions { + contentType := extensionToContentType[ext] + if strings.HasSuffix(r.URL.Path, ext) { + w.Header().Add("Content-Type", contentType) + fmt.Fprintln(w, "test") + return + } } - w.Header().Add("Content-Type", "application/gzip") - fmt.Fprintln(w, "test") + + w.Header().Add("Content-Type", "text/html") + fmt.Fprintln(w, "broken") })) } func TestHTTPInstaller(t *testing.T) { ensure.HelmHome(t) - srv := mockArchiveServer() + srv := mockArchiveServer(map[string]string{ + ".tar.gz": "application/gzip", + }) defer srv.Close() source := srv.URL + "/plugins/fake-plugin-0.0.1.tar.gz" @@ -129,7 +145,9 @@ func TestHTTPInstaller(t *testing.T) { func TestHTTPInstallerNonExistentVersion(t *testing.T) { ensure.HelmHome(t) - srv := mockArchiveServer() + srv := mockArchiveServer(map[string]string{ + ".tar.gz": "application/gzip", + }) defer srv.Close() source := srv.URL + "/plugins/fake-plugin-0.0.1.tar.gz" @@ -161,7 +179,9 @@ func TestHTTPInstallerNonExistentVersion(t *testing.T) { } func TestHTTPInstallerUpdate(t *testing.T) { - srv := mockArchiveServer() + srv := mockArchiveServer(map[string]string{ + ".tar.gz": "application/gzip", + }) defer srv.Close() source := srv.URL + "/plugins/fake-plugin-0.0.1.tar.gz" ensure.HelmHome(t) diff --git a/internal/plugin/installer/installer.go b/internal/plugin/installer/installer.go index a6599c443..c4aeebc35 100644 --- a/internal/plugin/installer/installer.go +++ b/internal/plugin/installer/installer.go @@ -174,41 +174,51 @@ func isLocalReference(source string) bool { // It works by checking whether the source looks like a URL and, if it does, running a // HEAD operation to see if the remote resource is a file that we understand. func isRemoteHTTPArchive(source string) bool { - if strings.HasPrefix(source, "http://") || strings.HasPrefix(source, "https://") { - // First, check if the URL ends with a known archive suffix - // This is more reliable than content-type detection + if !isHTTPURL(source) { + return false + } + + contentType, err := getRemoteContentType(source) + if err != nil { + return false + } + + // Handle octet-stream specially by checking file extension + if contentType == "application/octet-stream" { for suffix := range Extractors { if strings.HasSuffix(source, suffix) { return true } } - // If no suffix match, try HEAD request to check content type - res, err := http.Head(source) - if err != nil { - // If we get an error at the network layer, we can't install it. So - // we return false. - return false - } - - // Next, we look for the content type or content disposition headers to see - // if they have matching extractors. - contentType := res.Header.Get("content-type") - foundSuffix, ok := mediaTypeToExtension(contentType) - if !ok { - // Media type not recognized - return false - } + return false + } - for suffix := range Extractors { - if strings.HasSuffix(foundSuffix, suffix) { - return true - } - } + // Check if we have an extractor for this media type + if suffix, ok := mediaTypeToExtension(contentType); ok { + _, hasExtractor := Extractors[suffix] + return hasExtractor } + return false } +// isHTTPURL checks if the source is an HTTP or HTTPS URL +func isHTTPURL(source string) bool { + return strings.HasPrefix(source, "http://") || strings.HasPrefix(source, "https://") +} + +// getRemoteContentType performs a HEAD request and returns the content-type +func getRemoteContentType(url string) (string, error) { + res, err := http.Head(url) + if err != nil { + return "", err + } + defer res.Body.Close() + + return res.Header.Get("content-type"), nil +} + // isPlugin checks if the directory contains a plugin.yaml file. func isPlugin(dirname string) bool { _, err := os.Stat(filepath.Join(dirname, plugin.PluginFileName)) diff --git a/internal/plugin/installer/installer_test.go b/internal/plugin/installer/installer_test.go index dcd76fe9c..9714e72ce 100644 --- a/internal/plugin/installer/installer_test.go +++ b/internal/plugin/installer/installer_test.go @@ -18,9 +18,13 @@ package installer import "testing" func TestIsRemoteHTTPArchive(t *testing.T) { - srv := mockArchiveServer() + srv := mockArchiveServer(map[string]string{ + ".tar.gz": "application/gzip", + ".binary.tar.gz": "application/octet-stream", + }) defer srv.Close() source := srv.URL + "/plugins/fake-plugin-0.0.1.tar.gz" + binarySource := srv.URL + "/plugins/fake-plugin-0.0.1.binary.tar.gz" if isRemoteHTTPArchive("/not/a/URL") { t.Errorf("Expected non-URL to return false") @@ -44,4 +48,8 @@ func TestIsRemoteHTTPArchive(t *testing.T) { if isRemoteHTTPArchive(source + "-not-an-extension") { t.Error("Expected media type match to fail") } + + if !isRemoteHTTPArchive(binarySource) { + t.Errorf("Expected %q to be a valid archive URL", binarySource) + } }