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

Avoid null terminated strings in the interface for cgo #192

Open
dylanahsmith opened this issue Oct 1, 2021 · 2 comments
Open

Avoid null terminated strings in the interface for cgo #192

dylanahsmith opened this issue Oct 1, 2021 · 2 comments

Comments

@dylanahsmith
Copy link
Collaborator

Here is a script that demonstrates the problem

package main

import (
	"fmt"
	v8 "rogchap.com/v8go"
)

func main() {
	iso := v8.NewIsolate()
	ctx := v8.NewContext(iso)
	val, err := ctx.RunScript(`"before\x00after"`, "script.js")
	if err != nil { panic(err) }
	fmt.Printf("%#v\n", val.String())
}

which prints "before" but it should print "before\x00after" like fmt.Printf("%#v\n", "before\x00after") does.

Similarly, you can try using null characters in JS and you will see that the null character is preserved.

Instead, v8go.h functions should return strings as a character pointer and length (could be wrapped in a struct) so they can be converted to Go strings with C.GoStringN and avoid using C.GoString for JS strings. That way these null characters can be preserved.

There also is an opportunity to avoid extra data copying by returning pointers directly to the V8's string data, when we know that data will be copied to a Go string anyways. Right now, the V8 string data is copied to a null terminated string, which is then copied to a Go string.

dylanahsmith added a commit that referenced this issue Oct 1, 2021
@dylanahsmith
Copy link
Collaborator Author

dylanahsmith commented Oct 8, 2021

For passing in strings, we might want adapter functions in the import "C" preamble so that we can access _GoString_, _GoStringPtr and _GoStringLen. That way the full string can be preserved and won't need extra copying. If we want to avoid defining those adapter functions in a comment, then we could move them into a C header file to include from that preamble, as long as this header file isn't included from v8go.cc.

If we do define a wrapper string struct, then we probably want to define it with the same memory layout as _GoString_ to make it more likely that the conversion from _GoString_ can be optimized away by the compiler. E.g. typedef struct { const char *data, size_t len } GoString.

@backkem
Copy link

backkem commented Dec 23, 2021

I'd like to help solve this to make sure the base64 polyfill from kuoruan/v8go-polyfills works correctly on 'binary strings'.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants