Improve error message for insecure 'content_security_policy' manifest entries.

Improve error message for insecure 'content_security_policy' manifest entries.

BUG=143341
TBR=ben@chromium.org


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=154701

[Mike West, 2012-08-17 08:55:43]

Hello Aaron and Adam!

This is a quick update to the error message generated when an extension's
'content_security_policy' manifest entry isn't secure.

Are there any tests for this functionality currently? I didn't see anything in a
quick grep through the code.

Thanks!

Mihai: FYI. I don't think this effects platform apps, but I'm not entirely sure
that's the case.

-mike

[Mihai Parparita -not on Chrome, 2012-08-17 22:47:29]

On Fri, Aug 17, 2012 at 1:55 AM, <mkwst@chromium.org> wrote:

> Mihai: FYI. I don't think this effects platform apps, but I'm not entirely
> sure
> that's the case.
>

Indeed, this CL doesn't affect apps.

Mihai

[Mike West, 2012-08-21 05:35:29]

Friendly ping, Adam and Aaron. :)

Thanks!

-mike

[Aaron Boodman, 2012-08-21 23:17:53]

The tests for this kind of thing are in chrome/common/extensions/manifest_tests/

[Mike West, 2012-08-26 09:50:31]

On 2012/08/21 23:17:53, Aaron Boodman wrote:
> The tests for this kind of thing are in
chrome/common/extensions/manifest_tests/

Brilliant, I've added some tests for this change. Would you mind taking a look?

Thanks!

-mike

[abarth-chromium, 2012-08-27 06:51:57]

LGTM, but I'm not an owner.  One minor comment below.

https://chromiumcodereview.appspot.com/10828363/diff/5001/chrome/common/exten...
File chrome/common/extensions/extension_manifest_constants.cc (right):

https://chromiumcodereview.appspot.com/10828363/diff/5001/chrome/common/exten...
chrome/common/extensions/extension_manifest_constants.cc:459: "
http://localhost).";
It's confusing when you're using single and when you're using double quotes,
especially with 'self' needing the single quotes...  I'm not sure what to
suggest to improve this given that many of the other messages using single
quotes as well.

[Mike West, 2012-08-27 08:38:39]

On 2012/08/27 06:51:57, abarth wrote:
> LGTM, but I'm not an owner.  One minor comment below.
> 
>
https://chromiumcodereview.appspot.com/10828363/diff/5001/chrome/common/exten...
> File chrome/common/extensions/extension_manifest_constants.cc (right):
> 
>
https://chromiumcodereview.appspot.com/10828363/diff/5001/chrome/common/exten...
> chrome/common/extensions/extension_manifest_constants.cc:459: "
> http://localhost%29.%22;
> It's confusing when you're using single and when you're using double quotes,
> especially with 'self' needing the single quotes...  I'm not sure what to
> suggest to improve this given that many of the other messages using single
> quotes as well.

You're right. I'm rephrasing the error slightly to try to make things more
clear, but I won't land it until Mihai or Aaron has a chance to take a look.

(This is a scenario where backtics would come in handy. :) )

[Aaron Boodman, 2012-08-30 04:31:47]

lgtm

http://codereview.chromium.org/10828363/diff/16001/chrome/common/extensions/e...
File chrome/common/extensions/extension_manifest_constants.cc (right):

http://codereview.chromium.org/10828363/diff/16001/chrome/common/extensions/e...
chrome/common/extensions/extension_manifest_constants.cc:457: " 'object-src'
directives can only whitelist secure resources. You may"
For the second sentence, how about:

You may include any of the following sources: 'self', 'http://localhost',
'http://127.0.0.1', or any https:// or chrome-extension:// origin. For more
information see: <link to extensions doc>.

[Mike West, 2012-09-03 14:10:13]

TBRing the `chrome_tests.gypi` change. Hi Ben!

[commit-bot: I haz the power, 2012-09-03 14:10:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mkwst@chromium.org/10828363/19002

[commit-bot: I haz the power, 2012-09-03 17:07:33]

Change committed as 154701

[Ben Goodger (Google), 2012-09-04 15:36:30]

gyp lgtm