WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
106668
Web Inspector: Make V8 LiveEdit API disabled by default
https://bugs.webkit.org/show_bug.cgi?id=106668
Summary
Web Inspector: Make V8 LiveEdit API disabled by default
Peter Rybin
Reported
2013-01-11 08:37:54 PST
LiveEdit is a sophisticated routine in V8, and potentially it may have some vulnerabilities. Enable C-level blocks in V8 core, that won't allow LiveEdit code unless actually called LiveEdit command. Related Chromium bug:
https://code.google.com/p/chromium/issues/detail?id=159124
Attachments
Patch
(1.36 KB, patch)
2013-01-11 08:41 PST
,
Peter Rybin
no flags
Details
Formatted Diff
Diff
Patch
(2.42 KB, patch)
2013-01-11 09:53 PST
,
Peter Rybin
no flags
Details
Formatted Diff
Diff
Patch
(2.53 KB, patch)
2013-01-18 12:37 PST
,
Peter Rybin
no flags
Details
Formatted Diff
Diff
Patch
(2.57 KB, patch)
2013-01-19 11:07 PST
,
Peter Rybin
no flags
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Peter Rybin
Comment 1
2013-01-11 08:41:59 PST
Created
attachment 182349
[details]
Patch
Peter Rybin
Comment 2
2013-01-11 08:42:36 PST
This is a follow-up for
https://bugs.webkit.org/show_bug.cgi?id=104039
WebKit Review Bot
Comment 3
2013-01-11 08:45:55 PST
Comment on
attachment 182349
[details]
Patch
Attachment 182349
[details]
did not pass chromium-ews (chromium-xvfb): Output:
http://queues.webkit.org/results/15808414
Vsevolod Vlasov
Comment 4
2013-01-11 09:14:01 PST
Some thoughts: 1. I would consider setting live_edit_enabled_ to false in Debugger constructor. It might break other existing v8 embedders though. 2. If we don't want to change v8 API in this sense, I would expect to see this in WebCore v8 bindings, where other setLiveEditEnabled calls reside.
Peter Beverloo (cr-android ews)
Comment 5
2013-01-11 09:25:44 PST
Comment on
attachment 182349
[details]
Patch
Attachment 182349
[details]
did not pass cr-android-ews (chromium-android): Output:
http://queues.webkit.org/results/15811350
Peter Rybin
Comment 6
2013-01-11 09:53:59 PST
Created
attachment 182358
[details]
Patch
Adam Barth
Comment 7
2013-01-11 10:36:51 PST
Comment on
attachment 182358
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=182358&action=review
> Source/WebCore/bindings/v8/V8Initializer.cpp:108 > +// V8 initialization part common for both Document and Worker.
This comment doesn't really add anything. Perhaps we should remove it?
> Source/WebCore/bindings/v8/V8Initializer.cpp:125 > v8::V8::IgnoreOutOfMemoryException();
This call looks shared by both Document and worker, perhaps we should move it into initializeV8Common as well. (There look to be a couple others that we could share as well.)
Adam Barth
Comment 8
2013-01-11 10:39:27 PST
This seems like something that's better done in V8, but we can do it here if we don't want to change the V8 API in this way.
Peter Rybin
Comment 9
2013-01-14 08:35:16 PST
Non-liveedit-related changes are moved into a separate prerequisite change:
https://bugs.webkit.org/show_bug.cgi?id=106790
Peter Rybin
Comment 10
2013-01-18 12:37:50 PST
Created
attachment 183529
[details]
Patch
Peter Rybin
Comment 11
2013-01-18 12:46:49 PST
Moving the default value into V8 is not quite elegant, because it won't spare us from changes anyway, but would require all other V8 embedders to manually change the default value back without any pay-off for them (you only can use enable/disable, if you wrote the entire debug command dispatcher in C++ like WebKit did). All other comments are addresses.
WebKit Review Bot
Comment 12
2013-01-18 23:21:21 PST
Comment on
attachment 183529
[details]
Patch Rejecting
attachment 183529
[details]
from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1 cwd: /mnt/git/webkit-commit-queue /mnt/git/webkit-commit-queue/Source/WebCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Full output:
http://queues.webkit.org/results/15973076
Yury Semikhatsky
Comment 13
2013-01-18 23:26:01 PST
(In reply to
comment #12
)
> (From update of
attachment 183529
[details]
) > Rejecting
attachment 183529
[details]
from commit-queue. > > Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1 cwd: /mnt/git/webkit-commit-queue > > /mnt/git/webkit-commit-queue/Source/WebCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). > > Full output:
http://queues.webkit.org/results/15973076
Peter you'll need to restore "Reviewed by..." line in the ChangeLog in order commit queue to understand it.
Peter Rybin
Comment 14
2013-01-19 11:07:00 PST
Created
attachment 183629
[details]
Patch
Peter Rybin
Comment 15
2013-01-19 11:10:27 PST
> Peter you'll need to restore "Reviewed by..." line in the ChangeLog in order commit queue to understand it.
Thanks! My bad. Oh bureaucracy!.. :)
WebKit Review Bot
Comment 16
2013-01-20 10:19:41 PST
Comment on
attachment 183629
[details]
Patch Clearing flags on attachment: 183629 Committed
r140273
: <
http://trac.webkit.org/changeset/140273
>
WebKit Review Bot
Comment 17
2013-01-20 10:19:46 PST
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug