diff --git a/CHANGELOG.md b/CHANGELOG.md index f58b1d46..3a08081c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +## v0.39.8 (WIP) + +- Properly reset JSVM global `$app` state overwrite so that pooled executors always get a clean state. + + ## v0.39.7 - Replaced `github.com/go-ozzo/ozzo-validation` with the fork `github.com/pocketbase/ozzo-validation` since the original library has recently changed ownership and the new maintainer cannot be trusted. diff --git a/plugins/jsvm/binds.go b/plugins/jsvm/binds.go index 8cbe06cf..77db1cb5 100644 --- a/plugins/jsvm/binds.go +++ b/plugins/jsvm/binds.go @@ -79,10 +79,11 @@ func hooksBinds(app core.App, loader *goja.Runtime, executors *vmsPool) { } err := executors.run(func(executor *goja.Runtime) error { - executor.Set("$app", goja.Undefined()) + oldApp := executor.Get("$app") executor.Set("__args", handlerArgs) res, err := executor.RunProgram(pr) executor.Set("__args", goja.Undefined()) + executor.Set("$app", oldApp) // reset to its default for the executor // check for returned Go error value if resErr := checkGojaValueForError(app, res); resErr != nil { @@ -192,10 +193,12 @@ func wrapHandlerFunc(executors *vmsPool, handler goja.Value) (func(*core.Request wrappedHandler := func(e *core.RequestEvent) error { return executors.run(func(executor *goja.Runtime) error { + oldApp := executor.Get("$app") executor.Set("$app", e.App) // overwrite the global $app with the hook scoped instance executor.Set("__args", []any{e}) res, err := executor.RunProgram(pr) executor.Set("__args", goja.Undefined()) + executor.Set("$app", oldApp) // check for returned Go error value if resErr := checkGojaValueForError(e.App, res); resErr != nil { @@ -247,10 +250,12 @@ func wrapMiddlewares(executors *vmsPool, rawMiddlewares ...goja.Value) ([]*hook. Priority: v.priority, Func: func(e *core.RequestEvent) error { return executors.run(func(executor *goja.Runtime) error { + oldApp := executor.Get("$app") executor.Set("$app", e.App) // overwrite the global $app with the hook scoped instance executor.Set("__args", []any{e}) res, err := executor.RunProgram(pr) executor.Set("__args", goja.Undefined()) + executor.Set("$app", oldApp) // check for returned Go error value if resErr := checkGojaValueForError(e.App, res); resErr != nil { @@ -267,10 +272,12 @@ func wrapMiddlewares(executors *vmsPool, rawMiddlewares ...goja.Value) ([]*hook. wrappedMiddlewares[i] = &hook.Handler[*core.RequestEvent]{ Func: func(e *core.RequestEvent) error { return executors.run(func(executor *goja.Runtime) error { + oldApp := executor.Get("$app") executor.Set("$app", e.App) // overwrite the global $app with the hook scoped instance executor.Set("__args", []any{e}) res, err := executor.RunProgram(pr) executor.Set("__args", goja.Undefined()) + executor.Set("$app", oldApp) // check for returned Go error value if resErr := checkGojaValueForError(e.App, res); resErr != nil { diff --git a/plugins/jsvm/binds_app_reset_test.go b/plugins/jsvm/binds_app_reset_test.go new file mode 100644 index 00000000..1c6f551d --- /dev/null +++ b/plugins/jsvm/binds_app_reset_test.go @@ -0,0 +1,314 @@ +package jsvm + +import ( + "net/http/httptest" + "testing" + + "github.com/dop251/goja" + "github.com/pocketbase/pocketbase/apis" + "github.com/pocketbase/pocketbase/core" + "github.com/pocketbase/pocketbase/tests" +) + +func TestHooksAppReset(t *testing.T) { + t.Parallel() + + testApp, _ := tests.NewTestApp() + defer testApp.Cleanup() + + createVM := func() *goja.Runtime { + vm := goja.New() + vm.SetFieldNameMapper(FieldMapper{}) + vm.Set("$app", testApp) + return vm + } + + loader := createVM() + + pool := newPool(1, createVM) + + hooksBinds(testApp, loader, pool) + + // register any hook + _, err := loader.RunScript("stub", ` + onRecordCreate((e) => { + e.next() + + $app = 123; // overwrite + }) + `) + if err != nil { + t.Fatal(err) + } + + // insert a dummy record to trigger the JS hook + { + collection, err := testApp.FindCollectionByNameOrId("demo2") + if err != nil { + t.Fatal(err) + } + + record := core.NewRecord(collection) + record.Set("title", "test") + if err := testApp.Save(record); err != nil { + t.Fatal(err) + } + } + + // check the executor state + pool.run(func(vm *goja.Runtime) error { + val, err := vm.RunScript("verify", `$app`) + if err != nil { + t.Fatal(err) + } + + if valApp := val.Export(); valApp != testApp { + t.Fatalf("Expected $app to reset to its original value, got %v", valApp) + } + + return nil + }) +} + +func TestRouterHandlerAppReset(t *testing.T) { + t.Parallel() + + testApp, _ := tests.NewTestApp() + defer testApp.Cleanup() + + createVM := func() *goja.Runtime { + vm := goja.New() + vm.SetFieldNameMapper(FieldMapper{}) + vm.Set("$app", testApp) + return vm + } + + loader := createVM() + + pool := newPool(1, createVM) + + routerBinds(testApp, loader, pool) + + // register route handler hook + _, err := loader.RunScript("stub", ` + routerAdd("GET", "/test", (e) => { + $app = 123; // overwrite + + return e.string(200, "test") + }) + `) + if err != nil { + t.Fatal(err) + } + + // create mock web server + { + baseRouter, err := apis.NewRouter(testApp) + if err != nil { + t.Fatal(err) + } + + // manually trigger the serve event to ensure that custom app routes and middlewares are registered + serveEvent := new(core.ServeEvent) + serveEvent.App = testApp + serveEvent.Router = baseRouter + _ = testApp.OnServe().Trigger(serveEvent, func(e *core.ServeEvent) error { + req := httptest.NewRequest("GET", "/test", nil) + + recorder := httptest.NewRecorder() + + mux, err := e.Router.BuildMux() + if err != nil { + t.Fatalf("Failed to build router mux: %v", err) + } + mux.ServeHTTP(recorder, req) + + if recorder.Code != 200 { + t.Fatalf("Expected status code %d, got %d", 200, recorder.Code) + } + + body := recorder.Body.String() + if body != "test" { + t.Fatalf("Expected body %q, got %q", "test", body) + } + + return nil + }) + } + + pool.run(func(vm *goja.Runtime) error { + val, err := vm.RunScript("verify", `$app`) + if err != nil { + t.Fatal(err) + } + + if valApp := val.Export(); valApp != testApp { + t.Fatalf("Expected $app to reset to its original value, got %v", valApp) + } + + return nil + }) +} + +func TestRouterMiddlewareFuncAppReset(t *testing.T) { + t.Parallel() + + testApp, _ := tests.NewTestApp() + defer testApp.Cleanup() + + createVM := func() *goja.Runtime { + vm := goja.New() + vm.SetFieldNameMapper(FieldMapper{}) + vm.Set("$app", testApp) + return vm + } + + loader := createVM() + + pool := newPool(1, createVM) + + routerBinds(testApp, loader, pool) + + // register route middleware func + _, err := loader.RunScript("stub", ` + routerUse((e) => { + e.string(200, "test") + + $app = 123; // overwrite + }) + `) + if err != nil { + t.Fatal(err) + } + + // create mock web server + { + baseRouter, err := apis.NewRouter(testApp) + if err != nil { + t.Fatal(err) + } + + // manually trigger the serve event to ensure that custom app routes and middlewares are registered + serveEvent := new(core.ServeEvent) + serveEvent.App = testApp + serveEvent.Router = baseRouter + _ = testApp.OnServe().Trigger(serveEvent, func(e *core.ServeEvent) error { + // it doesn't matter as long as the middleware is called + req := httptest.NewRequest("GET", "/anything", nil) + + recorder := httptest.NewRecorder() + + mux, err := e.Router.BuildMux() + if err != nil { + t.Fatalf("Failed to build router mux: %v", err) + } + mux.ServeHTTP(recorder, req) + + if recorder.Code != 200 { + t.Fatalf("Expected status code %d, got %d", 200, recorder.Code) + } + + body := recorder.Body.String() + if body != "test" { + t.Fatalf("Expected body %q, got %q", "test", body) + } + + return nil + }) + } + + pool.run(func(vm *goja.Runtime) error { + val, err := vm.RunScript("verify", `$app`) + if err != nil { + t.Fatal(err) + } + + if valApp := val.Export(); valApp != testApp { + t.Fatalf("Expected $app to reset to its original value, got %v", valApp) + } + + return nil + }) +} + +func TestRouterMiddlewareClassAppReset(t *testing.T) { + t.Parallel() + + testApp, _ := tests.NewTestApp() + defer testApp.Cleanup() + + createVM := func() *goja.Runtime { + vm := goja.New() + vm.SetFieldNameMapper(FieldMapper{}) + vm.Set("$app", testApp) + BindCore(vm) + return vm + } + + loader := createVM() + + pool := newPool(1, createVM) + + routerBinds(testApp, loader, pool) + + // register route middleware class + _, err := loader.RunScript("stub", ` + routerUse(new Middleware((e) => { + e.string(200, "test") + + $app = 123; // overwrite + })) + `) + if err != nil { + t.Fatal(err) + } + + // create mock web server + { + baseRouter, err := apis.NewRouter(testApp) + if err != nil { + t.Fatal(err) + } + + // manually trigger the serve event to ensure that custom app routes and middlewares are registered + serveEvent := new(core.ServeEvent) + serveEvent.App = testApp + serveEvent.Router = baseRouter + _ = testApp.OnServe().Trigger(serveEvent, func(e *core.ServeEvent) error { + // it doesn't matter as long as the middleware is called + req := httptest.NewRequest("GET", "/anything", nil) + + recorder := httptest.NewRecorder() + + mux, err := e.Router.BuildMux() + if err != nil { + t.Fatalf("Failed to build router mux: %v", err) + } + mux.ServeHTTP(recorder, req) + + if recorder.Code != 200 { + t.Fatalf("Expected status code %d, got %d", 200, recorder.Code) + } + + body := recorder.Body.String() + if body != "test" { + t.Fatalf("Expected body %q, got %q", "test", body) + } + + return nil + }) + } + + pool.run(func(vm *goja.Runtime) error { + val, err := vm.RunScript("verify", `$app`) + if err != nil { + t.Fatal(err) + } + + if valApp := val.Export(); valApp != testApp { + t.Fatalf("Expected $app to reset to its original value, got %v", valApp) + } + + return nil + }) +} diff --git a/tests/data/data.db b/tests/data/data.db index c08504d0..3e0f184e 100644 Binary files a/tests/data/data.db and b/tests/data/data.db differ