From 88a9f3dc3f618b7a92f42d22a7b2cdd99eeb5aed Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Mon, 18 Mar 2019 14:54:05 +0200 Subject: [PATCH 01/14] open() should not be able to make HTTP requests fix #963 --- js/bundle_test.go | 13 +++++++++++++ js/initcontext.go | 6 +++--- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/js/bundle_test.go b/js/bundle_test.go index 26f940ba3a8..81c2a88af51 100644 --- a/js/bundle_test.go +++ b/js/bundle_test.go @@ -404,6 +404,19 @@ func TestNewBundleFromArchive(t *testing.T) { assert.Equal(t, "hi!", v2.Export()) } +func TestOpenNotOpeningURLs(t *testing.T) { + fs := afero.NewMemMapFs() + src := &lib.SourceData{ + Filename: "/path/to/script.js", + Data: []byte(` + export let file = open("google.com"); + export default function() { }; + `), + } + _, err := NewBundle(src, fs, lib.RuntimeOptions{}) + assert.Error(t, err) +} + func TestBundleInstantiate(t *testing.T) { b, err := getSimpleBundle("/script.js", ` let val = true; diff --git a/js/initcontext.go b/js/initcontext.go index d5df7d500b1..ef10b8c99f4 100644 --- a/js/initcontext.go +++ b/js/initcontext.go @@ -168,12 +168,12 @@ func (i *InitContext) Open(name string, args ...string) (goja.Value, error) { filename := loader.Resolve(i.pwd, name) data, ok := i.files[filename] if !ok { - data_, err := loader.Load(i.fs, i.pwd, name) + var err error + 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" { From 0816fbf1cd4f77a5480b05218b410ed25cee640b Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Tue, 19 Mar 2019 12:21:06 +0200 Subject: [PATCH 02/14] open() now correctly handles files not starting with './' --- js/bundle_test.go | 104 +++++++++++++++++++++++++++++++++++++++++----- js/initcontext.go | 13 +++++- 2 files changed, 106 insertions(+), 11 deletions(-) diff --git a/js/bundle_test.go b/js/bundle_test.go index 81c2a88af51..47100d4bd7c 100644 --- a/js/bundle_test.go +++ b/js/bundle_test.go @@ -404,17 +404,101 @@ func TestNewBundleFromArchive(t *testing.T) { assert.Equal(t, "hi!", v2.Export()) } -func TestOpenNotOpeningURLs(t *testing.T) { - fs := afero.NewMemMapFs() - src := &lib.SourceData{ - Filename: "/path/to/script.js", - Data: []byte(` - export let file = open("google.com"); - export default function() { }; +func TestOpen(t *testing.T) { + t.Run("paths", func(t *testing.T) { + var testCases = [...]struct { + name string + openPath string + pwd string + isError bool + isArchiveError bool + }{ + { + name: "notOpeningUrls", + openPath: "github.com", + isError: true, + }, + { + 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/", + isError: true, + }, + { + name: "file is missing", + openPath: "/path/to/missing.txt", + isError: true, + }, + } + for _, tCase := range testCases { + tCase := tCase + + t.Run(tCase.name, func(t *testing.T) { + fs := afero.NewMemMapFs() + assert.NoError(t, fs.MkdirAll("/path/to", 0755)) + assert.NoError(t, afero.WriteFile(fs, "/path/to/file.txt", []byte(`hi`), 0644)) + src := &lib.SourceData{ + Filename: "/path/to/script.js", + Data: []byte(` + export let file = open("` + tCase.openPath + `"); + export default function() { return file }; `), - } - _, err := NewBundle(src, fs, lib.RuntimeOptions{}) - assert.Error(t, err) + } + sourceBundle, err := NewBundle(src, fs, lib.RuntimeOptions{}) + if tCase.isError { + assert.Error(t, err) + return + } + if !assert.NoError(t, err) { + return + } + sourceBundle.BaseInitContext.pwd = tCase.pwd + + arcBundle, err := NewBundleFromArchive(sourceBundle.makeArchive(), lib.RuntimeOptions{}) + if !assert.NoError(t, err) { + return + } + for source, b := range map[string]*Bundle{"source": sourceBundle, "archive": arcBundle} { + b := b + t.Run(source, func(t *testing.T) { + bi, err := b.Instantiate() + if !assert.NoError(t, err) { + return + } + v, err := bi.Default(goja.Undefined()) + if !assert.NoError(t, err) { + return + } + assert.Equal(t, "hi", v.Export()) + }) + } + }) + } + }) } func TestBundleInstantiate(t *testing.T) { diff --git a/js/initcontext.go b/js/initcontext.go index ef10b8c99f4..d614d17ec07 100644 --- a/js/initcontext.go +++ b/js/initcontext.go @@ -165,10 +165,21 @@ func (i *InitContext) compileImport(src, filename string) (*goja.Program, error) } func (i *InitContext) Open(name string, args ...string) (goja.Value, error) { - filename := loader.Resolve(i.pwd, name) + filename := name + if !filepath.IsAbs(name) { + filename = filepath.Join(i.pwd, name) + } + filename = filepath.ToSlash(filename) + data, ok := i.files[filename] if !ok { var err error + // 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 From dd81e88b01b47d9d06a751cfae1c7d2dcfa3a8eb Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Tue, 19 Mar 2019 12:46:11 +0200 Subject: [PATCH 03/14] Implement afero.Stat for normalizedFS - fixing a test --- lib/archive.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/archive.go b/lib/archive.go index b08ba25cf2c..187a1cb75f6 100644 --- a/lib/archive.go +++ 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". From d5758285fd1a0a1428742fab03e407a502d330e6 Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Tue, 19 Mar 2019 13:14:48 +0200 Subject: [PATCH 04/14] Fix shadowing of err, thanks golangci-lint --- js/initcontext.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/js/initcontext.go b/js/initcontext.go index d614d17ec07..c37641f873e 100644 --- a/js/initcontext.go +++ b/js/initcontext.go @@ -173,9 +173,13 @@ func (i *InitContext) Open(name string, args ...string) (goja.Value, error) { data, ok := i.files[filename] if !ok { - var err error + var ( + err error + isDir bool + ) + // Workaround for https://github.com/spf13/afero/issues/201 - if isDir, err := afero.IsDir(i.fs, filename); err != nil { + 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") From d64c8f1f35202a85a722e664e85ebe2e1402ce3b Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Wed, 20 Mar 2019 10:15:24 +0200 Subject: [PATCH 05/14] Fix open to work both in MemMapFS and OsFs on Windows --- js/bundle_test.go | 211 ++++++++++++++++++++++++++-------------------- js/initcontext.go | 4 +- 2 files changed, 123 insertions(+), 92 deletions(-) diff --git a/js/bundle_test.go b/js/bundle_test.go index 47100d4bd7c..db77384569b 100644 --- a/js/bundle_test.go +++ b/js/bundle_test.go @@ -23,6 +23,9 @@ package js import ( "crypto/tls" "fmt" + "io/ioutil" + "os" + "path/filepath" "testing" "time" @@ -31,6 +34,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" ) @@ -405,100 +409,127 @@ func TestNewBundleFromArchive(t *testing.T) { } func TestOpen(t *testing.T) { - t.Run("paths", func(t *testing.T) { - var testCases = [...]struct { - name string - openPath string - pwd string - isError bool - isArchiveError bool - }{ - { - name: "notOpeningUrls", - openPath: "github.com", - isError: true, - }, - { - 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/", - isError: true, - }, - { - name: "file is missing", - openPath: "/path/to/missing.txt", - isError: true, - }, - } - for _, tCase := range testCases { - tCase := tCase + 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, + }, + } + 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() { os.RemoveAll(prefix) } + }, + } - t.Run(tCase.name, func(t *testing.T) { - fs := afero.NewMemMapFs() - assert.NoError(t, fs.MkdirAll("/path/to", 0755)) - assert.NoError(t, afero.WriteFile(fs, "/path/to/file.txt", []byte(`hi`), 0644)) - src := &lib.SourceData{ - Filename: "/path/to/script.js", - Data: []byte(` - export let file = open("` + tCase.openPath + `"); + for name, fsInit := range fss { + fs, prefix, cleanUp := fsInit() + defer cleanUp() + fs = afero.NewReadOnlyFs(fs) + t.Run(name, func(t *testing.T) { + fs = afero.NewReadOnlyFs(fs) + for _, tCase := range testCases { + tCase := tCase + var openPath = tCase.openPath + // if fullpath prepend prefix + if openPath[0] == '/' { + openPath = filepath.Join(prefix, openPath) + } + + t.Run(tCase.name, func(t *testing.T) { + src := &lib.SourceData{ + Filename: filepath.Join(prefix, "/path/to/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 - } - if !assert.NoError(t, err) { - return - } - sourceBundle.BaseInitContext.pwd = tCase.pwd + } + sourceBundle, err := NewBundle(src, fs, lib.RuntimeOptions{}) + if tCase.isError { + assert.Error(t, err) + return + } + if !assert.NoError(t, err) { + return + } + sourceBundle.BaseInitContext.pwd = filepath.Join(prefix, tCase.pwd) - arcBundle, err := NewBundleFromArchive(sourceBundle.makeArchive(), lib.RuntimeOptions{}) - if !assert.NoError(t, err) { - return - } - for source, b := range map[string]*Bundle{"source": sourceBundle, "archive": arcBundle} { - b := b - t.Run(source, func(t *testing.T) { - bi, err := b.Instantiate() - if !assert.NoError(t, err) { - return - } - v, err := bi.Default(goja.Undefined()) - if !assert.NoError(t, err) { - return - } - assert.Equal(t, "hi", v.Export()) - }) - } - }) - } - }) + arcBundle, err := NewBundleFromArchive(sourceBundle.makeArchive(), lib.RuntimeOptions{}) + if !assert.NoError(t, err) { + return + } + for source, b := range map[string]*Bundle{"source": sourceBundle, "archive": arcBundle} { + b := b + t.Run(source, func(t *testing.T) { + bi, err := b.Instantiate() + if !assert.NoError(t, err) { + return + } + v, err := bi.Default(goja.Undefined()) + if !assert.NoError(t, err) { + return + } + assert.Equal(t, "hi", v.Export()) + }) + } + }) + } + }) + } } func TestBundleInstantiate(t *testing.T) { diff --git a/js/initcontext.go b/js/initcontext.go index c37641f873e..4b7acfa91aa 100644 --- a/js/initcontext.go +++ b/js/initcontext.go @@ -166,8 +166,8 @@ func (i *InitContext) compileImport(src, filename string) (*goja.Program, error) func (i *InitContext) Open(name string, args ...string) (goja.Value, error) { filename := name - if !filepath.IsAbs(name) { - filename = filepath.Join(i.pwd, name) + if filename[0] != '/' && filename[0] != filepath.Separator && !filepath.IsAbs(filename) { + filename = filepath.Join(i.pwd, filename) } filename = filepath.ToSlash(filename) From d1fe8e5242f420d2c14ba1b8c353c38b1737c91b Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Thu, 21 Mar 2019 17:55:08 +0200 Subject: [PATCH 06/14] Fix open() tests on windows and add some more windows specific test cases --- js/bundle_test.go | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/js/bundle_test.go b/js/bundle_test.go index db77384569b..32dd7cd9669 100644 --- a/js/bundle_test.go +++ b/js/bundle_test.go @@ -26,6 +26,8 @@ import ( "io/ioutil" "os" "path/filepath" + "runtime" + "strings" "testing" "time" @@ -484,13 +486,16 @@ func TestOpen(t *testing.T) { fs = afero.NewReadOnlyFs(fs) for _, tCase := range testCases { tCase := tCase - var openPath = tCase.openPath - // if fullpath prepend prefix - if openPath[0] == '/' { - openPath = filepath.Join(prefix, openPath) - } - t.Run(tCase.name, func(t *testing.T) { + var testFunc = func(t *testing.T) { + var openPath = tCase.openPath + // if fullpath prepend prefix + if openPath[0] == '/' || openPath[0] == '\\' { + openPath = filepath.Join(prefix, openPath) + } + if runtime.GOOS == "windows" { + openPath = strings.Replace(openPath, `\`, `\\`, -1) + } src := &lib.SourceData{ Filename: filepath.Join(prefix, "/path/to/script.js"), Data: []byte(` @@ -526,7 +531,15 @@ func TestOpen(t *testing.T) { 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) + } } }) } From 40188af48a029569f04fedb6f11e597467a13c74 Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Fri, 22 Mar 2019 16:23:40 +0200 Subject: [PATCH 07/14] Add some more tests for open() and fix setting pwd in tests --- js/bundle_test.go | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/js/bundle_test.go b/js/bundle_test.go index 32dd7cd9669..6c579266662 100644 --- a/js/bundle_test.go +++ b/js/bundle_test.go @@ -460,6 +460,21 @@ func TestOpen(t *testing.T) { 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", + }, } fss := map[string]func() (afero.Fs, string, func()){ "MemMapFS": func() (afero.Fs, string, func()) { @@ -496,8 +511,12 @@ func TestOpen(t *testing.T) { 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, "/path/to/script.js"), + Filename: filepath.Join(prefix, filepath.Join(pwd, "script.js")), Data: []byte(` export let file = open("` + openPath + `"); export default function() { return file }; @@ -511,7 +530,6 @@ func TestOpen(t *testing.T) { if !assert.NoError(t, err) { return } - sourceBundle.BaseInitContext.pwd = filepath.Join(prefix, tCase.pwd) arcBundle, err := NewBundleFromArchive(sourceBundle.makeArchive(), lib.RuntimeOptions{}) if !assert.NoError(t, err) { From 8cefca75d5fb8571ebcacc02258b8480f716224f Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Fri, 22 Mar 2019 16:27:41 +0200 Subject: [PATCH 08/14] open() test cleanup --- js/bundle_test.go | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/js/bundle_test.go b/js/bundle_test.go index 6c579266662..80b83a5ac13 100644 --- a/js/bundle_test.go +++ b/js/bundle_test.go @@ -498,7 +498,6 @@ func TestOpen(t *testing.T) { defer cleanUp() fs = afero.NewReadOnlyFs(fs) t.Run(name, func(t *testing.T) { - fs = afero.NewReadOnlyFs(fs) for _, tCase := range testCases { tCase := tCase @@ -527,25 +526,17 @@ func TestOpen(t *testing.T) { assert.Error(t, err) return } - if !assert.NoError(t, err) { - return - } + require.NoError(t, err) arcBundle, err := NewBundleFromArchive(sourceBundle.makeArchive(), lib.RuntimeOptions{}) - if !assert.NoError(t, err) { - return - } + for source, b := range map[string]*Bundle{"source": sourceBundle, "archive": arcBundle} { b := b t.Run(source, func(t *testing.T) { bi, err := b.Instantiate() - if !assert.NoError(t, err) { - return - } + require.NoError(t, err) v, err := bi.Default(goja.Undefined()) - if !assert.NoError(t, err) { - return - } + require.NoError(t, err) assert.Equal(t, "hi", v.Export()) }) } From 2b644deb35418b6b784f70777c1fb7f68b032d95 Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Fri, 22 Mar 2019 16:28:02 +0200 Subject: [PATCH 09/14] Better error message when open is used with a directory --- js/initcontext.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/initcontext.go b/js/initcontext.go index 4b7acfa91aa..06a79c2f330 100644 --- a/js/initcontext.go +++ b/js/initcontext.go @@ -182,7 +182,7 @@ func (i *InitContext) Open(name string, args ...string) (goja.Value, error) { 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") + return nil, errors.New("open() can't be used with directories") } data, err = afero.ReadFile(i.fs, filename) if err != nil { From a5c376600bdce31fbd19c4259827d2a64f0039a3 Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Fri, 22 Mar 2019 16:28:38 +0200 Subject: [PATCH 10/14] open() optimization ;) --- js/initcontext.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/js/initcontext.go b/js/initcontext.go index 06a79c2f330..aa9afde46e3 100644 --- a/js/initcontext.go +++ b/js/initcontext.go @@ -164,8 +164,7 @@ 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 := name +func (i *InitContext) Open(filename string, args ...string) (goja.Value, error) { if filename[0] != '/' && filename[0] != filepath.Separator && !filepath.IsAbs(filename) { filename = filepath.Join(i.pwd, filename) } From fb44c330d943d745f47e90e852913759e618c9a5 Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Mon, 25 Mar 2019 12:13:38 +0200 Subject: [PATCH 11/14] fix golangci-lint issues --- js/bundle_test.go | 4 +++- js/initcontext.go | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/js/bundle_test.go b/js/bundle_test.go index 80b83a5ac13..224c0177e23 100644 --- a/js/bundle_test.go +++ b/js/bundle_test.go @@ -489,7 +489,7 @@ func TestOpen(t *testing.T) { 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() { os.RemoveAll(prefix) } + return fs, prefix, func() { require.NoError(t, os.RemoveAll(prefix)) } }, } @@ -530,6 +530,8 @@ func TestOpen(t *testing.T) { 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) { diff --git a/js/initcontext.go b/js/initcontext.go index aa9afde46e3..db77bbb4d22 100644 --- a/js/initcontext.go +++ b/js/initcontext.go @@ -164,6 +164,7 @@ func (i *InitContext) compileImport(src, filename string) (*goja.Program, error) return pgm, err } +// 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[0] != '/' && filename[0] != filepath.Separator && !filepath.IsAbs(filename) { filename = filepath.Join(i.pwd, filename) From a1e35ae1063476bcda6e03dae18b0f98bd5add7c Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Mon, 25 Mar 2019 12:16:24 +0200 Subject: [PATCH 12/14] Add a comment explaining why not only filepath.IsAbs is used --- js/initcontext.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/js/initcontext.go b/js/initcontext.go index db77bbb4d22..77f1cb0deaa 100644 --- a/js/initcontext.go +++ b/js/initcontext.go @@ -166,7 +166,11 @@ func (i *InitContext) compileImport(src, filename string) (*goja.Program, error) // 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[0] != '/' && filename[0] != filepath.Separator && !filepath.IsAbs(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) From 42b7e7ab3c270dce71d922530f341823ef35cd0c Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Tue, 26 Mar 2019 09:59:19 +0200 Subject: [PATCH 13/14] Don't panic on open() with an empty string --- js/bundle_test.go | 8 +++++++- js/initcontext.go | 4 ++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/js/bundle_test.go b/js/bundle_test.go index 224c0177e23..603afda382b 100644 --- a/js/bundle_test.go +++ b/js/bundle_test.go @@ -475,6 +475,12 @@ func TestOpen(t *testing.T) { 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()) { @@ -504,7 +510,7 @@ func TestOpen(t *testing.T) { var testFunc = func(t *testing.T) { var openPath = tCase.openPath // if fullpath prepend prefix - if openPath[0] == '/' || openPath[0] == '\\' { + if openPath != "" && (openPath[0] == '/' || openPath[0] == '\\') { openPath = filepath.Join(prefix, openPath) } if runtime.GOOS == "windows" { diff --git a/js/initcontext.go b/js/initcontext.go index 77f1cb0deaa..1dcf6d402f1 100644 --- a/js/initcontext.go +++ b/js/initcontext.go @@ -166,6 +166,10 @@ func (i *InitContext) compileImport(src, filename string) (*goja.Program, error) // 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 From 5e80aa46fc163b2558143277ba6bd3994efe678d Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Tue, 26 Mar 2019 11:01:29 +0200 Subject: [PATCH 14/14] Add release notes for #965 --- release notes/upcoming.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/release notes/upcoming.md b/release notes/upcoming.md index 879269443dc..65f5d56e162 100644 --- a/release notes/upcoming.md +++ b/release notes/upcoming.md @@ -10,4 +10,8 @@ Description of feature. ## Bugs fixed! -* Category: description of bug. (#PR) \ No newline at end of file +* 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