832398 - Incorrect error message for CSP in X-Content-Security-Policy header using allow directive

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Ian Melven :imelven]

[10:11:32.015] CSP WARN:  allow directive is deprecated, use the equivalent default-source directive instead

We want to deprecate old style CSP before Fx24 but this really bugs me and it's a tiny change.

Comment 1

[Ian Melven :imelven]

To be clear, what needs to be changed is

default-source -> default-src

There is no 'default-source' directive in CSP.

Comment 2

[Sid Stamm [:geekboy or :sstamm]]

Looks like this needs to be changed in at least 25 locales in l10n-central:
http://mxr.mozilla.org/l10n-central/search?string=default-source

Axel: this is a string change, but the change doesn't need translation since it's a code token and not different in different languages.  Can we just patch all the changes, or do we need to re-localize the string?

Comment 3

[Sid Stamm [:geekboy or :sstamm]]

Attachment: patch for mozilla-central

Comment 4

[Axel Hecht [:Pike]]

Most of our localizations work on aurora, so the query on central isn't very telling.

http://mxr.mozilla.org/l10n-mozilla-aurora/search?string=default-source&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=l10n-mozilla-aurora show 75 locales, which is basically "all".

We'll need to re-localize the string by changing the ID.

Comment 5

[Sid Stamm [:geekboy or :sstamm]]

(In reply to Axel Hecht [:Pike] from comment #4)
> We'll need to re-localize the string by changing the ID.

That seems like an awful lot of work for what would essentially amount to a verbatim find/replace of "default-source" to "default-src"... I mean, we can do it, but I'm happy to write the patches to do the find/replace to reduce work for the localizers.

Comment 6

[Axel Hecht [:Pike]]

You'd need to marshall 75 patches into 75 different localization teams, some of which don't have any idea on how to apply a patch to a repo or don't work on a repo to begin with. Just having them to redo that one string is about as much work for them, I guess.

Comment 7

[Sid Stamm [:geekboy or :sstamm]]

Attachment: patch for mozilla-central

Okay, then.  I guess we change the string in mozilla-central and get it re-localized.  Axel, can you review?

Comment 8

[Sid Stamm [:geekboy or :sstamm]]

Comment on attachment 704013 [details] [diff] [review]
patch for mozilla-central

Hm, looks like you've got a couple of accounts here, Axel.  Flagging this one for review instead.

Comment 9

[Axel Hecht [:Pike]]

Comment on attachment 704013 [details] [diff] [review]
patch for mozilla-central

Review of attachment 704013 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with a nit, we should make the original comment actually useful, now that we have a chance.

::: dom/locales/en-US/chrome/security/csp.properties
@@ +44,5 @@
>  failedToParseUnrecognizedSource = Failed to parse unrecognized source %1$S
>  # LOCALIZATION NOTE (reportPostRedirect):
>  # %1$S is the specified report URI before redirect
>  reportPostRedirect = Post of violation report to %1$S failed, as a redirect occurred
> +# LOCALIZATION NOTE (allowDirectiveIsDeprecated):

it was a nice idea to add a comment back when we added this in bug 634778, but an empty comment, well....

I guess the relevant help here would be to not translate "allow" and "default-src". Something like

# Don't translate "allow" and "default-src", as they are part of ...

... and then I lack a good idea. If you don't have one, just leave the ,.. out, as we got the import dont-translate message across.

Comment 10

[Sid Stamm [:geekboy or :sstamm]]

Attachment: patch for mozilla-central

Totally dropped off my radar.  Thanks for the review Axel.  I've taken your advice and made the localization comment useful.  :)

Carrying over r=l10n and unbitrotting (not much to do there).

Comment 11

[Sid Stamm [:geekboy or :sstamm]]

Inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/c1d4ca637c61

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c1d4ca637c61