refactor store download method (#76)

* refactor store download method

Signed-off-by: Pablo Chacin <[email protected]>

pkg/store/downloader/downloader.go

@@ -0,0 +1,82 @@
+// Package downloader implements utility functions for downloading objects from a store
+package downloader
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"io"
+	"net/http"
+	"net/url"
+	"os"
+	"path/filepath"
+
+	"github.com/grafana/k6build"
+	"github.com/grafana/k6build/pkg/store"
+	"github.com/grafana/k6build/pkg/util"
+)
+
+// Download returns the content of the object
+func Download(ctx context.Context, object store.Object) (io.ReadCloser, error) {
+	url, err := url.Parse(object.URL)
+	if err != nil {
+		return nil, k6build.NewWrappedError(store.ErrAccessingObject, err)
+	}
+
+	switch url.Scheme {
+	case "file":
+		objectPath, err := util.URLToFilePath(url)
+		if err != nil {
+			return nil, err
+		}
+
+		// prevent malicious path
+		objectPath, err = sanitizePath(objectPath)
+		if err != nil {
+			return nil, err
+		}
+
+		objectFile, err := os.Open(objectPath) //nolint:gosec // path is sanitized
+		if err != nil {
+			// FIXME: is the path has invalid characters, still will return ErrNotExists
+			if errors.Is(err, os.ErrNotExist) {
+				return nil, store.ErrObjectNotFound
+			}
+			return nil, k6build.NewWrappedError(store.ErrAccessingObject, err)
+		}
+
+		return objectFile, nil
+	case "http", "https":
+		req, err := http.NewRequestWithContext(ctx, http.MethodGet, object.URL, nil)
+		if err != nil {
+			return nil, k6build.NewWrappedError(store.ErrAccessingObject, err)
+		}
+
+		resp, err := http.DefaultClient.Do(req)
+		if err != nil {
+			return nil, k6build.NewWrappedError(store.ErrAccessingObject, err)
+		}
+
+		if resp.StatusCode == http.StatusNotFound {
+			return nil, store.ErrObjectNotFound
+		}
+
+		if resp.StatusCode != http.StatusOK {
+			return nil, k6build.NewWrappedError(store.ErrAccessingObject, fmt.Errorf("HTTP response: %s", resp.Status))
+		}
+
+		return resp.Body, nil
+	default:
+		return nil, fmt.Errorf("%w unsupported schema: %s", store.ErrInvalidURL, url.Scheme)
+	}
+}
+
+func sanitizePath(path string) (string, error) {
+	path = filepath.Clean(path)
+
+	if !filepath.IsAbs(path) {
+		return "", fmt.Errorf("%w : invalid path %s", store.ErrInvalidURL, path)
+	}
+
+	return path, nil
+}

pkg/store/downloader/downloader_test.go

@@ -0,0 +1,124 @@
+package downloader
+
+import (
+	"bytes"
+	"context"
+	"errors"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/grafana/k6build/pkg/store"
+	"github.com/grafana/k6build/pkg/util"
+)
+
+func fileURL(dir string, path string) string {
+	url, err := util.URLFromFilePath(filepath.Join(dir, path))
+	if err != nil {
+		panic(err)
+	}
+	return url.String()
+}
+
+func httpURL(srv *httptest.Server, path string) string {
+	return srv.URL + "/" + path
+}
+
+func TestDownload(t *testing.T) {
+	t.Parallel()
+
+	storeDir := t.TempDir()
+
+	objects := []struct {
+		id      string
+		content []byte
+	}{
+		{
+			id:      "object",
+			content: []byte("content"),
+		},
+	}
+
+	for _, o := range objects {
+		if err := os.WriteFile(filepath.Join(storeDir, o.id), o.content, 0o600); err != nil {
+			t.Fatalf("test setup %v", err)
+		}
+	}
+
+	srv := httptest.NewServer(http.FileServer(http.Dir(storeDir)))
+	t.Cleanup(srv.Close)
+
+	testCases := []struct {
+		title     string
+		id        string
+		url       string
+		expected  []byte
+		expectErr error
+	}{
+		{
+			title:     "download file url",
+			id:        "object",
+			url:       fileURL(storeDir, "object"),
+			expected:  []byte("content"),
+			expectErr: nil,
+		},
+		{
+			title:     "download non existing file url",
+			id:        "object",
+			url:       fileURL(storeDir, "another_object"),
+			expectErr: store.ErrObjectNotFound,
+		},
+		//  FIXME: can't check url is outside object store's directory
+		// {
+		// 	title:     "download malicious file url",
+		// 	id:        "object",
+		// 	url:       fileURL(storeDir, "/../../object"),
+		// 	expectErr: store.ErrInvalidURL,
+		// },
+		{
+			title:     "download http url",
+			id:        "object",
+			url:       httpURL(srv, "object"),
+			expected:  []byte("content"),
+			expectErr: nil,
+		},
+		{
+			title:     "download non existing http url",
+			id:        "object",
+			url:       httpURL(srv, "another-object"),
+			expectErr: store.ErrObjectNotFound,
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		t.Run(tc.title, func(t *testing.T) {
+			t.Parallel()
+
+			object := store.Object{ID: tc.id, URL: tc.url}
+			content, err := Download(context.TODO(), object)
+			if !errors.Is(err, tc.expectErr) {
+				t.Fatalf("expected %v got %v", tc.expectErr, err)
+			}
+
+			// if expected error, don't check returned object
+			if tc.expectErr != nil {
+				return
+			}
+
+			defer content.Close() //nolint:errcheck
+
+			data := bytes.Buffer{}
+			_, err = data.ReadFrom(content)
+			if err != nil {
+				t.Fatalf("reading content: %v", err)
+			}
+
+			if !bytes.Equal(data.Bytes(), tc.expected) {
+				t.Fatalf("expected %v got %v", tc.expected, data)
+			}
+		})
+	}
+}

pkg/store/file/file.go

@@ -8,7 +8,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"net/url"
 	"os"
 	"path/filepath"
 	"strings"
@@ -129,51 +128,6 @@ func (f *Store) Get(_ context.Context, id string) (store.Object, error) {
 	}, nil
 }
 
-// Download returns the content of the object given its url
-func (f *Store) Download(_ context.Context, object store.Object) (io.ReadCloser, error) {
-	url, err := url.Parse(object.URL)
-	if err != nil {
-		return nil, k6build.NewWrappedError(store.ErrAccessingObject, err)
-	}
-
-	switch url.Scheme {
-	case "file":
-		objectPath, err := util.URLToFilePath(url)
-		if err != nil {
-			return nil, err
-		}
-
-		// prevent malicious path
-		objectPath, err = f.sanitizePath(objectPath)
-		if err != nil {
-			return nil, err
-		}
-
-		objectFile, err := os.Open(objectPath) //nolint:gosec // path is sanitized
-		if err != nil {
-			// FIXME: is the path has invalid characters, still will return ErrNotExists
-			if errors.Is(err, os.ErrNotExist) {
-				return nil, store.ErrObjectNotFound
-			}
-			return nil, k6build.NewWrappedError(store.ErrAccessingObject, err)
-		}
-
-		return objectFile, nil
-	default:
-		return nil, fmt.Errorf("%w unsupported schema: %s", store.ErrInvalidURL, url.Scheme)
-	}
-}
-
-func (f *Store) sanitizePath(path string) (string, error) {
-	path = filepath.Clean(path)
-
-	if !filepath.IsAbs(path) || !strings.HasPrefix(path, f.dir) {
-		return "", fmt.Errorf("%w : invalid path %s", store.ErrInvalidURL, path)
-	}
-
-	return path, nil
-}
-
 // lockObject obtains a mutex used to prevent concurrent builds of the same artifact and
 // returns a function that will unlock the mutex associated to the given id in the object store.
 // The lock is also removed from the map. Subsequent calls will get another lock on the same