Fixes for open() fix #963 (#965)

Fixes include: 
  - don't panic with an empty filename (`""`)
  - don't make HTTP requests (#963)
  - correctly open simple filenames like `"file.json"` and paths such as `"relative/path/to.txt"` as relative (to the current working directory) paths; previously they had to start with a dot (i.e. `"./relative/path/to.txt"`) for that to happen
  - windows: work with paths starting with `/` or `\` as absolute from the current drive
  - return error when `open()`ing a directory

js/bundle_test.go

@@ -23,6 +23,11 @@ package js
 import (
 	"crypto/tls"
 	"fmt"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"runtime"
+	"strings"
 	"testing"
 	"time"
 
@@ -31,6 +36,7 @@ import (
 	"github.com/loadimpact/k6/lib/types"
 	"github.com/spf13/afero"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"gopkg.in/guregu/null.v3"
 )
 
@@ -404,6 +410,158 @@ func TestNewBundleFromArchive(t *testing.T) {
 	assert.Equal(t, "hi!", v2.Export())
 }
 
+func TestOpen(t *testing.T) {
+	var testCases = [...]struct {
+		name           string
+		openPath       string
+		pwd            string
+		isError        bool
+		isArchiveError bool
+	}{
+		{
+			name:     "notOpeningUrls",
+			openPath: "github.com",
+			isError:  true,
+			pwd:      "/path/to",
+		},
+		{
+			name:     "simple",
+			openPath: "file.txt",
+			pwd:      "/path/to",
+		},
+		{
+			name:     "simple with dot",
+			openPath: "./file.txt",
+			pwd:      "/path/to",
+		},
+		{
+			name:     "simple with two dots",
+			openPath: "../to/file.txt",
+			pwd:      "/path/not",
+		},
+		{
+			name:     "fullpath",
+			openPath: "/path/to/file.txt",
+			pwd:      "/path/to",
+		},
+		{
+			name:     "fullpath2",
+			openPath: "/path/to/file.txt",
+			pwd:      "/path",
+		},
+		{
+			name:     "file is dir",
+			openPath: "/path/to/",
+			pwd:      "/path/to",
+			isError:  true,
+		},
+		{
+			name:     "file is missing",
+			openPath: "/path/to/missing.txt",
+			isError:  true,
+		},
+		{
+			name:     "relative1",
+			openPath: "to/file.txt",
+			pwd:      "/path",
+		},
+		{
+			name:     "relative2",
+			openPath: "./path/to/file.txt",
+			pwd:      "/",
+		},
+		{
+			name:     "relative wonky",
+			openPath: "../path/to/file.txt",
+			pwd:      "/path",
+		},
+		{
+			name:     "empty open doesn't panic",
+			openPath: "",
+			pwd:      "/path",
+			isError:  true,
+		},
+	}
+	fss := map[string]func() (afero.Fs, string, func()){
+		"MemMapFS": func() (afero.Fs, string, func()) {
+			fs := afero.NewMemMapFs()
+			require.NoError(t, fs.MkdirAll("/path/to", 0755))
+			require.NoError(t, afero.WriteFile(fs, "/path/to/file.txt", []byte(`hi`), 0644))
+			return fs, "", func() {}
+		},
+		"OsFS": func() (afero.Fs, string, func()) {
+			prefix, err := ioutil.TempDir("", "k6_open_test")
+			require.NoError(t, err)
+			fs := afero.NewOsFs()
+			require.NoError(t, fs.MkdirAll(filepath.Join(prefix, "/path/to"), 0755))
+			require.NoError(t, afero.WriteFile(fs, filepath.Join(prefix, "/path/to/file.txt"), []byte(`hi`), 0644))
+			return fs, prefix, func() { require.NoError(t, os.RemoveAll(prefix)) }
+		},
+	}
+
+	for name, fsInit := range fss {
+		fs, prefix, cleanUp := fsInit()
+		defer cleanUp()
+		fs = afero.NewReadOnlyFs(fs)
+		t.Run(name, func(t *testing.T) {
+			for _, tCase := range testCases {
+				tCase := tCase
+
+				var testFunc = func(t *testing.T) {
+					var openPath = tCase.openPath
+					// if fullpath prepend prefix
+					if openPath != "" && (openPath[0] == '/' || openPath[0] == '\\') {
+						openPath = filepath.Join(prefix, openPath)
+					}
+					if runtime.GOOS == "windows" {
+						openPath = strings.Replace(openPath, `\`, `\\`, -1)
+					}
+					var pwd = tCase.pwd
+					if pwd == "" {
+						pwd = "/path/to/"
+					}
+					src := &lib.SourceData{
+						Filename: filepath.Join(prefix, filepath.Join(pwd, "script.js")),
+						Data: []byte(`
+			export let file = open("` + openPath + `");
+			export default function() { return file };
+		`),
+					}
+					sourceBundle, err := NewBundle(src, fs, lib.RuntimeOptions{})
+					if tCase.isError {
+						assert.Error(t, err)
+						return
+					}
+					require.NoError(t, err)
+
+					arcBundle, err := NewBundleFromArchive(sourceBundle.makeArchive(), lib.RuntimeOptions{})
+
+					require.NoError(t, err)
+
+					for source, b := range map[string]*Bundle{"source": sourceBundle, "archive": arcBundle} {
+						b := b
+						t.Run(source, func(t *testing.T) {
+							bi, err := b.Instantiate()
+							require.NoError(t, err)
+							v, err := bi.Default(goja.Undefined())
+							require.NoError(t, err)
+							assert.Equal(t, "hi", v.Export())
+						})
+					}
+				}
+
+				t.Run(tCase.name, testFunc)
+				if runtime.GOOS == "windows" {
+					// windowsify the testcase
+					tCase.openPath = strings.Replace(tCase.openPath, `/`, `\`, -1)
+					tCase.pwd = strings.Replace(tCase.pwd, `/`, `\`, -1)
+					t.Run(tCase.name+" with windows slash", testFunc)
+				}
+			}
+		})
+	}
+}
+
 func TestBundleInstantiate(t *testing.T) {
 	b, err := getSimpleBundle("/script.js", `
 		let val = true;

js/initcontext.go

@@ -164,16 +164,39 @@ func (i *InitContext) compileImport(src, filename string) (*goja.Program, error)
 	return pgm, err
 }
 
-func (i *InitContext) Open(name string, args ...string) (goja.Value, error) {
-	filename := loader.Resolve(i.pwd, name)
+// Open implements open() in the init context and will read and return the contents of a file
+func (i *InitContext) Open(filename string, args ...string) (goja.Value, error) {
+	if filename == "" {
+		return nil, errors.New("open() can't be used with an empty filename")
+	}
+
+	// Here IsAbs should be enough but unfortunately it doesn't handle absolute paths starting from
+	// the current drive on windows like `\users\noname\...`. Also it makes it more easy to test and
+	// will probably be need for archive execution under windows if always consider '/...' as an
+	// absolute path.
+	if filename[0] != '/' && filename[0] != '\\' && !filepath.IsAbs(filename) {
+		filename = filepath.Join(i.pwd, filename)
+	}
+	filename = filepath.ToSlash(filename)
+
 	data, ok := i.files[filename]
 	if !ok {
-		data_, err := loader.Load(i.fs, i.pwd, name)
+		var (
+			err   error
+			isDir bool
+		)
+
+		// Workaround for https://github.com/spf13/afero/issues/201
+		if isDir, err = afero.IsDir(i.fs, filename); err != nil {
+			return nil, err
+		} else if isDir {
+			return nil, errors.New("open() can't be used with directories")
+		}
+		data, err = afero.ReadFile(i.fs, filename)
 		if err != nil {
 			return nil, err
 		}
-		i.files[filename] = data_.Data
-		data = data_.Data
+		i.files[filename] = data
 	}
 
 	if len(args) > 0 && args[0] == "b" {

lib/archive.go

@@ -63,6 +63,10 @@ func (m *normalizedFS) OpenFile(name string, flag int, mode os.FileMode) (afero.
 	return m.Fs.OpenFile(NormalizeAndAnonymizePath(name), flag, mode)
 }
 
+func (m *normalizedFS) Stat(name string) (os.FileInfo, error) {
+	return m.Fs.Stat(NormalizeAndAnonymizePath(name))
+}
+
 // An Archive is a rollup of all resources and options needed to reproduce a test identically elsewhere.
 type Archive struct {
 	// The runner to use, eg. "js".

release notes/upcoming.md

@@ -10,4 +10,8 @@ Description of feature.
 
 ## Bugs fixed!
 
-* Category: description of bug. (#PR)
+* JS: Many fixes for `open()`: (#965)
+  - don't panic with an empty filename (`""`)
+  - don't make HTTP requests (#963)
+  - correctly open simple filenames like `"file.json"` and paths such as `"relative/path/to.txt"` as relative (to the current working directory) paths; previously they had to start with a dot (i.e. `"./relative/path/to.txt"`) for that to happen
+  - windows: work with paths starting with `/` or `\` as absolute from the current drive