Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(10)

Issue 2671553006: Allow global prototype to be a Proxy (Closed)

Created:
2 years, 5 months ago by Bradley Farias
Modified:
2 years, 3 months ago
Reviewers:
bradley.meck, caitp, Dan Ehrenberg, Toon Verwaest
CC:
v8-reviews_googlegroups.com
Target Ref:
refs/heads/master
Project:
v8
Visibility:
Public.

Description

Allow global prototype to be a Proxy There was a security issue from allowing WindowProxy to have its prototype mutated in https://bugs.chromium.org/p/chromium/issues/detail?id=399951 . This has since been resolved by making `window`, `location`, and `Object.prototype.__proto__` immutable. This change was fixed in https://bugs.chromium.org/p/v8/issues/detail?id=5149 . Reverts https://codereview.chromium.org/1529303003 R=littledan@chromium.org R=verwaest@chromium.org Review-Url: https://codereview.chromium.org/2671553006 Cr-Commit-Position: refs/heads/master@{#44078} Committed: https://chromium.googlesource.com/v8/v8/+/b123ee34111f643fec6f7b662912deafd1165e3e

Patch Set 1 #

Total comments: 3

Patch Set 2 : Allow global prototype to be a Proxy #

Patch Set 3 : Allow global prototype to be a Proxy #

Total comments: 1

Patch Set 4 : Allow global prototype to be a Proxy #

Patch Set 5 : update bytecode #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+50 lines, -16 lines) Patch
M src/messages.h View 1 2 3 1 chunk +0 lines, -1 line 0 comments Download
M src/objects.cc View 1 2 3 1 chunk +0 lines, -6 lines 0 comments Download
M test/cctest/interpreter/bytecode_expectations/ForAwaitOf.golden View 1 2 3 4 4 chunks +4 lines, -4 lines 1 comment Download
M test/cctest/interpreter/bytecode_expectations/ForOf.golden View 1 2 3 4 4 chunks +4 lines, -4 lines 0 comments Download
M test/cctest/interpreter/bytecode_expectations/Generators.golden View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
A test/mjsunit/es6/global-proto-proxy.js View 1 2 1 chunk +41 lines, -0 lines 0 comments Download

Messages

Total messages: 22 (9 generated)
caitp
Could you please add a line in the CL description stating that the reason for ...
2 years, 5 months ago (2017-02-03 22:28:45 UTC) #2
caitp
https://codereview.chromium.org/2671553006/diff/1/test/mjsunit/es6/global-proto-proxy.js File test/mjsunit/es6/global-proto-proxy.js (right): https://codereview.chromium.org/2671553006/diff/1/test/mjsunit/es6/global-proto-proxy.js#newcode26 test/mjsunit/es6/global-proto-proxy.js:26: // OF THIS SOFTWARE, EVEN IF ADVISED OF THE ...
2 years, 5 months ago (2017-02-03 22:41:31 UTC) #4
Bradley Farias
On 2017/02/03 22:41:31, caitp wrote: > https://codereview.chromium.org/2671553006/diff/1/test/mjsunit/es6/global-proto-proxy.js > File test/mjsunit/es6/global-proto-proxy.js (right): > > https://codereview.chromium.org/2671553006/diff/1/test/mjsunit/es6/global-proto-proxy.js#newcode26 > ...
2 years, 5 months ago (2017-02-03 22:50:20 UTC) #5
Dan Ehrenberg
Thanks for coming back and revisiting this! A couple optional nits: - Could you word-wrap ...
2 years, 5 months ago (2017-02-05 10:25:34 UTC) #6
Toon Verwaest
https://codereview.chromium.org/2671553006/diff/40001/src/objects.cc File src/objects.cc (left): https://codereview.chromium.org/2671553006/diff/40001/src/objects.cc#oldcode1083 src/objects.cc:1083: NewTypeError(MessageTemplate::kReadGlobalReferenceThroughProxy, name), What about removing the message too?
2 years, 5 months ago (2017-02-08 16:47:52 UTC) #7
Bradley Farias
On 2017/02/08 16:47:52, Toon Verwaest wrote: > https://codereview.chromium.org/2671553006/diff/40001/src/objects.cc > File src/objects.cc (left): > > https://codereview.chromium.org/2671553006/diff/40001/src/objects.cc#oldcode1083 ...
2 years, 4 months ago (2017-03-16 19:24:46 UTC) #8
Toon Verwaest
lgtm, thanks!
2 years, 4 months ago (2017-03-17 13:31:10 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2671553006/60001
2 years, 4 months ago (2017-03-21 17:19:08 UTC) #11
commit-bot: I haz the power
Try jobs failed on following builders: v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/builds/15218) v8_linux64_gyp_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED, ...
2 years, 4 months ago (2017-03-21 17:37:42 UTC) #13
Bradley Farias
On 2017/03/21 17:37:42, commit-bot: I haz the power wrote: > Try jobs failed on following ...
2 years, 3 months ago (2017-03-22 21:43:26 UTC) #14
caitp
ditto for the other bytecode expectations changes. https://codereview.chromium.org/2671553006/diff/80001/test/cctest/interpreter/bytecode_expectations/ForAwaitOf.golden File test/cctest/interpreter/bytecode_expectations/ForAwaitOf.golden (right): https://codereview.chromium.org/2671553006/diff/80001/test/cctest/interpreter/bytecode_expectations/ForAwaitOf.golden#newcode220 test/cctest/interpreter/bytecode_expectations/ForAwaitOf.golden:220: B(Wide), B(LdaSmi), ...
2 years, 3 months ago (2017-03-23 18:00:53 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2671553006/80001
2 years, 3 months ago (2017-03-23 18:08:44 UTC) #19
commit-bot: I haz the power
2 years, 3 months ago (2017-03-23 18:33:45 UTC) #22
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/v8/v8/+/b123ee34111f643fec6f7b662912deafd11...

Powered by Google App Engine
This is Rietveld 408576698