Bug 112681 - CSP 1.1: Stub out SecurityPolicyViolationEvent interface.
Summary: CSP 1.1: Stub out SecurityPolicyViolationEvent interface.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mike West
URL:
Keywords:
Depends on:
Blocks: 85558
  Show dependency treegraph
 
Reported: 2013-03-19 04:19 PDT by Mike West
Modified: 2013-03-19 23:25 PDT (History)
7 users (show)

See Also:


Attachments
Patch (29.27 KB, patch)
2013-03-19 07:01 PDT, Mike West
no flags Details | Formatted Diff | Diff
Patch (26.99 KB, patch)
2013-03-19 11:01 PDT, Mike West
no flags Details | Formatted Diff | Diff
Patch (26.45 KB, patch)
2013-03-19 14:17 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 2013-03-19 04:19:44 PDT
https://dvcs.w3.org/hg/content-security-policy/raw-file/tip/csp-specification.dev.html#securitypolicyviolationevent-events defines a first pass at a SecurityPolicyViolationEvent interface. We should play around with an implementation to get some experience to feed back into the working group.

Whatever we do here should remain safely locked behind CSP_NEXT.
Comment 1 Mike West 2013-03-19 07:01:24 PDT
Created attachment 193819 [details]
Patch
Comment 2 Mike West 2013-03-19 07:02:35 PDT
Comment on attachment 193819 [details]
Patch

Clearing r? until I've done a few rounds with the EWS bots, because I hate adding files. I'll be shocked if this compiles everywhere. :)
Comment 3 Mike West 2013-03-19 09:00:09 PDT
Comment on attachment 193819 [details]
Patch

Hrm. I am pleasantly shocked. :)

r?ing. Would one of you fine folks on CC mind taking a look?
Comment 4 Adam Barth 2013-03-19 09:59:39 PDT
Comment on attachment 193819 [details]
Patch

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

This looks fine, but I'd like to look at it one more time to make sure the web-visible changes are properly hidden when CSP.next is disabled at runtime.

> Source/WebCore/dom/Document.idl:344
> +    [NotEnumerable, Conditional=CSP_NEXT] attribute EventListener onsecuritypolicyviolation;

This attribute needs to be enabled at runtime or else it will be visible to the web.

> Source/WebCore/dom/SecurityPolicyViolationEvent.h:36
> +    };

No need for a ; here.

> Source/WebCore/dom/SecurityPolicyViolationEvent.h:55
> +    static PassRefPtr<SecurityPolicyViolationEvent> create(const String& documentURI, const String& referrer, const String& blockedURI, const String& violatedDirective, const String& effectiveDirective, const String& originalPolicy, const String& sourceURL, int lineNumber)

I'd skip this one too.  Even C++ code is better off using SecurityPolicyViolationEventInit.

> Source/WebCore/dom/SecurityPolicyViolationEvent.h:65
> +    void initSecurityPolicyViolationEvent(const AtomicString& eventType, bool canBubble, bool cancelable, const String& documentURI, const String& referrer, const String& blockedURI, const String& violatedDirective, const String& effectiveDirective, const String& originalPolicy, const String& sourceURL, int lineNumber)

We should skip this function.  We only implement these sorts of functions for legacy events.

> Source/WebCore/dom/SecurityPolicyViolationEvent.idl:29
> +    void initSecurityPolicyViolationEvent(in [Optional=DefaultIsUndefined] DOMString type,

We should remove this function from the API.  We're implementing new events in the DOM4 Events style.

> Source/WebCore/page/DOMWindow.idl:573
> +    [Conditional=CSP_NEXT] attribute SecurityPolicyViolationEventConstructor SecurityPolicyViolationEvent;

This attribute should also be enabled at runtime.
Comment 5 Mike West 2013-03-19 11:01:32 PDT
Created attachment 193867 [details]
Patch
Comment 6 Mike West 2013-03-19 11:02:39 PDT
(In reply to comment #5)
> Created an attachment (id=193867) [details]
> Patch

Thanks, Adam. The runtime flag is important, thanks for remembering it. I've taken another pass; would you mind taking another look?
Comment 7 Adam Barth 2013-03-19 12:22:12 PDT
Comment on attachment 193867 [details]
Patch

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

> Source/WebCore/dom/SecurityPolicyViolationEvent.h:69
> +    virtual const AtomicString& interfaceName() const { return eventNames().interfaceForSecurityPolicyViolationEvent; }

Does this whole header need a CSP_NEXT ifdef?

> Source/WebCore/dom/SecurityPolicyViolationEvent.h:76
> +    SecurityPolicyViolationEvent(const String& documentURI, const String& referrer, const String& blockedURI, const String& violatedDirective, const String& effectiveDirective, const String& originalPolicy, const String& sourceURL, int lineNumber)

This function now has no callers.
Comment 8 Mike West 2013-03-19 14:17:17 PDT
Created attachment 193919 [details]
Patch
Comment 9 Mike West 2013-03-19 14:20:10 PDT
(In reply to comment #8)
> Created an attachment (id=193919) [details]
> Patch

Addressed the bits of feedback from the last round, and I'll throw this to the EWS bots one more time, since there was a merge conflict in the xcode project. Assuming that the bots are happy, I'll CQ it in the morning.

Thanks!
Comment 10 Mike West 2013-03-19 22:59:21 PDT
Comment on attachment 193919 [details]
Patch

Bots are happy. CQing. :)
Comment 11 WebKit Review Bot 2013-03-19 23:25:31 PDT
Comment on attachment 193919 [details]
Patch

Clearing flags on attachment: 193919

Committed r146305: <http://trac.webkit.org/changeset/146305>
Comment 12 WebKit Review Bot 2013-03-19 23:25:35 PDT
All reviewed patches have been landed.  Closing bug.