String::NewExternal should not crash the renderer.

String::NewExternal should not crash the renderer.

The blink may pass a very long string to v8
for making a handle to it. v8 has max string length
limit and creates exception. But NewExternal code does not
check that the handle is null and crashes the renderer.

With the fix the js code receives the exception.

BUG=414615
LOG=N
R=yangguo@chromium.org, yurys@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=24250

[loislo, 2014-09-26 09:07:44]

loislo@chromium.org changed reviewers:
+ alph@chromium.org, yangguo@chromium.org, yurys@chromium.org

[loislo, 2014-09-26 09:07:55]

ptal

[loislo, 2014-09-26 09:29:37]

loislo@chromium.org changed reviewers:
+ jochen@chromium.org

[loislo, 2014-09-26 09:29:37]

+jochen

[Yang, 2014-09-26 09:29:59]

https://codereview.chromium.org/603393004/diff/1/src/api.cc
File src/api.cc (right):

https://codereview.chromium.org/603393004/diff/1/src/api.cc#newcode5508
src/api.cc:5508: if (maybe_string.ToHandle(&string))
At this point we have a pending exception. It needs to be promoted to scheduled
exception. Please use something like

EXCEPTION_PREAMBLE(i_isolate),
i::Handle<i::String> result;
has_pending_exception = !NewExternalStringHandle(i_isolate,
resource).ToHandle(&result);
EXCEPTION_BAILOUT_CHECK(i_isolate, Local<String>());
i_isolate->heap()->external_string_table()->AddString(*result);
return Utils::ToLocal(string);

https://codereview.chromium.org/603393004/diff/1/src/api.cc#newcode5551
src/api.cc:5551: return Utils::ToLocal(string);
Ditto.

[yurys, 2014-09-26 09:30:34]

lgtm

https://codereview.chromium.org/603393004/diff/1/test/cctest/test-strings.cc
File test/cctest/test-strings.cc (right):

https://codereview.chromium.org/603393004/diff/1/test/cctest/test-strings.cc#...
test/cctest/test-strings.cc:1380: uint16_t string_[10];
Length 1 should be enough

https://codereview.chromium.org/603393004/diff/1/test/cctest/test-strings.cc#...
test/cctest/test-strings.cc:1390: char string_[10];
ditto

[loislo, 2014-09-26 10:27:14]

comments addressed

[Yang, 2014-09-26 10:28:09]

On 2014/09/26 10:27:14, loislo wrote:
> comments addressed

LGTM. Thanks!

[loislo, 2014-09-26 11:14:17]

Committed patchset #2 (id:20001) manually as 24250 (tree was closed).