Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix mapJSHandle invalid memory address or nil pointer dereference #1544

Merged
merged 6 commits into from
Nov 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 12 additions & 6 deletions browser/frame_mapping.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,19 +59,25 @@ func mapFrame(vu moduleVU, f *common.Frame) mapping { //nolint:gocognit,cyclop
return nil, f.DispatchEvent(selector, typ, exportArg(eventInit), popts) //nolint:wrapcheck
}), nil
},
"evaluate": func(pageFunction sobek.Value, gargs ...sobek.Value) *sobek.Promise {
"evaluate": func(pageFunc sobek.Value, gargs ...sobek.Value) (*sobek.Promise, error) {
if sobekEmptyString(pageFunc) {
return nil, fmt.Errorf("evaluate requires a page function")
}
return k6ext.Promise(vu.Context(), func() (any, error) {
return f.Evaluate(pageFunction.String(), exportArgs(gargs)...) //nolint:wrapcheck
})
return f.Evaluate(pageFunc.String(), exportArgs(gargs)...)
}), nil
},
"evaluateHandle": func(pageFunction sobek.Value, gargs ...sobek.Value) *sobek.Promise {
"evaluateHandle": func(pageFunc sobek.Value, gargs ...sobek.Value) (*sobek.Promise, error) {
if sobekEmptyString(pageFunc) {
return nil, fmt.Errorf("evaluateHandle requires a page function")
}
return k6ext.Promise(vu.Context(), func() (any, error) {
jsh, err := f.EvaluateHandle(pageFunction.String(), exportArgs(gargs)...)
jsh, err := f.EvaluateHandle(pageFunc.String(), exportArgs(gargs)...)
if err != nil {
return nil, err //nolint:wrapcheck
}
return mapJSHandle(vu, jsh), nil
})
}), nil
},
"fill": func(selector, value string, opts sobek.Value) *sobek.Promise {
return k6ext.Promise(vu.Context(), func() (any, error) {
Expand Down
6 changes: 6 additions & 0 deletions browser/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package browser
import (
"context"
"errors"
"strings"

"github.com/grafana/sobek"

Expand Down Expand Up @@ -61,3 +62,8 @@ func exportArgs(gargs []sobek.Value) []any {
func sobekValueExists(v sobek.Value) bool {
return v != nil && !sobek.IsUndefined(v) && !sobek.IsNull(v)
}

// sobekEmptyString returns true if a given value is not nil or an empty string.
func sobekEmptyString(v sobek.Value) bool {
return !sobekValueExists(v) || strings.TrimSpace(v.String()) == ""
}
25 changes: 25 additions & 0 deletions browser/helpers_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package browser

import (
"testing"

"github.com/grafana/sobek"
"github.com/stretchr/testify/require"
)

func TestSobekEmptyString(t *testing.T) {
// SobekEmpty string should return true if the argument
// is an empty string or not defined in the Sobek runtime.
rt := sobek.New()
require.NoError(t, rt.Set("sobekEmptyString", sobekEmptyString))
for _, s := range []string{"() => true", "'() => false'"} { // not empty
v, err := rt.RunString(`sobekEmptyString(` + s + `)`)
require.NoError(t, err)
require.Falsef(t, v.ToBoolean(), "got: true, want: false for %q", s)
}
for _, s := range []string{"", " ", "null", "undefined"} { // empty
v, err := rt.RunString(`sobekEmptyString(` + s + `)`)
require.NoError(t, err)
require.Truef(t, v.ToBoolean(), "got: false, want: true for %q", s)
}
}
20 changes: 14 additions & 6 deletions browser/js_handle_mapping.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package browser

import (
"fmt"

"github.com/grafana/sobek"

"github.com/grafana/xk6-browser/common"
Expand All @@ -15,26 +17,32 @@ func mapJSHandle(vu moduleVU, jsh common.JSHandleAPI) mapping {
},
"dispose": func() *sobek.Promise {
return k6ext.Promise(vu.Context(), func() (any, error) {
return nil, jsh.Dispose() //nolint:wrapcheck
return nil, jsh.Dispose()
})
},
"evaluate": func(pageFunc sobek.Value, gargs ...sobek.Value) *sobek.Promise {
"evaluate": func(pageFunc sobek.Value, gargs ...sobek.Value) (*sobek.Promise, error) {
if sobekEmptyString(pageFunc) {
return nil, fmt.Errorf("evaluate requires a page function")
}
return k6ext.Promise(vu.Context(), func() (any, error) {
args := make([]any, 0, len(gargs))
for _, a := range gargs {
args = append(args, exportArg(a))
}
return jsh.Evaluate(pageFunc.String(), args...) //nolint:wrapcheck
})
return jsh.Evaluate(pageFunc.String(), args...)
}), nil
},
"evaluateHandle": func(pageFunc sobek.Value, gargs ...sobek.Value) *sobek.Promise {
"evaluateHandle": func(pageFunc sobek.Value, gargs ...sobek.Value) (*sobek.Promise, error) {
if sobekEmptyString(pageFunc) {
return nil, fmt.Errorf("evaluateHandle requires a page function")
}
return k6ext.Promise(vu.Context(), func() (any, error) {
h, err := jsh.EvaluateHandle(pageFunc.String(), exportArgs(gargs)...)
if err != nil {
return nil, err //nolint:wrapcheck
}
return mapJSHandle(vu, h), nil
})
}), nil
},
"getProperties": func() *sobek.Promise {
return k6ext.Promise(vu.Context(), func() (any, error) {
Expand Down
16 changes: 11 additions & 5 deletions browser/page_mapping.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,19 +80,25 @@ func mapPage(vu moduleVU, p *common.Page) mapping { //nolint:gocognit,cyclop
return nil, p.EmulateVisionDeficiency(typ) //nolint:wrapcheck
})
},
"evaluate": func(pageFunction sobek.Value, gargs ...sobek.Value) *sobek.Promise {
"evaluate": func(pageFunc sobek.Value, gargs ...sobek.Value) (*sobek.Promise, error) {
if sobekEmptyString(pageFunc) {
return nil, fmt.Errorf("evaluate requires a page function")
}
return k6ext.Promise(vu.Context(), func() (any, error) {
return p.Evaluate(pageFunction.String(), exportArgs(gargs)...) //nolint:wrapcheck
})
return p.Evaluate(pageFunc.String(), exportArgs(gargs)...)
}), nil
},
"evaluateHandle": func(pageFunc sobek.Value, gargs ...sobek.Value) *sobek.Promise {
"evaluateHandle": func(pageFunc sobek.Value, gargs ...sobek.Value) (*sobek.Promise, error) {
if sobekEmptyString(pageFunc) {
return nil, fmt.Errorf("evaluateHandle requires a page function")
}
return k6ext.Promise(vu.Context(), func() (any, error) {
jsh, err := p.EvaluateHandle(pageFunc.String(), exportArgs(gargs)...)
if err != nil {
return nil, err //nolint:wrapcheck
}
return mapJSHandle(vu, jsh), nil
})
}), nil
},
"fill": func(selector string, value string, opts sobek.Value) *sobek.Promise {
return k6ext.Promise(vu.Context(), func() (any, error) {
Expand Down
5 changes: 5 additions & 0 deletions tests/page_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,11 @@ func TestPageEvaluateMappingError(t *testing.T) {
script: "(6)",
wantErr: "Given expression does not evaluate to a function",
},
{
name: "void",
script: "",
wantErr: "evaluate requires a page function",
},
ankur22 marked this conversation as resolved.
Show resolved Hide resolved
}

for _, tt := range tests {
Expand Down
Loading