Bug 112681 – CSP 1.1: Stub out SecurityPolicyViolationEvent interface.

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[Mike West]

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.

[Mike West]

Created attachment 193819 [details]
Patch

[Mike West]

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. :)

[Mike West]

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?

[Adam Barth]

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.

[Mike West]

Created attachment 193867 [details]
Patch

[Mike West]

(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?

[Adam Barth]

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.

[Mike West]

Created attachment 193919 [details]
Patch

[Mike West]

(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!

[Mike West]

Comment on attachment 193919 [details]
Patch

Bots are happy. CQing. :)

[WebKit Review Bot]

Comment on attachment 193919 [details]
Patch

Clearing flags on attachment: 193919

Committed r146305: <http://trac.webkit.org/changeset/146305>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.