Last Comment Bug 814391 - Permanent Orange: test_cspreports.js hangs for Thunderbird after bug 802905 landed (buildbot.slave.commands.TimeoutError)
: Permanent Orange: test_cspreports.js hangs for Thunderbird after bug 802905 l...
Status: RESOLVED FIXED
: intermittent-failure, regression
Product: MailNews Core
Classification: Components
Component: Security (show other bugs)
: Trunk
: x86 All
: -- normal (vote)
: Thunderbird 20.0
Assigned To: Mike Conley (:mconley) - (Away until June 29th)
:
Mentors:
Depends on:
Blocks: 802905
  Show dependency treegraph
 
Reported: 2012-11-22 05:27 PST by Mark Banner (:standard8)
Modified: 2012-11-26 02:05 PST (History)
5 users (show)
standard8: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch v1 (913 bytes, patch)
2012-11-23 11:37 PST, Mike Conley (:mconley) - (Away until June 29th)
standard8: review+
Details | Diff | Review

Description Mark Banner (:standard8) 2012-11-22 05:27:53 PST
Since bug 802905 landed, we've been seeing permanent hangs running test_cspreports.js against Thunderbird builds.

If I ctrl-c the test running locally on a debug build, I get:

TEST-UNEXPECTED-FAIL | /Users/moztest/comm/main/tb/mozilla/_tests/xpcshell/content/base/test/unit/test_cspreports.js | test failed (with xpcshell return code: -2), see following log:
>>>>>>>
### XPCOM_MEM_LEAK_LOG defined -- logging leaks to /var/folders/3r/4lhjf61d4fqdxjl7q5_yzz300000gp/T/tmp4Ku8MG/runxpcshelltests_leaks.log

TEST-INFO | (xpcshell/head.js) | test 1 pending

TEST-INFO | (xpcshell/head.js) | test 2 pending
Created test 0 : allow 'none'; report-uri http://localhost:9000/test0


TEST-INFO | (xpcshell/head.js) | test 3 pending
Created test 1 : allow 'none'; report-uri http://localhost:9000/test1


TEST-INFO | (xpcshell/head.js) | test 4 pending
Created test 2 : allow 'none'; report-uri http://localhost:9000/test2


TEST-INFO | (xpcshell/head.js) | test 4 finished

TEST-INFO | (xpcshell/head.js) | running event loop
WARNING: NS_ENSURE_SUCCESS(rv, nsresult::NS_ERROR_UNEXPECTED) failed with result 0x80004005: file /Users/moztest/comm/main/src/mozilla/extensions/cookie/nsPermissionManager.cpp, line 377
WARNING: NS_ENSURE_SUCCESS(rv, nullptr) failed with result 0x80004003: file /Users/moztest/comm/main/src/mailnews/base/src/nsMsgContentPolicy.cpp, line 614
WARNING: NS_ENSURE_TRUE(aRequestingContext) failed: file /Users/moztest/comm/main/src/mailnews/base/src/nsMsgContentPolicy.cpp, line 728
WARNING: NS_ENSURE_SUCCESS(rv, nsresult::NS_OK) failed with result 0x80004003: file /Users/moztest/comm/main/src/mailnews/base/src/nsMsgContentPolicy.cpp, line 291
WARNING: NS_ENSURE_SUCCESS(rv, nullptr) failed with result 0x80004003: file /Users/moztest/comm/main/src/mailnews/base/src/nsMsgContentPolicy.cpp, line 614
WARNING: NS_ENSURE_TRUE(aRequestingContext) failed: file /Users/moztest/comm/main/src/mailnews/base/src/nsMsgContentPolicy.cpp, line 728
WARNING: NS_ENSURE_SUCCESS(rv, nsresult::NS_OK) failed with result 0x80004003: file /Users/moztest/comm/main/src/mailnews/base/src/nsMsgContentPolicy.cpp, line 291
WARNING: NS_ENSURE_SUCCESS(rv, nullptr) failed with result 0x80004003: file /Users/moztest/comm/main/src/mailnews/base/src/nsMsgContentPolicy.cpp, line 614
WARNING: NS_ENSURE_TRUE(aRequestingContext) failed: file /Users/moztest/comm/main/src/mailnews/base/src/nsMsgContentPolicy.cpp, line 728
WARNING: NS_ENSURE_SUCCESS(rv, nsresult::NS_OK) failed with result 0x80004003: file /Users/moztest/comm/main/src/mailnews/base/src/nsMsgContentPolicy.cpp, line 291
<<<<<<<
Comment 1 Mike Conley (:mconley) - (Away until June 29th) 2012-11-23 11:37:16 PST
Created attachment 684751 [details] [diff] [review]
Patch v1

So, I think we're failing this because mailnews's nsMsgContentPolicy is blocking the CSP report type, which we're not supposed to do.

Testing this patch on try:

https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=787096cf6bba
Comment 2 Mike Conley (:mconley) - (Away until June 29th) 2012-11-23 13:18:08 PST
Comment on attachment 684751 [details] [diff] [review]
Patch v1

So this appears to fix it... am I doing the right thing here?
Comment 3 Tanvi Vyas - out until 6/27 [:tanvi] 2012-11-23 17:04:41 PST
(In reply to Mike Conley (:mconley) from comment #2)
> Comment on attachment 684751 [details] [diff] [review]
> Patch v1
> 
> So this appears to fix it... am I doing the right thing here?

Hi Mark,

Bug 802905 changed the content type for CSP reports to be more specific (TYPE_CSP_REPORT instead of TYPE_OTHER).  Adding code to allow for TYPE_CSP_REPORT should hopefully do the trick.  However, I don't see the code where the code to allow TYPE_OTHER is: http://mxr.mozilla.org/comm-central/source/mailnews/base/src/nsMsgContentPolicy.cpp#208
It looks like you are just checking for TYPE_DOCUMENT, so I'm not sure where CSP reports were allowed before your patch.
Comment 4 Mark Banner (:standard8) 2012-11-26 00:42:18 PST
(In reply to Tanvi Vyas from comment #3)
> Bug 802905 changed the content type for CSP reports to be more specific
> (TYPE_CSP_REPORT instead of TYPE_OTHER).  Adding code to allow for
> TYPE_CSP_REPORT should hopefully do the trick.  However, I don't see the
> code where the code to allow TYPE_OTHER is:
> http://mxr.mozilla.org/comm-central/source/mailnews/base/src/
> nsMsgContentPolicy.cpp#208
> It looks like you are just checking for TYPE_DOCUMENT, so I'm not sure where
> CSP reports were allowed before your patch.

From bug 802905 comment 0, it looks like the CSP reports have a null requesting location, we'd have therefore returned early just after the type document. As at that stage the default is to accept, we'd have been accepting these reports.

As the bug has now separated them out, and given a requesting location, I think that's why it is now failing, so Mike's patch is correct.
Comment 5 Mark Banner (:standard8) 2012-11-26 00:42:32 PST
Comment on attachment 684751 [details] [diff] [review]
Patch v1

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

Thanks Mike.
Comment 6 Mark Banner (:standard8) 2012-11-26 02:05:52 PST
Checked in: https://hg.mozilla.org/comm-central/rev/56b11777b6cc

Note You need to log in before you can comment on or make changes to this bug.