Skip to content

Commit

Permalink
Merge pull request #43 from grafana/fix-file-handing-windows
Browse files Browse the repository at this point in the history
Fix file handing windows
  • Loading branch information
pablochacin authored Oct 2, 2024
2 parents b500cff + af077ba commit 8dff64e
Show file tree
Hide file tree
Showing 5 changed files with 364 additions and 33 deletions.
1 change: 1 addition & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ issues:
- lll
- gosec
- noctx
- gochecknoglobals
- path: js\/modules\/k6\/http\/.*_test\.go
linters:
# k6/http module's tests are quite complex because they often have several nested levels.
Expand Down
15 changes: 12 additions & 3 deletions pkg/cache/file/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"strings"

"github.com/grafana/k6build/pkg/cache"
"github.com/grafana/k6build/pkg/util"
)

// Cache a Cache backed by a file system
Expand Down Expand Up @@ -65,6 +66,7 @@ func (f *Cache) Store(_ context.Context, id string, content io.Reader) (cache.Ob
if err != nil {
return cache.Object{}, fmt.Errorf("%w: %w", cache.ErrCreatingObject, err)
}
defer objectFile.Close() //nolint:errcheck

// write content to object file and copy to buffer to calculate checksum
// TODO: optimize memory by copying content in blocks
Expand All @@ -83,10 +85,11 @@ func (f *Cache) Store(_ context.Context, id string, content io.Reader) (cache.Ob
return cache.Object{}, fmt.Errorf("%w: %w", cache.ErrCreatingObject, err)
}

objectURL, _ := util.URLFromFilePath(objectFile.Name())
return cache.Object{
ID: id,
Checksum: checksum,
URL: fmt.Sprintf("file://%s", objectFile.Name()),
URL: objectURL.String(),
}, nil
}

Expand All @@ -108,10 +111,11 @@ func (f *Cache) Get(_ context.Context, id string) (cache.Object, error) {
return cache.Object{}, fmt.Errorf("%w: %w", cache.ErrAccessingObject, err)
}

objectURL, _ := util.URLFromFilePath(filepath.Join(objectDir, "data"))
return cache.Object{
ID: id,
Checksum: string(checksum),
URL: fmt.Sprintf("file://%s", filepath.Join(objectDir, "data")),
URL: objectURL.String(),
}, nil
}

Expand All @@ -124,8 +128,13 @@ func (f *Cache) Download(_ context.Context, object cache.Object) (io.ReadCloser,

switch url.Scheme {
case "file":
objectPath, err := util.URLToFilePath(url)
if err != nil {
return nil, err
}

// prevent malicious path
objectPath, err := f.sanitizePath(url.Path)
objectPath, err = f.sanitizePath(objectPath)
if err != nil {
return nil, err
}
Expand Down
54 changes: 24 additions & 30 deletions pkg/cache/file/file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@ import (
"fmt"
"net/url"
"os"
"path/filepath"
"testing"

"github.com/grafana/k6build/pkg/cache"
"github.com/grafana/k6build/pkg/util"
)

type object struct {
Expand Down Expand Up @@ -103,12 +105,13 @@ func TestFileCacheStoreObject(t *testing.T) {
return
}

fileURL, err := url.Parse(obj.URL)
objectURL, _ := url.Parse(obj.URL)
filePath, err := util.URLToFilePath(objectURL)
if err != nil {
t.Fatalf("invalid url %v", err)
}

content, err := os.ReadFile(fileURL.Path)
content, err := os.ReadFile(filePath)
if err != nil {
t.Fatalf("reading object url %v", err)
}
Expand Down Expand Up @@ -170,12 +173,13 @@ func TestFileCacheRetrieval(t *testing.T) {
return
}

fileURL, err := url.Parse(obj.URL)
objectURL, _ := url.Parse(obj.URL)
fileUPath, err := util.URLToFilePath(objectURL)
if err != nil {
t.Fatalf("invalid url %v", err)
}

data, err := os.ReadFile(fileURL.Path)
data, err := os.ReadFile(fileUPath)
if err != nil {
t.Fatalf("reading object url %v", err)
}
Expand All @@ -193,42 +197,28 @@ func TestFileCacheRetrieval(t *testing.T) {

testCases := []struct {
title string
object cache.Object
id string
url string
expected []byte
expectErr error
}{
{
title: "download existing object",
object: cache.Object{
ID: "object",
URL: fmt.Sprintf("file://%s/object/data", cacheDir),
},
title: "download existing object",
id: "object",
url: filepath.Join(cacheDir, "/object/data"),
expected: []byte("content"),
expectErr: nil,
},
{
title: "download non existing object",
object: cache.Object{
ID: "object",
URL: fmt.Sprintf("file://%s/another_object/data", cacheDir),
},
expectErr: cache.ErrObjectNotFound,
},
{
title: "download malformed url",
object: cache.Object{
ID: "object",
URL: fmt.Sprintf("file://%s/invalid&path/data", cacheDir),
},
// FIXME: this should be an ErrInvalidURL
title: "download non existing object",
id: "object",
url: filepath.Join(cacheDir, "/another_object/data"),
expectErr: cache.ErrObjectNotFound,
},
{
title: "download malicious url",
object: cache.Object{
ID: "object",
URL: fmt.Sprintf("file://%s/../../data", cacheDir),
},
title: "download malicious url",
id: "object",
url: filepath.Join(cacheDir, "/../../data"),
expectErr: cache.ErrInvalidURL,
},
}
Expand All @@ -237,7 +227,9 @@ func TestFileCacheRetrieval(t *testing.T) {
t.Run(tc.title, func(t *testing.T) {
t.Parallel()

content, err := fileCache.Download(context.TODO(), tc.object)
objectURL, _ := util.URLFromFilePath(tc.url)
object := cache.Object{ID: tc.id, URL: objectURL.String()}
content, err := fileCache.Download(context.TODO(), object)
if !errors.Is(err, tc.expectErr) {
t.Fatalf("expected %v got %v", tc.expectErr, err)
}
Expand All @@ -247,6 +239,8 @@ func TestFileCacheRetrieval(t *testing.T) {
return
}

defer content.Close() //nolint:errcheck

data := bytes.Buffer{}
_, err = data.ReadFrom(content)
if err != nil {
Expand Down
137 changes: 137 additions & 0 deletions pkg/util/url.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
// Package util implements utility functions
package util

import (
"errors"
"fmt"
"net/url"
"path/filepath"
"runtime"
"strings"
)

// Adapted from https://go-review.googlesource.com/c/vuln/+/438175/7/internal/web/url.go

// URLFromFilePath converts the given absolute path to a URL.
func URLFromFilePath(path string) (*url.URL, error) {
if !filepath.IsAbs(path) {
return nil, fmt.Errorf("path is not absolute %q", path)
}

// If path has a Windows volume name, convert the volume to a host and prefix
// per https://blogs.msdn.microsoft.com/ie/2006/12/06/file-uris-in-windows/.
if vol := filepath.VolumeName(path); vol != "" {
if strings.HasPrefix(vol, `\\`) {
path = filepath.ToSlash(path[2:])
i := strings.IndexByte(path, '/')

if i < 0 {
// A degenerate case.
// \\host.example.com (without a share name)
// becomes
// file://host.example.com/
return &url.URL{
Scheme: "file",
Host: path,
Path: "/",
}, nil
}

// \\host.example.com\Share\path\to\file
// becomes
// file://host.example.com/Share/path/to/file
return &url.URL{
Scheme: "file",
Host: path[:i],
Path: filepath.ToSlash(path[i:]),
}, nil
}

// C:\path\to\file
// becomes
// file:///C:/path/to/file
return &url.URL{
Scheme: "file",
Path: "/" + filepath.ToSlash(path),
}, nil
}

// /path/to/file
// becomes
// file:///path/to/file
return &url.URL{
Scheme: "file",
Path: filepath.ToSlash(path),
}, nil
}

// URLToFilePath converts a file-scheme url to a file path.
func URLToFilePath(u *url.URL) (string, error) {
if u.Scheme != "file" {
return "", errors.New("non-file URL")
}

checkAbs := func(path string) (string, error) {
if !filepath.IsAbs(path) {
return "", fmt.Errorf("path is not absolute %q", path)
}
return path, nil
}

if u.Path == "" {
if u.Host != "" || u.Opaque == "" {
return "", errors.New("file URL missing path")
}
return checkAbs(filepath.FromSlash(u.Opaque))
}

path, err := convertFileURLPath(u.Host, u.Path)
if err != nil {
return path, err
}
return checkAbs(path)
}

func convertFileURLPath(host, path string) (string, error) {
if runtime.GOOS == "windows" {
return convertFileURLPathWindows(host, path)
}
switch host {
case "", "localhost":
default:
return "", errors.New("file URL specifies non-local host")
}
return filepath.FromSlash(path), nil
}

func convertFileURLPathWindows(host, path string) (string, error) {
if len(path) == 0 || path[0] != '/' {
return "", fmt.Errorf("path is not absolute %q", path)
}

path = filepath.FromSlash(path)

// We interpret Windows file URLs per the description in
// https://blogs.msdn.microsoft.com/ie/2006/12/06/file-uris-in-windows/.

// The host part of a file URL (if any) is the UNC volume name,
// but RFC 8089 reserves the authority "localhost" for the local machine.
if host != "" && host != "localhost" {
// A common "legacy" format omits the leading slash before a drive letter,
// encoding the drive letter as the host instead of part of the path.
// (See https://blogs.msdn.microsoft.com/freeassociations/2005/05/19/the-bizarre-and-unhappy-story-of-file-urls/.)
// We do not support that format, but we should at least emit a more
// helpful error message for it.
if filepath.VolumeName(host) != "" {
return "", errors.New("file URL encodes volume in host field: too few slashes?")
}
return `\\` + host + path, nil
}

// If host is empty, path must contain an initial slash followed by a
// drive letter and path. Remove the slash and verify that the path is valid.
if vol := filepath.VolumeName(path[1:]); vol == "" || strings.HasPrefix(vol, `\\`) {
return "", errors.New("file URL missing drive letter")
}
return path[1:], nil
}
Loading

0 comments on commit 8dff64e

Please sign in to comment.