Closed
Bug 898190
Opened 8 years ago
Closed 8 years ago
csp test uses invalid policy values (test failure from bug 888172 on beta 23)
Categories
(Core :: Security, defect, P1)
Tracking
()
RESOLVED
FIXED
mozilla25
People
(Reporter: geekboy, Assigned: geekboy)
References
(Depends on 1 open bug)
Details
(Whiteboard: [qa-])
Attachments
(1 file)
1.66 KB,
patch
|
grobinson
:
review+
lsblakk
:
approval-mozilla-aurora+
lsblakk
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
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
Assignee | ||
Comment 1•8 years ago
|
||
Attachment #781301 -
Flags: review?(grobinson)
Assignee | ||
Updated•8 years ago
|
Attachment #781301 -
Attachment is patch: true
Comment 2•8 years ago
|
||
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+
Assignee | ||
Comment 3•8 years ago
|
||
verified the test still passes locally and pushed to central: https://hg.mozilla.org/mozilla-central/rev/76fb8f815ac7
Assignee | ||
Comment 4•8 years ago
|
||
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?
Updated•8 years ago
|
Attachment #781301 -
Flags: approval-mozilla-beta?
Attachment #781301 -
Flags: approval-mozilla-beta+
Attachment #781301 -
Flags: approval-mozilla-aurora?
Attachment #781301 -
Flags: approval-mozilla-aurora+
Assignee | ||
Comment 5•8 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/fedcc7eb515b https://hg.mozilla.org/releases/mozilla-beta/rev/7499e60e458b
Assignee | ||
Updated•8 years ago
|
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox23:
--- → fixed
status-firefox24:
--- → fixed
status-firefox25:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
Comment 6•8 years ago
|
||
(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.
Assignee | ||
Comment 7•8 years ago
|
||
Yep, Garrett and I are checking it out.
Assignee | ||
Comment 8•8 years ago
|
||
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.
Description
•