Skip to content

Commit

Permalink
Contain main module within it's own scope as well (#2571)
Browse files Browse the repository at this point in the history
Previous to this the variables defined in the main script were
actually globally accessible as they were just part of the `globalThis`.

This obviously is terrible idea and this is more or less how nodejs
has done this as well, for a very long time.

This does mean that this is a breaking change, but arguably anyone
(ab)using the ability to access the main script variables globally is
likely, should've been aware that this is a bug.
  • Loading branch information
mstoykov authored Jul 18, 2022
1 parent 0127a50 commit e2316a2
Show file tree
Hide file tree
Showing 9 changed files with 197 additions and 86 deletions.
2 changes: 1 addition & 1 deletion core/engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -967,7 +967,7 @@ func TestVuInitException(t *testing.T) {

var exception errext.Exception
require.ErrorAs(t, err, &exception)
assert.Equal(t, "Error: oops in 2\n\tat file:///script.js:10:9(31)\n", err.Error())
assert.Equal(t, "Error: oops in 2\n\tat file:///script.js:10:9(30)\n\tat native\n", err.Error())

var errWithHint errext.HasHint
require.ErrorAs(t, err, &errWithHint)
Expand Down
62 changes: 46 additions & 16 deletions js/bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,9 @@ type BundleInstance struct {
// TODO: maybe just have a reference to the Bundle? or save and pass rtOpts?
env map[string]string

exports map[string]goja.Callable

exports map[string]goja.Callable
moduleVUImpl *moduleVUImpl
pgm programWithSource
}

// NewBundle creates a new bundle from a source file and a filesystem.
Expand All @@ -90,7 +90,7 @@ func NewBundle(
Strict: true,
SourceMapLoader: generateSourceMapLoader(logger, filesystems),
}
pgm, _, err := c.Compile(code, src.URL.String(), true)
pgm, _, err := c.Compile(code, src.URL.String(), false)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -142,7 +142,7 @@ func NewBundleFromArchive(
CompatibilityMode: compatMode,
SourceMapLoader: generateSourceMapLoader(logger, arc.Filesystems),
}
pgm, _, err := c.Compile(string(arc.Data), arc.FilenameURL.String(), true)
pgm, _, err := c.Compile(string(arc.Data), arc.FilenameURL.String(), false)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -208,7 +208,8 @@ func (b *Bundle) makeArchive() *lib.Archive {

// getExports validates and extracts exported objects
func (b *Bundle) getExports(logger logrus.FieldLogger, rt *goja.Runtime, options bool) error {
exportsV := rt.Get("exports")
pgm := b.BaseInitContext.programs[b.Filename.String()] // this is the main script and it's always present
exportsV := pgm.module.Get("exports")
if goja.IsNull(exportsV) || goja.IsUndefined(exportsV) {
return errors.New("exports must be an object")
}
Expand Down Expand Up @@ -262,26 +263,31 @@ func (b *Bundle) Instantiate(logger logrus.FieldLogger, vuID uint64) (*BundleIns
}

rt := vuImpl.runtime
pgm := init.programs[b.Filename.String()] // this is the main script and it's always present
bi := &BundleInstance{
Runtime: rt,
exports: make(map[string]goja.Callable),
env: b.RuntimeOptions.Env,
moduleVUImpl: vuImpl,
pgm: pgm,
}

// Grab any exported functions that could be executed. These were
// already pre-validated in cmd.validateScenarioConfig(), just get them here.
exports := rt.Get("exports").ToObject(rt)
exports := pgm.module.Get("exports").ToObject(rt)
for k := range b.exports {
fn, _ := goja.AssertFunction(exports.Get(k))
bi.exports[k] = fn
}

jsOptions := rt.Get("options")
jsOptions := exports.Get("options")
var jsOptionsObj *goja.Object
if jsOptions == nil || goja.IsNull(jsOptions) || goja.IsUndefined(jsOptions) {
jsOptionsObj = rt.NewObject()
rt.Set("options", jsOptionsObj)
err := exports.Set("options", jsOptionsObj)
if err != nil {
return nil, fmt.Errorf("couldn't set exported options with merged values: %w", err)
}
} else {
jsOptionsObj = jsOptions.ToObject(rt)
}
Expand All @@ -298,16 +304,23 @@ func (b *Bundle) Instantiate(logger logrus.FieldLogger, vuID uint64) (*BundleIns

// Instantiates the bundle into an existing runtime. Not public because it also messes with a bunch
// of other things, will potentially thrash data and makes a mess in it if the operation fails.

func (b *Bundle) initializeProgramObject(rt *goja.Runtime, init *InitContext) programWithSource {
pgm := programWithSource{
pgm: b.Program,
src: b.Source,
exports: rt.NewObject(),
module: rt.NewObject(),
}
_ = pgm.module.Set("exports", pgm.exports)
init.programs[b.Filename.String()] = pgm
return pgm
}

func (b *Bundle) instantiate(logger logrus.FieldLogger, rt *goja.Runtime, init *InitContext, vuID uint64) (err error) {
rt.SetFieldNameMapper(common.FieldNameMapper{})
rt.SetRandSource(common.NewRandSource())

exports := rt.NewObject()
rt.Set("exports", exports)
module := rt.NewObject()
_ = module.Set("exports", exports)
rt.Set("module", module)

env := make(map[string]string, len(b.RuntimeOptions.Env))
for key, value := range b.RuntimeOptions.Env {
env[key] = value
Expand All @@ -330,10 +343,21 @@ func (b *Bundle) instantiate(logger logrus.FieldLogger, rt *goja.Runtime, init *
init.moduleVUImpl.ctx = context.Background()
init.moduleVUImpl.initEnv = initenv
init.moduleVUImpl.eventLoop = eventloop.New(init.moduleVUImpl)
pgm := b.initializeProgramObject(rt, init)

err = common.RunWithPanicCatching(logger, rt, func() error {
return init.moduleVUImpl.eventLoop.Start(func() error {
_, errRun := rt.RunProgram(b.Program)
return errRun
f, errRun := rt.RunProgram(b.Program)
if errRun != nil {
return errRun
}
if call, ok := goja.AssertFunction(f); ok {
if _, errRun = call(pgm.exports, pgm.module, pgm.exports); errRun != nil {
return errRun
}
return nil
}
panic("Somehow a commonjs main module is not wrapped in a function")
})
})

Expand All @@ -344,6 +368,12 @@ func (b *Bundle) instantiate(logger logrus.FieldLogger, rt *goja.Runtime, init *
}
return err
}
exportsV := pgm.module.Get("exports")
if goja.IsNull(exportsV) {
return errors.New("exports must be an object")
}
pgm.exports = exportsV.ToObject(rt)
init.programs[b.Filename.String()] = pgm
unbindInit()
init.moduleVUImpl.ctx = nil
init.moduleVUImpl.initEnv = nil
Expand Down
33 changes: 6 additions & 27 deletions js/bundle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,11 @@ func TestNewBundle(t *testing.T) {
_, err := getSimpleBundle(t, "/script.js", `throw new Error("aaaa");`)
exception := new(scriptException)
require.ErrorAs(t, err, &exception)
require.EqualError(t, err, "Error: aaaa\n\tat file:///script.js:1:7(2)\n")
require.EqualError(t, err, "Error: aaaa\n\tat file:///script.js:2:7(3)\n\tat native\n")
})
t.Run("InvalidExports", func(t *testing.T) {
t.Parallel()
_, err := getSimpleBundle(t, "/script.js", `exports = null`)
_, err := getSimpleBundle(t, "/script.js", `module.exports = null`)
require.EqualError(t, err, "exports must be an object")
})
t.Run("DefaultUndefined", func(t *testing.T) {
Expand Down Expand Up @@ -169,13 +169,13 @@ func TestNewBundle(t *testing.T) {
// ES2015 modules are not supported
{
"Modules", "base", `export default function() {};`,
"file:///script.js: Line 1:1 Unexpected reserved word",
"file:///script.js: Line 2:1 Unexpected reserved word (and 2 more errors)",
},
// BigInt is not supported
{
"BigInt", "base",
`module.exports.default = function() {}; BigInt(1231412444)`,
"ReferenceError: BigInt is not defined\n\tat file:///script.js:1:47(6)\n",
"ReferenceError: BigInt is not defined\n\tat file:///script.js:2:47(7)\n\tat native\n",
},
}

Expand Down Expand Up @@ -761,27 +761,6 @@ func TestBundleInstantiate(t *testing.T) {
require.Equal(t, true, v.Export())
})

t.Run("SetAndRun", func(t *testing.T) {
t.Parallel()
b, err := getSimpleBundle(t, "/script.js", `
export let options = {
vus: 5,
teardownTimeout: '1s',
};
let val = true;
export default function() { return val; }
`)
require.NoError(t, err)
logger := testutils.NewLogger(t)

bi, err := b.Instantiate(logger, 0)
require.NoError(t, err)
bi.Runtime.Set("val", false)
v, err := bi.exports[consts.DefaultFn](goja.Undefined())
require.NoError(t, err)
require.Equal(t, false, v.Export())
})

t.Run("Options", func(t *testing.T) {
t.Parallel()
b, err := getSimpleBundle(t, "/script.js", `
Expand All @@ -798,7 +777,7 @@ func TestBundleInstantiate(t *testing.T) {
bi, err := b.Instantiate(logger, 0)
require.NoError(t, err)
// Ensure `options` properties are correctly marshalled
jsOptions := bi.Runtime.Get("options").ToObject(bi.Runtime)
jsOptions := bi.pgm.exports.Get("options").ToObject(bi.Runtime)
vus := jsOptions.Get("vus").Export()
require.Equal(t, int64(5), vus)
tdt := jsOptions.Get("teardownTimeout").Export()
Expand All @@ -809,7 +788,7 @@ func TestBundleInstantiate(t *testing.T) {
b.Options.VUs = null.IntFrom(10)
bi2, err := b.Instantiate(logger, 0)
require.NoError(t, err)
jsOptions = bi2.Runtime.Get("options").ToObject(bi2.Runtime)
jsOptions = bi2.pgm.exports.Get("options").ToObject(bi2.Runtime)
vus = jsOptions.Get("vus").Export()
require.Equal(t, int64(10), vus)
b.Options.VUs = optOrig
Expand Down
19 changes: 10 additions & 9 deletions js/compiler/compiler.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,22 +183,23 @@ type compilationState struct {
// srcMap is the current full sourceMap that has been generated read so far
srcMap []byte
srcMapError error
main bool
wrapped bool // whether the original source is wrapped in a function to make it a commonjs module

compiler *Compiler
}

// Compile the program in the given CompatibilityMode, wrapping it between pre and post code
func (c *Compiler) Compile(src, filename string, main bool) (*goja.Program, string, error) {
return c.compileImpl(src, filename, main, c.Options.CompatibilityMode, nil)
// TODO isESM will be used once goja support ESM modules natively
func (c *Compiler) Compile(src, filename string, isESM bool) (*goja.Program, string, error) {
return c.compileImpl(src, filename, !isESM, c.Options.CompatibilityMode, nil)
}

// sourceMapLoader is to be used with goja's WithSourceMapLoader
// it not only gets the file from disk in the simple case, but also returns it if the map was generated from babel
// additioanlly it fixes off by one error in commonjs dependencies due to having to wrap them in a function.
func (c *compilationState) sourceMapLoader(path string) ([]byte, error) {
if path == sourceMapURLFromBabel {
if !c.main {
if c.wrapped {
return c.increaseMappingsByOne(c.srcMap)
}
return c.srcMap, nil
Expand All @@ -214,18 +215,18 @@ func (c *compilationState) sourceMapLoader(path string) ([]byte, error) {
c.srcMap = nil
return nil, c.srcMapError
}
if !c.main {
if c.wrapped {
return c.increaseMappingsByOne(c.srcMap)
}
return c.srcMap, nil
}

func (c *Compiler) compileImpl(
src, filename string, main bool, compatibilityMode lib.CompatibilityMode, srcMap []byte,
src, filename string, wrap bool, compatibilityMode lib.CompatibilityMode, srcMap []byte,
) (*goja.Program, string, error) {
code := src
state := compilationState{srcMap: srcMap, compiler: c, main: main}
if !main { // the lines in the sourcemap (if available) will be fixed by increaseMappingsByOne
state := compilationState{srcMap: srcMap, compiler: c, wrapped: wrap}
if wrap { // the lines in the sourcemap (if available) will be fixed by increaseMappingsByOne
code = "(function(module, exports){\n" + code + "\n})\n"
}
opts := parser.WithDisableSourceMaps
Expand All @@ -248,7 +249,7 @@ func (c *Compiler) compileImpl(
return nil, code, err
}
// the compatibility mode "decreases" here as we shouldn't transform twice
return c.compileImpl(code, filename, main, lib.CompatibilityModeBase, state.srcMap)
return c.compileImpl(code, filename, wrap, lib.CompatibilityModeBase, state.srcMap)
}
return nil, code, err
}
Expand Down
7 changes: 4 additions & 3 deletions js/initcontext.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,10 @@ import (
)

type programWithSource struct {
pgm *goja.Program
src string
module *goja.Object
pgm *goja.Program
src string
module *goja.Object
exports *goja.Object
}

const openCantBeUsedOutsideInitContextMsg = `The "open()" function is only available in the init stage ` +
Expand Down
12 changes: 6 additions & 6 deletions js/initcontext_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,13 @@ func TestInitContextRequire(t *testing.T) {
bi, err := b.Instantiate(logger, 0)
assert.NoError(t, err, "instance error")

exports := bi.Runtime.Get("exports").ToObject(bi.Runtime)
exports := bi.pgm.exports
require.NotNil(t, exports)
_, defaultOk := goja.AssertFunction(exports.Get("default"))
assert.True(t, defaultOk, "default export is not a function")
assert.Equal(t, "abc123", exports.Get("dummy").String())

k6 := bi.Runtime.Get("_k6").ToObject(bi.Runtime)
k6 := exports.Get("_k6").ToObject(bi.Runtime)
require.NotNil(t, k6)
_, groupOk := goja.AssertFunction(k6.Get("group"))
assert.True(t, groupOk, "k6.group is not a function")
Expand All @@ -96,7 +96,7 @@ func TestInitContextRequire(t *testing.T) {
bi, err := b.Instantiate(logger, 0)
require.NoError(t, err)

exports := bi.Runtime.Get("exports").ToObject(bi.Runtime)
exports := bi.pgm.exports
require.NotNil(t, exports)
_, defaultOk := goja.AssertFunction(exports.Get("default"))
assert.True(t, defaultOk, "default export is not a function")
Expand Down Expand Up @@ -130,7 +130,7 @@ func TestInitContextRequire(t *testing.T) {
require.NoError(t, afero.WriteFile(fs, "/file.js", []byte(`throw new Error("aaaa")`), 0o755))
_, err := getSimpleBundle(t, "/script.js", `import "/file.js"; export default function() {}`, fs)
assert.EqualError(t, err,
"Error: aaaa\n\tat file:///file.js:2:7(3)\n\tat go.k6.io/k6/js.(*InitContext).Require-fm (native)\n\tat file:///script.js:1:0(14)\n")
"Error: aaaa\n\tat file:///file.js:2:7(3)\n\tat go.k6.io/k6/js.(*InitContext).Require-fm (native)\n\tat file:///script.js:1:0(15)\n\tat native\n")
})

imports := map[string]struct {
Expand Down Expand Up @@ -282,7 +282,7 @@ func TestInitContextOpen(t *testing.T) {
t.Parallel()
bi, err := createAndReadFile(t, tc.file, tc.content, tc.length, "")
require.NoError(t, err)
assert.Equal(t, string(tc.content), bi.Runtime.Get("data").Export())
assert.Equal(t, string(tc.content), bi.pgm.exports.Get("data").Export())
})
}

Expand All @@ -291,7 +291,7 @@ func TestInitContextOpen(t *testing.T) {
bi, err := createAndReadFile(t, "/path/to/file.bin", []byte("hi!\x0f\xff\x01"), 6, "b")
require.NoError(t, err)
buf := bi.Runtime.NewArrayBuffer([]byte{104, 105, 33, 15, 255, 1})
assert.Equal(t, buf, bi.Runtime.Get("data").Export())
assert.Equal(t, buf, bi.pgm.exports.Get("data").Export())
})

testdata := map[string]string{
Expand Down
Loading

0 comments on commit e2316a2

Please sign in to comment.