User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/49.0.2623.87 Safari/537.36 Steps to reproduce: On upgrading to FF 45.0.1, our production site broke because our CSP policy was not parser compliant. <meta http-equiv="Content-Security-Policy" content=" script-src https://ajax.googleapis.com https://cdnjs.cloudflare.com 'self'; style-src https://fonts.googleapis.com https://cdnjs.cloudflare.com 'self'; "> While readable, this is indeed non-compliant because of step 2.3 in the parsing spec – there can only be one space between the directive name and the directive value. However, correct me if I am wrong, but as per https://www.w3.org/TR/2012/CR-CSP-20121115/#parsing, something like this should work: <meta http-equiv="Content-Security-Policy" content=" script-src https://ajax.googleapis.com https://cdnjs.cloudflare.com 'self'; style-src https://fonts.googleapis.com https://cdnjs.cloudflare.com 'self';"> Step 2.3 says that a single whitespace must be between the directive name and value, which is the case here. However, Step 2.1 implies that there can be any amount of whitespace before the directive name. Actual results: None of the security directives are added, and so CSP blocks all requested content on the page. Expected results: The security directives should have been added and the resources should have been loaded.

(As an aside, I'm not sure what the spec says about whitespace between URLs in the directive value. Maybe that is something to discuss as well.)

Thanks for reporting!

With the introduction of meta CSP we should also have updated the parser to allow line-breaks within the different directives. Within this patch I relaxed the CSP parser. Dan do you think we need to uplift?

Comment on attachment 8742358 [details] [diff] [review] bug_1261634_skip_whitespace_within_csp.patch Review of attachment 8742358 [details] [diff] [review]: ----------------------------------------------------------------- sorry for r-, but if we're going to fix this we might as well go all the way to spec compliance. The single space after the directive seems uncommonly strict for HTTP/HTML but note that, too, references the full HTML "space" definition, not just 0x20. ::: dom/security/nsCSPParser.h @@ +56,4 @@ > inline void skipWhiteSpace() > { > + while (mCurChar < mEndChar && > + (*mCurChar == ' ' || *mCurChar == '\n')) { The CSP spec references the HTML spec: "The space characters, for the purposes of this specification, are U+0020 SPACE, "tab" (U+0009), "LF" (U+000A), "FF" (U+000C), and "CR" (U+000D)." You could almost use isspace() except that includes vertical tab. We might have an HTML-appropriate isspace in nsContentUtil or similar.

Comment on attachment 8742359 [details] [diff] [review] bug_1261634_skip_whitespace_within_csp_tests.patch Review of attachment 8742359 [details] [diff] [review]: ----------------------------------------------------------------- Likewise the test should check the other "space" characters.

(In reply to Daniel Veditz [:dveditz] from comment #5) > sorry for r-, but if we're going to fix this we might as well go all the way > to spec compliance. The single space after the directive seems uncommonly > strict for HTTP/HTML but note that, too, references the full HTML "space" > definition, not just 0x20. You are absolutely right Dan - thanks for pointing that out. In fact, IsHTMLWhitespace() is what we want. Even after the directive name we should not be too strict and just allow a single 'space', also any of the IsHTMLWhitespace() should be allowed.

Updated the test to contain a weired mash of all possible html whitespace characters.

Comment on attachment 8744010 [details] [diff] [review] bug_1261634_skip_whitespace_within_csp.patch Review of attachment 8744010 [details] [diff] [review]: ----------------------------------------------------------------- r=dveditz

https://hg.mozilla.org/integration/mozilla-inbound/rev/d8091c6ddad7 https://hg.mozilla.org/integration/mozilla-inbound/rev/5c0797963074

https://hg.mozilla.org/mozilla-central/rev/d8091c6ddad7 https://hg.mozilla.org/mozilla-central/rev/5c0797963074