csp test uses invalid policy values (test failure from bug 888172 on beta 23)

RESOLVED FIXED in Firefox 23

Status

()

defect
P1
major
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: geekboy, Assigned: geekboy)

Tracking

(Depends on 1 bug)

23 Branch
mozilla25
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox23 fixed, firefox24 fixed, firefox25 fixed)

Details

(Whiteboard: [qa-])

Attachments

(1 attachment)

When we uplifted bug 888172 to beta 23 its tests failed.  That's because of a bad CSP policy string that should never have worked in the first case.  For some reason, the test only fails in 23, not 24 or 25.

Test that fails:
content/base/test/test_CSP_bug888172.html
Posted patch fixSplinter Review
Attachment #781301 - Flags: review?(grobinson)
Attachment #781301 - Attachment is patch: true
Comment on attachment 781301 [details] [diff] [review]
fix

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

That should do it. Still don't know how this ever worked.
Attachment #781301 - Flags: review?(grobinson) → review+
verified the test still passes locally and pushed to central:
https://hg.mozilla.org/mozilla-central/rev/76fb8f815ac7
Comment on attachment 781301 [details] [diff] [review]
fix

[Approval Request Comment]
Bug caused by (feature/regressing bug #): tests for bug 888172 fail on beta 23
User impact if declined: none
Testing completed (on m-c, etc.): applied to local beta and passes, landed on m-c 
Risk to taking this patch (and alternatives if risky): zero risk, fixes a test (big win)
String or IDL/UUID changes made by this patch:
Attachment #781301 - Flags: approval-mozilla-beta?
Attachment #781301 - Flags: approval-mozilla-aurora?
Attachment #781301 - Flags: approval-mozilla-beta?
Attachment #781301 - Flags: approval-mozilla-beta+
Attachment #781301 - Flags: approval-mozilla-aurora?
Attachment #781301 - Flags: approval-mozilla-aurora+
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
(In reply to Sid Stamm [:geekboy or :sstamm] from comment #0)
> For some reason, the test only fails in 23, not 24 or 25.

I really hope somebody's looking into what that reason is.
Yep, Garrett and I are checking it out.
Turns out that the test for bug 888172 exposed a sleeping bug in our CSP 1.0 parser implementation that the patch for bug 780978 fixed.  

http://mxr.mozilla.org/mozilla-beta/source/content/base/src/CSPUtils.jsm#893

defaultSrcDir.clone() throws when defaultSrcDir is undefined -- this leads to the CSP being rejected and reverted to "default-src *" which blocks inline scripts.  The fix is to put in a null check and create a defaultSrcDir if one is not present.

Since we uplifted a bunch of stuff but not bug 780978, the bug surfaced on beta 23 and not other builds.  Earlier builds were not affected and later ones were fixed by bug 780978.  This only surfaces with non-prefixed CSP headers, and only happens in 23 (not 22/earlier or 24/later). Since the bug itself has already been fixed in 24 and adoption of the standardized header is currently low, I think we should just let this one go.
Assuming no verification needed here. Please add the verifyme keyword and remove the [qa-] whiteboard tag to request verification.
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.