Skip to content
This repository was archived by the owner on Jan 30, 2025. It is now read-only.

Fix mapJSHandle invalid memory address or nil pointer dereference #1544

Merged
merged 6 commits into from
Nov 26, 2024
Merged

Fix mapJSHandle invalid memory address or nil pointer dereference #1544

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6a7c1c8
Add browser sobekEmptyString helper
inancgumus Nov 20, 2024
ff81c83
Fix jshandle evaluates for missing arg
inancgumus Nov 20, 2024
4e0606f
Fix page evaluates for missing arg
inancgumus Nov 20, 2024
b598322
Fix frame evaluates for missing arg
inancgumus Nov 20, 2024
55a0484
Add TestPageEvaluateMappingError.void
inancgumus Nov 20, 2024
b948275
Add TestSobekEmptyString
inancgumus Nov 25, 2024
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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",
},
}

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