Bug 94432

Summary: 'plugin-types' CSP warning should include details about explicit type declaration when relevant.
Product: WebKit Reporter: Mike West <mkwst>
Component: WebCore Misc.Assignee: Mike West <mkwst>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 93197    
Attachments:
Description Flags
Patch
none
Changing the string a bit.
none
Patch none

Description Mike West 2012-08-19 12:38:27 PDT
The `plugin-types` directive, as currently implemented, enforces a strict requirement that all plugin types be explicitly declared in a protected resource. If a developer doesn't include an explicit `type` attribute on her `object` or `embed` elements, the `plugin-types` directive will block it. This isn't clear from the current error message.

I'd like to add an additional line to the error in the case where a plugin is blocked due to a lack of an explicit declaration.
Comment 1 Mike West 2012-08-19 12:45:00 PDT
Created attachment 159302 [details]
Patch
Comment 2 Adam Barth 2012-08-19 13:54:30 PDT
Comment on attachment 159302 [details]
Patch

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

> Source/WebCore/page/ContentSecurityPolicy.cpp:866
> +        message = message + "\nWhen enforcing media type restrictions via CSP, the plugin's media type must be explicitly declared with a 'type' attribute on the containing element (e.g. '<object type=\"[TYPE GOES HERE]\" ...>').\n";

CSP -> Content-Security-Policy
Comment 3 Mike West 2012-08-19 14:03:05 PDT
Comment on attachment 159302 [details]
Patch

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

>> Source/WebCore/page/ContentSecurityPolicy.cpp:866
>> +        message = message + "\nWhen enforcing media type restrictions via CSP, the plugin's media type must be explicitly declared with a 'type' attribute on the containing element (e.g. '<object type=\"[TYPE GOES HERE]\" ...>').\n";
> 
> CSP -> Content-Security-Policy

The whole message is:

    "Refused to load 'data:application/x-webkit-test-netscape,logifloaded' (MIME type '') because it violates the following Content Security Policy Directive: 'plugin-types application/x-invalid-type'.
     
     When enforcing media type restrictions via CSP, the plugin's media type must be explicitly declared with a 'type' attribute on the containing element (e.g. '<object type="[TYPE GOES HERE]" ...>')."

Since I spelled it out in the first sentence, it didn't seem necessary in the second. *shrug* I'm happy to expand it if you think it's potentially confusing.
Comment 4 Adam Barth 2012-08-19 14:55:59 PDT
Ok.
Comment 5 Mike West 2012-08-20 01:17:24 PDT
Created attachment 159355 [details]
Changing the string a bit.
Comment 6 Mike West 2012-08-20 01:19:50 PDT
Created attachment 159356 [details]
Patch
Comment 7 Mike West 2012-08-20 01:20:55 PDT
(In reply to comment #5)
> Created an attachment (id=159355) [details]
> Changing the string a bit.

It's now "CONSOLE MESSAGE: Refused to load 'http://127.0.0.1:8000/plugins/resources/mock-plugin.pl' (MIME type '') because it violates the following Content Security Policy Directive: 'plugin-types application/x-invalid-type'. When enforcing the 'plugin-types' directive, the plugin's media type must be explicitly declared with a 'type' attribute on the containing element (e.g. '<object type="[TYPE GOES HERE]" ...>')." which avoids the problem of mentioning CSP twice in the error message. :)
Comment 8 Adam Barth 2012-08-20 11:16:18 PDT
Excellent
Comment 9 WebKit Review Bot 2012-08-20 11:33:36 PDT
Comment on attachment 159356 [details]
Patch

Clearing flags on attachment: 159356

Committed r126047: <http://trac.webkit.org/changeset/126047>
Comment 10 WebKit Review Bot 2012-08-20 11:33:39 PDT
All reviewed patches have been landed.  Closing bug.