Bug 91707 - Implement the experimental Content Security Policy script interface.
: Implement the experimental Content Security Policy script interface.
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: WebCore Misc.
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To: Mike West
: WebExposed
Depends on:
Blocks: 85558
  Show dependency treegraph
 
Reported: 2012-07-18 20:25 PDT by Mike West
Modified: 2012-07-26 02:50 PDT (History)
11 users (show)

See Also:


Attachments
Patch (76.23 KB, patch)
2012-07-18 21:43 PDT, Mike West
no flags Details | Formatted Diff | Diff
Adam's feedback + GTK, still no XCode. (79.75 KB, patch)
2012-07-19 10:01 PDT, Mike West
no flags Details | Formatted Diff | Diff
AdGTK again. I hate XCode. (79.96 KB, patch)
2012-07-20 08:56 PDT, Mike West
no flags Details | Formatted Diff | Diff
XCode is happy. I think. (87.55 KB, patch)
2012-07-20 13:58 PDT, Mike West
no flags Details | Formatted Diff | Diff
Patch for landing. (87.63 KB, patch)
2012-07-26 01:21 PDT, Mike West
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mike West 2012-07-18 20:25:50 PDT
CSP 1.1 defines a script interface meant to give developers the ability to query the currently active policy for details about the context in which their scripts and other resources will be loaded. See https://dvcs.w3.org/hg/content-security-policy/raw-file/tip/csp-specification.dev.html#script-interfaces--experimental for details.
Comment 1 Mike West 2012-07-18 21:43:07 PDT
Created attachment 153176 [details]
Patch
Comment 2 Mike West 2012-07-18 21:46:29 PDT
Dropping the r?, as this can't land as-is. I still need to add the files to the xcode project, and I'm having a devil of a time doing so.

Still, if you wouldn't mind having a look, Adam, I'd appreciate it. This is more or less the same patch I've been running by you on GitHub for a while now, rebased on top of trunk.
Comment 3 Adam Barth 2012-07-18 21:58:13 PDT
(In reply to comment #2)
> Dropping the r?, as this can't land as-is. I still need to add the files to the xcode project, and I'm having a devil of a time doing so.

You basically need a Mac for that.

> Still, if you wouldn't mind having a look, Adam, I'd appreciate it. This is more or less the same patch I've been running by you on GitHub for a while now, rebased on top of trunk.

Sure.
Comment 4 Build Bot 2012-07-18 22:00:15 PDT
Comment on attachment 153176 [details]
Patch

Attachment 153176 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13279798
Comment 5 Adam Barth 2012-07-18 22:01:47 PDT
Comment on attachment 153176 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=153176&action=review

Looks pretty good.  Weaving the sendReport everywhere is kind of ugly.  Maybe using an enum would have better readability at the call sites?

> Source/WebCore/page/DOMSecurityPolicy.idl:30
> +        InterfaceName=securityPolicy

securityPolicy -> SecurityPolicy
Comment 6 Mike West 2012-07-18 22:26:34 PDT
Thanks! If/when I can get WebKit checked out on my laptop, I'll keep fiddling with the xcode file.

(In reply to comment #5)
> (From update of attachment 153176 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=153176&action=review
> 
> Looks pretty good.  Weaving the sendReport everywhere is kind of ugly.  Maybe using an enum would have better readability at the call sites?

It is ugly. While rebasing I was wondering again whether it made sense. I think it does, but it's ugly.

I'm not sure an enum makes it less ugly, but it probably would make it more readable.

> 
> > Source/WebCore/page/DOMSecurityPolicy.idl:30
> > +        InterfaceName=securityPolicy
> 
> securityPolicy -> SecurityPolicy

Will do.
Comment 7 Gyuyoung Kim 2012-07-19 00:21:41 PDT
In my humble opinion, it looks you missed to touch below file for GTK port.

http://trac.webkit.org/browser/trunk/Source/WebCore/bindings/gobject/GNUmakefile.am

For example, as below,

+       DerivedSources/webkit/WebKitDOMDOMSecurityPolicy.h \
+       DerivedSources/webkit/WebKitDOMDOMSecurityPolicyPrivate.h \

@@ -277,6 +279,7 @@ webkitgtk_built_h_api += \
        DerivedSources/webkit/WebKitDOMDocumentFragment.h \
        DerivedSources/webkit/WebKitDOMDocumentType.h \
        DerivedSources/webkit/WebKitDOMDOMImplementation.h \
+       DerivedSources/webkit/WebKitDOMDOMSecurityPolicy.h \
Comment 8 Mike West 2012-07-19 10:01:54 PDT
Created attachment 153285 [details]
Adam's feedback + GTK, still no XCode.
Comment 9 Mike West 2012-07-19 10:03:35 PDT
(In reply to comment #7)
> In my humble opinion, it looks you missed to touch below file for GTK port.
> 
> http://trac.webkit.org/browser/trunk/Source/WebCore/bindings/gobject/GNUmakefile.am
> 
> For example, as below,
> 
> +       DerivedSources/webkit/WebKitDOMDOMSecurityPolicy.h \
> +       DerivedSources/webkit/WebKitDOMDOMSecurityPolicyPrivate.h \
> 
> @@ -277,6 +279,7 @@ webkitgtk_built_h_api += \
>         DerivedSources/webkit/WebKitDOMDocumentFragment.h \
>         DerivedSources/webkit/WebKitDOMDocumentType.h \
>         DerivedSources/webkit/WebKitDOMDOMImplementation.h \
> +       DerivedSources/webkit/WebKitDOMDOMSecurityPolicy.h \

Thank you! I've added these to the file (though it looks like I need to add `DerivedSources/webkit/WebKitDOMDOMSecurityPolicy.cpp`, based on what others have done). Can you take a look at the current patch and verify that I've done it correctly?
Comment 10 Build Bot 2012-07-19 10:25:04 PDT
Comment on attachment 153285 [details]
Adam's feedback + GTK, still no XCode.

Attachment 153285 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13272889
Comment 11 Mike West 2012-07-20 08:56:43 PDT
Created attachment 153517 [details]
AdGTK again. I hate XCode.
Comment 12 Mike West 2012-07-20 09:07:21 PDT
(In reply to comment #11)
> Created an attachment (id=153517) [details]
> AdGTK again. I hate XCode.

I've been kicking around with XCode for a few hours, and all I have to show for it are an ever-widening variety of error messages. It's driving me nuts.

Can you give me some pointers as to exactly where I need to add these files? Or should I hop over to webkit-dev for help?
Comment 13 Adam Barth 2012-07-20 09:08:45 PDT
> I've been kicking around with XCode for a few hours, and all I have to show for it are an ever-widening variety of error messages. It's driving me nuts.

What do the error messages look like?
Comment 14 Mike West 2012-07-20 09:26:50 PDT
(In reply to comment #13)
> > I've been kicking around with XCode for a few hours, and all I have to show for it are an ever-widening variety of error messages. It's driving me nuts.
> 
> What do the error messages look like?

I started with:

In file included from /Users/mkwst/Repositories/webkit/WebKitBuild/Debug/DerivedSources/WebCore/DOMDocument.mm:48:
/Users/mkwst/Repositories/webkit/WebKitBuild/Debug/DerivedSources/WebCore/DOMDOMSecurityPolicyInternal.h:27:9: fatal error: 'WebCore/DOMDOMSecurityPolicy.h' file not found [2]
 #import <WebCore/DOMDOMSecurityPolicy.h>

And the most recent failure at resolving that was:

PBXCp /Users/mkwst/Repositories/webkit/WebKitBuild/Debug/DerivedSources/WebCore/../../../../../../WebKitBuild/Debug/DerivedSources/WebCore/DOMDOMSecurityPolicy.h /Users/mkwst/Repositories/webkit/WebKitBuild/Debug/WebCore.framework/Versions/A/PrivateHeaders/DOMDOMSecurityPolicy.h
    cd /Users/mkwst/Repositories/webkit/Source/WebCore
    builtin-copy -exclude .DS_Store -exclude CVS -exclude .svn -exclude .git -exclude .hg -strip-debug-symbols -resolve-src-symlinks /Users/mkwst/Repositories/webkit/WebKitBuild/Debug/DerivedSources/WebCore/../../../../../../WebKitBuild/Debug/DerivedSources/WebCore/DOMDOMSecurityPolicy.h /Users/mkwst/Repositories/webkit/WebKitBuild/Debug/WebCore.framework/Versions/A/PrivateHeaders
error: /Users/mkwst/Repositories/webkit/WebKitBuild/Debug/DerivedSources/WebCore/../../../../../../WebKitBuild/Debug/DerivedSources/WebCore/DOMDOMSecurityPolicy.h: No such file or directory

Basically, adding the three files I actually wrote seems to be no problem. I'm failing to add the generated files.
Comment 15 Adam Barth 2012-07-20 09:31:47 PDT
DOMDOMSecurityPolicy.h is related to the Objective-C bindings.  You might need to add the header to bindings/objc/DerivedSources in the Xcode project file.
Comment 16 Mike West 2012-07-20 09:55:45 PDT
(In reply to comment #15)
> DOMDOMSecurityPolicy.h is related to the Objective-C bindings.  You might need to add the header to bindings/objc/DerivedSources in the Xcode project file.

I love you. :)

It looks like it's compiling. If it continues to look like it's compiling, I'll upload a patch for review.

Thanks!
Comment 17 Mike West 2012-07-20 13:58:32 PDT
Created attachment 153592 [details]
XCode is happy. I think.
Comment 18 Mike West 2012-07-26 00:16:48 PDT
(In reply to comment #17)
> Created an attachment (id=153592) [details]
> XCode is happy. I think.

Friendly ping. :) This patch now compiles cleanly under XCode, and I've swapped out the boolean flag for an enum. Would you mind taking another look, Adam?

Thanks!
Comment 19 Adam Barth 2012-07-26 00:32:54 PDT
Comment on attachment 153592 [details]
XCode is happy. I think.

View in context: https://bugs.webkit.org/attachment.cgi?id=153592&action=review

> Source/WebCore/page/DOMSecurityPolicy.cpp:37
> +

extra blank line here.

> Source/WebCore/page/DOMSecurityPolicy.h:63
> +    bool allowsConnectionTo(String url) const;
> +    bool allowsFontFrom(String url) const;
> +    bool allowsFrameFrom(String url) const;
> +    bool allowsImageFrom(String url) const;
> +    bool allowsMediaFrom(String url) const;
> +    bool allowsObjectFrom(String url) const;
> +    bool allowsScriptFrom(String url) const;
> +    bool allowsStyleFrom(String url) const;

These should all be const String&

> Source/WebCore/page/DOMSecurityPolicy.idl:30
> +        InterfaceName=securityPolicy

securityPolicy -> SecurityPolicy
Comment 20 Mike West 2012-07-26 01:21:47 PDT
Created attachment 154574 [details]
Patch for landing.

Thanks, Adam.
Comment 21 Adam Barth 2012-07-26 01:55:00 PDT
Comment on attachment 154574 [details]
Patch for landing.

Aren't you a committer?
Comment 22 Adam Barth 2012-07-26 01:55:12 PDT
You can set the commit-queue flag yourself.  :)
Comment 23 Adam Barth 2012-07-26 01:56:19 PDT
Maybe your paperwork hasn't gone through yet?
Comment 24 WebKit Review Bot 2012-07-26 02:50:16 PDT
Comment on attachment 154574 [details]
Patch for landing.

Clearing flags on attachment: 154574

Committed r123722: <http://trac.webkit.org/changeset/123722>
Comment 25 WebKit Review Bot 2012-07-26 02:50:26 PDT
All reviewed patches have been landed.  Closing bug.