passing invalid utf8 out to attached ruby proc causes extension-level crash about mini_racer HOT 4 CLOSED

I am all for adding proper UTF-16 support but this particular issue is nothing to do with conversion

 context.attach('echo', proc{|v| return v })

Should be

context.attach('echo', proc{|v| v })

return from a proc has very very confusing semantics ...

irb(main):007:0> @my_proc = proc{return "hi"}
=> #<Proc:0x007f211dbad7c8@(irb):7>
irb(main):008:0> def x; puts 1; @my_proc.call; puts 2; end
=> :x
irb(main):009:0> x
1
LocalJumpError: unexpected return

vs

irb(main):010:0> @my_lambda = lambda{return "hi"}
=> #<Proc:0x007f211db7ded8@(irb):10 (lambda)>
irb(main):011:0> def x; puts 1; @my_lambda.call; puts 2; end
=> :x
irb(main):012:0> x
1
2
=> nil

Use lambdas, not procs

from mini_racer.

Indeed, I'm not able to repro the issue with this simplified test, which means it's probably in my ruby code on the other side of the attached method somewhere.

Looks like after adjusting the test case, the 'malformed utf8' can travel safely to and from rubyland as I assume you discovered after fixing my typo.

I also have added some code to strip out invalid utf8 chars coming in from JS as a post-process operation.

There's probably a better way (use utf-16 encoded strings being passed into attached methods? maybe?)

Anyhow. Thanks for the assist.

from mini_racer.

I am totally open to adding utf16 support but it gets pretty hairy and we would need to be super careful not to make all string handling slower just cause we add support for it.

from mini_racer.

Yeah, it's a tough proposition, because in terms of stability, having all the strings that are passed into attached methods matching JS string formats should reduce friction at this barrier- but utf16 is definitely a heftier encoding--
does seem like it would be more performant to not change the encoding of the strings when traveling between rb&js though.

from mini_racer.