991466 - CSP in C++: Remove specCompliant flag everywhere after removing the old parser

Status: RESOLVED FIXED

(Core :: DOM: Security, defect)

Description

[Christoph Kerschbaumer [:ckerschb]]

The new parser does not support the 'X-Content-Security-Policy' header, therefore we can remove the specCompliant flag everywhere, e.g.
> void appendPolicy(in AString policyString, in nsIURI selfURI,
>                    in boolean reportOnly, in boolean specCompliant);

Comment 1

[Sid Stamm [:geekboy or :sstamm]]

Attachment: remove-specCompliant

First whack at removing specCompliant.  This will probably suffer some bitrot depending on when some of the other removals (and test changes) land.

Comment 2

[Sid Stamm [:geekboy or :sstamm]]

Attachment: remove-specCompliant

This patch removes spec-compliant arguments from everywhere and updates the tests accordingly to use spec-compliant policies.  This applies to mozilla-central now that bug 949533 has landed.

Comment 3

[Christoph Kerschbaumer [:ckerschb]]

Comment on attachment 8449785 [details] [diff] [review]
remove-specCompliant

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

This all looks good to me, but probably we also want to remove:
> security.csp.speccompliant
from all the tests since it's not needed anymore, see:
http://mxr.mozilla.org/mozilla-central/search?string=security.csp.speccompliant&find=&findi=&filter=%5E%5B%5E%5C0%5D*%24&hitlimit=&tree=mozilla-central

Please remove from testcases and flag me again!

Comment 4

[Sid Stamm [:geekboy or :sstamm]]

Attachment: remove security.csp.speccompliant from mochitests

As requested, this patch adds removal of that pref from the mochitests.  Applied with the previous patch, the tests still all pass.

Comment 5

[Sid Stamm [:geekboy or :sstamm]]

Comment on attachment 8449785 [details] [diff] [review]
remove-specCompliant

Reflagging this patch for review; the additional changes you requested are in the other patch on this bug and I don't think you requested changes to this patch.  Anything else to change?

Comment 6

[Sid Stamm [:geekboy or :sstamm]]

Forgot to add: I put the test changes in a separate patch because it was very mechanical (pretty close to the same changes in each file) and there are > 30 files touched for that basic change.

Comment 7

[Christoph Kerschbaumer [:ckerschb]]

Comment on attachment 8449847 [details] [diff] [review]
remove security.csp.speccompliant from mochitests

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

Exactly what I was looking for! Quick turnaround - I am impressed!

Comment 8

[Christoph Kerschbaumer [:ckerschb]]

Comment on attachment 8449785 [details] [diff] [review]
remove-specCompliant

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

That one was ok before - since you incorporated the requested changes in a separate patch I can safely r+ this one!

Comment 9

[Sid Stamm [:geekboy or :sstamm]]

Comment on attachment 8449785 [details] [diff] [review]
remove-specCompliant

For good measure, before landing I would also like jst to take a quick look at the changes to nsDocument.cpp (small changes, jst).

Comment 10

[Johnny Stenback (:jst)]

Comment on attachment 8449785 [details] [diff] [review]
remove-specCompliant

Should we worry about extension compat issues here due to the nsIContentSecurityPolicy API change? If so, please add the appropriate keywords to this bug so the right folks have a chance to weigh in before we ship this.

r=jst

Comment 11

[Sid Stamm [:geekboy or :sstamm]]

Thanks, jst!  I think we're ok with extension compat because the code *should* disregard the extra boolean argument (without error) from any js-callers of appendPolicy who pass it in.

Comment 12

[Sid Stamm [:geekboy or :sstamm]]

Since inbound has been a bit fragile lately: https://tbpl.mozilla.org/?tree=Try&rev=842b8bf0883c

Comment 13

[Sid Stamm [:geekboy or :sstamm]]

Try was green.

https://hg.mozilla.org/integration/mozilla-inbound/rev/3a34bf944379
https://hg.mozilla.org/integration/mozilla-inbound/rev/5ae58efd730b

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3a34bf944379
https://hg.mozilla.org/mozilla-central/rev/5ae58efd730b