Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2739833002:
[Bindings] Make both installMethodInternal's check same conditions (Closed)
DescriptionMake two installMethodInternal's check same conditions.
Before this CL, installMethodInternal() for objects did not check
method.accessCheckConfiguration, and it was inconsistent with
installMethodInternal() for templates.
This CL adds a check to do same confirmations.
BUG=617892
Review-Url: https://codereview.chromium.org/2739833002
Cr-Commit-Position: refs/heads/master@{#455415}
Committed: https://chromium.googlesource.com/chromium/src/+/1d7e887175cca415da1ce23eea7ca52be0adacbf
Patch Set 1 #
Messages
Total messages: 18 (9 generated)
Description was changed from ========== Check if a newly added method is cross-origin BUG=617892 ========== to ========== Make two installMethodInternal's check same conditions. Before this CL, installMethodInternal() for objects did not check method.accessCheckConfiguration, and it was inconsistent with installMethodInternal() for templates. This CL adds a check to do same confirmations. BUG=617892 ==========
The CQ bit was checked by peria@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
peria@chromium.org changed reviewers: + haraken@chromium.org, yukishiino@chromium.org
PTL. Current code on trunk does not need this check, but it'll be needed in snapshot.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
On 2017/03/08 06:32:11, peria wrote: > PTL. > > Current code on trunk does not need this check, but it'll be > needed in snapshot. Would you elaborate on it? The change looks good but I want to understand what this CL is going to change.
LGTM.
On 2017/03/08 07:17:53, haraken wrote: > Would you elaborate on it? The change looks good but I want to understand what > this CL is going to change. On my test failure, for example, a runtime enabled method window.openDatabase() should throw a cross-origin exception if it is called from a different host. But in snapshotted context, installed after deserialization, does not confirm its condition and executes successfully. This case is tested here; https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/http/test... And this CL fixes it.
On 2017/03/08 07:56:30, peria wrote: > On 2017/03/08 07:17:53, haraken wrote: > > Would you elaborate on it? The change looks good but I want to understand what > > this CL is going to change. > > On my test failure, for example, a runtime enabled method window.openDatabase() > should throw a cross-origin exception if it is called from a different host. > But in snapshotted context, installed after deserialization, does not confirm > its condition and executes successfully. > > This case is tested here; > https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/http/test... > > And this CL fixes it. dcheng changed the security check to use SetAcceptAnyReceiver more and simplifies it. This is a leftover. He updated installMethodInternal for FunctionTemplate version corectly, but forgot Function version.
On 2017/03/08 08:11:23, Yuki(slow_til_mar08) wrote: > On 2017/03/08 07:56:30, peria wrote: > > On 2017/03/08 07:17:53, haraken wrote: > > > Would you elaborate on it? The change looks good but I want to understand > what > > > this CL is going to change. > > > > On my test failure, for example, a runtime enabled method > window.openDatabase() > > should throw a cross-origin exception if it is called from a different host. > > But in snapshotted context, installed after deserialization, does not confirm > > its condition and executes successfully. > > > > This case is tested here; > > > https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/http/test... > > > > And this CL fixes it. > > dcheng changed the security check to use SetAcceptAnyReceiver more and > simplifies it. This is a leftover. He updated installMethodInternal for > FunctionTemplate version corectly, but forgot Function version. Makes sense. LGTM.
The CQ bit was checked by peria@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2017/03/08 08:11:23, Yuki(slow_til_mar08) wrote: > On 2017/03/08 07:56:30, peria wrote: > > On 2017/03/08 07:17:53, haraken wrote: > > > Would you elaborate on it? The change looks good but I want to understand > what > > > this CL is going to change. > > > > On my test failure, for example, a runtime enabled method > window.openDatabase() > > should throw a cross-origin exception if it is called from a different host. > > But in snapshotted context, installed after deserialization, does not confirm > > its condition and executes successfully. > > > > This case is tested here; > > > https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/http/test... > > > > And this CL fixes it. > > dcheng changed the security check to use SetAcceptAnyReceiver more and > simplifies it. This is a leftover. He updated installMethodInternal for > FunctionTemplate version corectly, but forgot Function version. Oh, thank you for this information.
CQ is committing da patch.
Bot data: {"patchset_id": 1, "attempt_start_ts": 1488962269071700, "parent_rev":
"a120b02dfcaecf58c41dd1ab19baac901249b197", "commit_rev":
"1d7e887175cca415da1ce23eea7ca52be0adacbf"}
Message was sent while issue was closed.
Description was changed from ========== Make two installMethodInternal's check same conditions. Before this CL, installMethodInternal() for objects did not check method.accessCheckConfiguration, and it was inconsistent with installMethodInternal() for templates. This CL adds a check to do same confirmations. BUG=617892 ========== to ========== Make two installMethodInternal's check same conditions. Before this CL, installMethodInternal() for objects did not check method.accessCheckConfiguration, and it was inconsistent with installMethodInternal() for templates. This CL adds a check to do same confirmations. BUG=617892 Review-Url: https://codereview.chromium.org/2739833002 Cr-Commit-Position: refs/heads/master@{#455415} Committed: https://chromium.googlesource.com/chromium/src/+/1d7e887175cca415da1ce23eea7c... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) as https://chromium.googlesource.com/chromium/src/+/1d7e887175cca415da1ce23eea7c... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
