Bug 91707 - Implement the experimental Content Security Policy script interface.
: Implement the experimental Content Security Policy script interface.
Status: RESOLVED FIXED
: WebKit
WebCore Misc.
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To:
:
: WebExposed
:
: 85558
  Show dependency treegraph
 
Reported: 2012-07-18 20:25 PST by
Modified: 2012-07-26 02:50 PST (History)


Attachments
Patch (76.23 KB, patch)
2012-07-18 21:43 PST, Mike West
no flags Review Patch | Details | Formatted Diff | Diff
Adam's feedback + GTK, still no XCode. (79.75 KB, patch)
2012-07-19 10:01 PST, Mike West
no flags Review Patch | Details | Formatted Diff | Diff
AdGTK again. I hate XCode. (79.96 KB, patch)
2012-07-20 08:56 PST, Mike West
no flags Review Patch | Details | Formatted Diff | Diff
XCode is happy. I think. (87.55 KB, patch)
2012-07-20 13:58 PST, Mike West
no flags Review Patch | Details | Formatted Diff | Diff
Patch for landing. (87.63 KB, patch)
2012-07-26 01:21 PST, Mike West
no flags 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 2012-07-18 20:25:50 PST
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 From 2012-07-18 21:43:07 PST -------
Created an attachment (id=153176) [details]
Patch
------- Comment #2 From 2012-07-18 21:46:29 PST -------
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 From 2012-07-18 21:58:13 PST -------
(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 From 2012-07-18 22:00:15 PST -------
(From update of attachment 153176 [details])
Attachment 153176 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13279798
------- Comment #5 From 2012-07-18 22:01:47 PST -------
(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?

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

securityPolicy -> SecurityPolicy
------- Comment #6 From 2012-07-18 22:26:34 PST -------
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] [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 From 2012-07-19 00:21:41 PST -------
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 From 2012-07-19 10:01:54 PST -------
Created an attachment (id=153285) [details]
Adam's feedback + GTK, still no XCode.
------- Comment #9 From 2012-07-19 10:03:35 PST -------
(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 From 2012-07-19 10:25:04 PST -------
(From update of attachment 153285 [details])
Attachment 153285 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13272889
------- Comment #11 From 2012-07-20 08:56:43 PST -------
Created an attachment (id=153517) [details]
AdGTK again. I hate XCode.
------- Comment #12 From 2012-07-20 09:07:21 PST -------
(In reply to comment #11)
> Created an attachment (id=153517) [details] [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 From 2012-07-20 09:08:45 PST -------
> 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 From 2012-07-20 09:26:50 PST -------
(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 From 2012-07-20 09:31:47 PST -------
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 From 2012-07-20 09:55:45 PST -------
(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 From 2012-07-20 13:58:32 PST -------
Created an attachment (id=153592) [details]
XCode is happy. I think.
------- Comment #18 From 2012-07-26 00:16:48 PST -------
(In reply to comment #17)
> Created an attachment (id=153592) [details] [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 From 2012-07-26 00:32:54 PST -------
(From update of attachment 153592 [details])
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 From 2012-07-26 01:21:47 PST -------
Created an attachment (id=154574) [details]
Patch for landing.

Thanks, Adam.
------- Comment #21 From 2012-07-26 01:55:00 PST -------
(From update of attachment 154574 [details])
Aren't you a committer?
------- Comment #22 From 2012-07-26 01:55:12 PST -------
You can set the commit-queue flag yourself.  :)
------- Comment #23 From 2012-07-26 01:56:19 PST -------
Maybe your paperwork hasn't gone through yet?
------- Comment #24 From 2012-07-26 02:50:16 PST -------
(From update of attachment 154574 [details])
Clearing flags on attachment: 154574

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