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

RESOLVED FIXED in mozilla22

Status

()

defect
--
trivial
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: imelven, Assigned: geekboy)

Tracking

(Blocks 1 bug)

Trunk
mozilla22
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Reporter

Description

6 years ago
[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.
Reporter

Comment 1

6 years ago
To be clear, what needs to be changed is

default-source -> default-src

There is no 'default-source' directive in CSP.
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?
Flags: needinfo?(axel)
Posted patch patch for mozilla-central (obsolete) — Splinter Review
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.
Flags: needinfo?(axel)
(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.
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.
Posted patch patch for mozilla-central (obsolete) — Splinter Review
Okay, then.  I guess we change the string in mozilla-central and get it re-localized.  Axel, can you review?
Assignee: imelven → sstamm
Attachment #703982 - Attachment is obsolete: true
Attachment #704013 - Flags: review?(axel)
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.
Attachment #704013 - Flags: review?(axel) → review?(l10n)
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.
Attachment #704013 - Flags: review?(l10n) → review+
Assignee

Updated

6 years ago
Flags: needinfo?(sstamm)
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).
Attachment #704013 - Attachment is obsolete: true
Attachment #730736 - Flags: review+
Flags: needinfo?(sstamm)
https://hg.mozilla.org/mozilla-central/rev/c1d4ca637c61
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.