New issue
Advanced search Search tips

Issue 876608 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 23
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 816352
issue 874055



Sign in to add a comment

bindings: IDL 'object' type should be validated in converting into native object

Project Member Reported by peria@chromium.org, Aug 22

Issue description

For 'object' type objects in IDL, we generate following code in our IDL compiler;

ScriptValue value = ScriptValue(ScriptState::Current(info.GetIsolate()), info[i]);

It means we don't validate if the V8 value is Object or not.
We should check it at binding layer and throw an error if the argument (info[i]) is not an Object.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 23

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0a53b73ec4e8e80c4f34d0a90625773b8affbdf4

commit 0a53b73ec4e8e80c4f34d0a90625773b8affbdf4
Author: Hitoshi Yoshida <peria@chromium.org>
Date: Thu Aug 23 06:09:56 2018

bindings: Check object type of IDL in arugments

Before this CL, we did not check if a passed value
for `object' argument is actually v8::Object.
It could trigger null references in case non-Object
values are passed into the arguments.
(non-Object values are taken as null objects in Blink.)

This CL adds a validation to check if the passed value
is a v8::Object or not when a non-null value is
passed into an `object` argument.


Bug:  876608 
Change-Id: Iedb35a3deee781ac4bbaddc0afcc8e7c25f0c6eb
Reviewed-on: https://chromium-review.googlesource.com/1184593
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Hitoshi Yoshida <peria@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585412}
[modify] https://crrev.com/0a53b73ec4e8e80c4f34d0a90625773b8affbdf4/third_party/blink/renderer/bindings/templates/methods.cpp.tmpl
[modify] https://crrev.com/0a53b73ec4e8e80c4f34d0a90625773b8affbdf4/third_party/blink/renderer/bindings/tests/idls/core/test_interface.idl
[modify] https://crrev.com/0a53b73ec4e8e80c4f34d0a90625773b8affbdf4/third_party/blink/renderer/bindings/tests/results/core/v8_test_interface.cc
[modify] https://crrev.com/0a53b73ec4e8e80c4f34d0a90625773b8affbdf4/third_party/blink/renderer/bindings/tests/results/core/v8_test_interface.h

Status: Fixed (was: Started)
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 24

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8698158ab1b36d545956fac3024f5f82a6470042

commit 8698158ab1b36d545956fac3024f5f82a6470042
Author: Hitoshi Yoshida <peria@chromium.org>
Date: Fri Aug 24 05:10:47 2018

bindings: Change order of arguments in code generater test

This is a follow-up CL for http://crrev.com/585412

We can't find a specific description in spec, but it is natural
to put optional arguments after all non-optional arguments.

This changes the order of arguments in a code generator test case
from: method(arg1, optional arg2, arg3)
to  : method(arg1, arg2, optional arg3)


Bug:  876608 
Change-Id: Ief33bbdeef7fefc832a03d78b5ca28ab68aa63f2
Reviewed-on: https://chromium-review.googlesource.com/1187847
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Hitoshi Yoshida <peria@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585690}
[modify] https://crrev.com/8698158ab1b36d545956fac3024f5f82a6470042/third_party/blink/renderer/bindings/tests/idls/core/test_interface.idl
[modify] https://crrev.com/8698158ab1b36d545956fac3024f5f82a6470042/third_party/blink/renderer/bindings/tests/results/core/v8_test_interface.cc

Sign in to add a comment