From f2e86083c6b3a0df746ae0283289569598f5beab Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Wed, 23 Oct 2024 18:35:06 +0300 Subject: [PATCH 1/2] Update Sobek and fix http.File to not use []byte Sobek updates bring fixes from goja for: - nesting of destructuring with arrays - using slices from go code in js code Also fix for async errors in modules triggering the promise rejection tracker as well, which will be needed for top-level-await The changes to handling of slices did break some tests and showcased that k6 was handling `http.File().data` badly. Previously it was `[]byte` which it really shouldn't be. This fix makes certain that the provided input is `string` or `ArrayBuffer` and keeps it that way until it needs to be written. This aligns with the documentation as well as removes a source of pure `[]byte` in js code which there shouldn't be any. Separate PR should be made to drop being able to use `common.ToBytes` with `[]byte` as that should never happen and should be 100% a bug. Also made the go code return a pointer as that just makes sense instead of a struct that will get copied. --- go.mod | 2 +- go.sum | 4 ++-- js/modules/k6/http/file.go | 19 ++++++++++--------- js/modules/k6/http/file_test.go | 14 +++++++------- js/modules/k6/http/request.go | 10 +++++++--- .../grafana/sobek/modules_sourcetext.go | 3 +++ .../grafana/sobek/object_goreflect.go | 3 +++ .../grafana/sobek/parser/expression.go | 2 +- vendor/modules.txt | 2 +- 9 files changed, 35 insertions(+), 24 deletions(-) diff --git a/go.mod b/go.mod index 3936e2e2533..16a4b3f46dc 100644 --- a/go.mod +++ b/go.mod @@ -76,7 +76,7 @@ require ( github.com/go-logr/stdr v1.2.2 // indirect github.com/google/pprof v0.0.0-20230728192033-2ba5b33183c6 // indirect github.com/google/uuid v1.6.0 // indirect - github.com/grafana/sobek v0.0.0-20240927094302-19dd311f018f + github.com/grafana/sobek v0.0.0-20241023145759-2dc9daf5bfa2 github.com/grpc-ecosystem/grpc-gateway/v2 v2.22.0 // indirect github.com/inconshreveable/mousetrap v1.0.0 // indirect github.com/josharian/intern v1.0.0 // indirect diff --git a/go.sum b/go.sum index 25e82c98c8b..22073589146 100644 --- a/go.sum +++ b/go.sum @@ -87,8 +87,8 @@ github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/gorilla/websocket v1.5.3 h1:saDtZ6Pbx/0u+bgYQ3q96pZgCzfhKXGPqt7kZ72aNNg= github.com/gorilla/websocket v1.5.3/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE= -github.com/grafana/sobek v0.0.0-20240927094302-19dd311f018f h1:9r05Uxs+Pq1UqZqGG/PoCUWd6xk2ab+mRWAM5xZgM9I= -github.com/grafana/sobek v0.0.0-20240927094302-19dd311f018f/go.mod h1:FmcutBFPLiGgroH42I4/HBahv7GxVjODcVWFTw1ISes= +github.com/grafana/sobek v0.0.0-20241023145759-2dc9daf5bfa2 h1:qgthy9RbAIxinOmXaB9nSaT/w00VTqeEQ7JI0a+ScUU= +github.com/grafana/sobek v0.0.0-20241023145759-2dc9daf5bfa2/go.mod h1:FmcutBFPLiGgroH42I4/HBahv7GxVjODcVWFTw1ISes= github.com/grafana/xk6-browser v1.8.5 h1:dNAG8dhcaEx/HOELEnGzAw8ShCvkpukfyTGUhebZsj0= github.com/grafana/xk6-browser v1.8.5/go.mod h1:yCtZ4G8U/imVBikBO4HJoMyNoejmECcJk4CK5XGSxis= github.com/grafana/xk6-dashboard v0.7.5 h1:TcILyffT/Ea/XD7xG1jMA5lwtusOPRbEQsQDHmO30Mk= diff --git a/js/modules/k6/http/file.go b/js/modules/k6/http/file.go index 202b1f83cdc..53f49fdc103 100644 --- a/js/modules/k6/http/file.go +++ b/js/modules/k6/http/file.go @@ -5,12 +5,12 @@ import ( "strings" "time" - "go.k6.io/k6/js/common" + "github.com/grafana/sobek" ) // FileData represents a binary file requiring multipart request encoding type FileData struct { - Data []byte + Data any Filename string ContentType string } @@ -22,7 +22,7 @@ func escapeQuotes(s string) string { } // File returns a FileData object. -func (mi *ModuleInstance) file(data interface{}, args ...string) FileData { +func (mi *ModuleInstance) file(data any, args ...string) (*FileData, error) { // supply valid default if filename and content-type are not specified fname, ct := fmt.Sprintf("%d", time.Now().UnixNano()), "application/octet-stream" @@ -34,14 +34,15 @@ func (mi *ModuleInstance) file(data interface{}, args ...string) FileData { } } - dt, err := common.ToBytes(data) - if err != nil { - common.Throw(mi.vu.Runtime(), err) + switch data.(type) { + case string, sobek.ArrayBuffer: + default: + return nil, fmt.Errorf("invalid type %T, expected string or ArrayBuffer", data) } - return FileData{ - Data: dt, + return &FileData{ + Data: data, Filename: fname, ContentType: ct, - } + }, nil } diff --git a/js/modules/k6/http/file_test.go b/js/modules/k6/http/file_test.go index f9d27e7b345..d89d36d0bfe 100644 --- a/js/modules/k6/http/file_test.go +++ b/js/modules/k6/http/file_test.go @@ -10,12 +10,12 @@ import ( func TestHTTPFile(t *testing.T) { t.Parallel() - input := []byte{104, 101, 108, 108, 111} + input := "hello" testCases := []struct { input func(rt *sobek.Runtime) interface{} args []string - expected FileData + expected *FileData expErr string }{ // We can't really test without specifying a filename argument, @@ -24,22 +24,22 @@ func TestHTTPFile(t *testing.T) { { func(*sobek.Runtime) interface{} { return input }, []string{"test.bin"}, - FileData{Data: input, Filename: "test.bin", ContentType: "application/octet-stream"}, + &FileData{Data: input, Filename: "test.bin", ContentType: "application/octet-stream"}, "", }, { func(*sobek.Runtime) interface{} { return string(input) }, []string{"test.txt", "text/plain"}, - FileData{Data: input, Filename: "test.txt", ContentType: "text/plain"}, + &FileData{Data: input, Filename: "test.txt", ContentType: "text/plain"}, "", }, { - func(rt *sobek.Runtime) interface{} { return rt.NewArrayBuffer(input) }, + func(rt *sobek.Runtime) interface{} { return rt.NewArrayBuffer([]byte(input)) }, []string{"test-ab.bin"}, - FileData{Data: input, Filename: "test-ab.bin", ContentType: "application/octet-stream"}, + &FileData{Data: input, Filename: "test-ab.bin", ContentType: "application/octet-stream"}, "", }, - {func(*sobek.Runtime) interface{} { return struct{}{} }, []string{}, FileData{}, "GoError: invalid type struct {}, expected string, []byte or ArrayBuffer"}, + {func(*sobek.Runtime) interface{} { return struct{}{} }, []string{}, &FileData{}, "GoError: invalid type struct {}, expected string or ArrayBuffer"}, } for i, tc := range testCases { diff --git a/js/modules/k6/http/request.go b/js/modules/k6/http/request.go index efff9661339..721113f6ca0 100644 --- a/js/modules/k6/http/request.go +++ b/js/modules/k6/http/request.go @@ -211,7 +211,7 @@ func (c *Client) parseRequest( // Otherwise parameters are treated as standard form field. for k, v := range data { switch ve := v.(type) { - case FileData: + case *FileData: // writing our own part to handle receiving // different content-type than the default application/octet-stream h := make(textproto.MIMEHeader) @@ -228,7 +228,11 @@ func (c *Client) parseRequest( return err } - if _, err := fw.Write(ve.Data); err != nil { + data, err := common.ToBytes(ve.Data) + if err != nil { + return err + } + if _, err := fw.Write(data); err != nil { return err } default: @@ -575,7 +579,7 @@ func (c *Client) parseBatchRequest(key interface{}, val interface{}) (*httpext.P func requestContainsFile(data map[string]interface{}) bool { for _, v := range data { - if _, ok := v.(FileData); ok { + if _, ok := v.(*FileData); ok { return true } } diff --git a/vendor/github.com/grafana/sobek/modules_sourcetext.go b/vendor/github.com/grafana/sobek/modules_sourcetext.go index f39ec1fe067..4442173a822 100644 --- a/vendor/github.com/grafana/sobek/modules_sourcetext.go +++ b/vendor/github.com/grafana/sobek/modules_sourcetext.go @@ -34,6 +34,9 @@ func (s *SourceTextModuleInstance) ExecuteModule(rt *Runtime, res, rej func(inte if res != nil { panic("sobek bug where a not async module was executed as async on") } + // we handle the failure below so we need to mark this as promise as handled so it doesn't + // trigger the PromiseRejectionTracker + rt.performPromiseThen(s.asyncPromise, rt.ToValue(func() {}), rt.ToValue(func() {}), nil) switch s.asyncPromise.state { case PromiseStateFulfilled: return s, nil diff --git a/vendor/github.com/grafana/sobek/object_goreflect.go b/vendor/github.com/grafana/sobek/object_goreflect.go index 16f837bd38e..12611c28f34 100644 --- a/vendor/github.com/grafana/sobek/object_goreflect.go +++ b/vendor/github.com/grafana/sobek/object_goreflect.go @@ -221,6 +221,9 @@ func (o *objectGoReflect) _getMethod(jsName string) reflect.Value { func (o *objectGoReflect) elemToValue(ev reflect.Value) (Value, reflectValueWrapper) { if isContainer(ev.Kind()) { + if ev.CanAddr() { + ev = ev.Addr() + } ret := o.val.runtime.toValue(ev.Interface(), ev) if obj, ok := ret.(*Object); ok { if w, ok := obj.self.(reflectValueWrapper); ok { diff --git a/vendor/github.com/grafana/sobek/parser/expression.go b/vendor/github.com/grafana/sobek/parser/expression.go index 875b1c1577e..7814080efb8 100644 --- a/vendor/github.com/grafana/sobek/parser/expression.go +++ b/vendor/github.com/grafana/sobek/parser/expression.go @@ -1536,7 +1536,7 @@ func (self *_parser) reinterpretAsArrayBindingPattern(left *ast.ArrayLiteral) as rest = self.reinterpretAsDestructBindingTarget(spread.Expression) value = value[:len(value)-1] } else { - value[i] = self.reinterpretAsBindingElement(item) + value[i] = self.reinterpretAsAssignmentElement(item) } } return &ast.ArrayPattern{ diff --git a/vendor/modules.txt b/vendor/modules.txt index 91bb48b8770..aba6af6245c 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -160,7 +160,7 @@ github.com/google/uuid # github.com/gorilla/websocket v1.5.3 ## explicit; go 1.12 github.com/gorilla/websocket -# github.com/grafana/sobek v0.0.0-20240927094302-19dd311f018f +# github.com/grafana/sobek v0.0.0-20241023145759-2dc9daf5bfa2 ## explicit; go 1.20 github.com/grafana/sobek github.com/grafana/sobek/ast From 67f3283d8803f2fff31bd24d41941c043d322919 Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Thu, 24 Oct 2024 10:37:02 +0300 Subject: [PATCH 2/2] fixup! Update Sobek and fix http.File to not use []byte --- js/modules/k6/http/file_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/modules/k6/http/file_test.go b/js/modules/k6/http/file_test.go index d89d36d0bfe..56de3911658 100644 --- a/js/modules/k6/http/file_test.go +++ b/js/modules/k6/http/file_test.go @@ -28,7 +28,7 @@ func TestHTTPFile(t *testing.T) { "", }, { - func(*sobek.Runtime) interface{} { return string(input) }, + func(*sobek.Runtime) interface{} { return input }, []string{"test.txt", "text/plain"}, &FileData{Data: input, Filename: "test.txt", ContentType: "text/plain"}, "",