Bug 33182 - [V8] Move security checks for setting src to javascript url out of V8Custom
: [V8] Move security checks for setting src to javascript url out of V8Custom
Status: RESOLVED FIXED
: WebKit
WebCore Misc.
: 528+ (Nightly build)
: PC Mac OS X 10.5
: P2 Normal
Assigned To:
:
:
:
: 32638
  Show dependency treegraph
 
Reported: 2010-01-04 15:10 PST by
Modified: 2010-01-05 11:06 PST (History)


Attachments
patch (14.50 KB, patch)
2010-01-04 15:51 PST, Nate Chapin
japhet: commit‑queue-
Review Patch | Details | Formatted Diff | Diff
patch minus style errors (14.48 KB, patch)
2010-01-04 15:58 PST, Nate Chapin
abarth: review-
japhet: commit‑queue-
Review Patch | Details | Formatted Diff | Diff
Functions are now in BindingSecurity.h. (9.82 KB, patch)
2010-01-05 10:16 PST, Nate Chapin
abarth: review+
japhet: commit‑queue-
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2010-01-04 15:10:06 PST
Ideally, these security checks would be in some common part of the custom bindings that all descendants of V8Element had access to.  Unfortunately, there doesn't seem to be such a place, and V8AttrCustom.cpp needs to have access to these checks as well.  Therefore, these should probably go in V8BindingSecurity (which will need to be expanded to something more than a typedef).
------- Comment #1 From 2010-01-04 15:51:03 PST -------
Created an attachment (id=45842) [details]
patch
------- Comment #2 From 2010-01-04 15:52:55 PST -------
Attachment 45842 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/ChangeLog:7:  Line contains tab character.  [whitespace/tab] [5]
WebCore/bindings/v8/V8BindingSecurity.h:68:  Could not find a newline character at the end of the file.  [whitespace/ending_newline] [5]
Total errors found: 2
------- Comment #3 From 2010-01-04 15:58:47 PST -------
Created an attachment (id=45843) [details]
patch minus style errors
------- Comment #4 From 2010-01-04 16:03:22 PST -------
style-queue ran check-webkit-style on attachment 45843 [details] without any errors.
------- Comment #5 From 2010-01-04 17:27:28 PST -------
(From update of attachment 45843 [details])
Sorry, I think i steered you slightly wrong here.  This code can go in the generic BindingSecurity object since it's not dependent on V8:

http://trac.webkit.org/browser/trunk/WebCore/bindings/BindingSecurity.h

That way the code will be shared with JSC eventually.
------- Comment #6 From 2010-01-04 19:15:34 PST -------
(In reply to comment #5)
> (From update of attachment 45843 [details] [details])
> Sorry, I think i steered you slightly wrong here.  This code can go in the
> generic BindingSecurity object since it's not dependent on V8:
> 
> http://trac.webkit.org/browser/trunk/WebCore/bindings/BindingSecurity.h
> 
> That way the code will be shared with JSC eventually.

That'll require a State<Binding> parameter then, right?  Is that ok?  I don't have a great idea of how the State is used.
------- Comment #7 From 2010-01-05 10:16:33 PST -------
Created an attachment (id=45902) [details]
Functions are now in BindingSecurity.h.

I'm not sure whether these functions belong in BindingSecurity.h or whether we should add BindingSecurity.cpp, so I defaulted to not adding a new file.
------- Comment #8 From 2010-01-05 10:17:26 PST -------
style-queue ran check-webkit-style on attachment 45902 [details] without any errors.
------- Comment #9 From 2010-01-05 11:01:26 PST -------
(From update of attachment 45902 [details])
This looks great.  Unfortunately, these functions need to be in the header because of the template parameter.

I'm about to land a change that moves this file, so we're going to conflict.  I can land this if you'd rather not deal with the merge conflict.
------- Comment #10 From 2010-01-05 11:06:39 PST -------
Committed r52812: <http://trac.webkit.org/changeset/52812>