Closed Bug 1278782 Opened 8 years ago Closed 8 years ago

TEST-UNEXPECTED-FAIL | netwerk/test/unit/test_authentication.js | xpcshell return code: 0

Categories

(MailNews Core :: Security, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 50.0

People

(Reporter: aleth, Assigned: aleth)

References

Details

(Keywords: intermittent-failure, regression)

Attachments

(3 files)

ERROR Component returned failure code: 0x805e0006 [nsIChannel.asyncOpen2] test_noauth@/Users/helvellyn/buildhg/cc1/obj-mail/_tests/xpcshell/netwerk/test/unit/test_authentication.js:331:3 run_test@/Users/helvellyn/buildhg/cc1/obj-mail/_tests/xpcshell/netwerk/test/unit/test_authentication.js:324:3 Reproducible locally.
I can't find any information on that failure code. Dragana, any ideas on what could be causing this?
Flags: needinfo?(dd.mozilla)
(In reply to aleth [:aleth] from comment #1) > I can't find any information on that failure code. Dragana, any ideas on > what could be causing this? The error code is for NS_ERROR_CONTENT_BLOCKED. Has this started after bug 1278641 has landed? It sounds like it since the change in that bug has to do with load types.
Flags: needinfo?(dd.mozilla)
(In reply to Dragana Damjanovic [:dragana] from comment #2) > Has this started after bug 1278641 has landed? It sounds like it since the > change in that bug has to do with load types. No, this was first observed (on comm-central) on Tuesday June 7.
Attached file console output of local run (deleted) —
To run this locally, we run mozilla/mach xpcshell-test netwerk/test/unit/test_authentication.js Output enclosed. I'm not sure which of the various warnings and errors are relevant. Maybe the ERROR Component returned failure code: 0x805e0006 [nsIChannel.asyncOpen2] as per comment #0
(In reply to aleth [:aleth] from comment #3) > (In reply to Dragana Damjanovic [:dragana] from comment #2) > > Has this started after bug 1278641 has landed? It sounds like it since the > > change in that bug has to do with load types. > > No, this was first observed (on comm-central) on Tuesday June 7. Backing out bug 1230462 fixes this.
Keywords: regression
Could you give us some further advice on that why the test might be failing in the TB environment after bug 1230462 landed.
Flags: needinfo?(dd.mozilla)
You can check what value pref network.auth.subresource-http-auth-allow has, and set it to 2 (allow all) (I have fix part of that logic in bug 1230462) It is reproducible locally, can you make a http log I could take a look at, probably that is going to say more.
Flags: needinfo?(dd.mozilla)
Thanks. network.auth.subresource-http-auth-allow is already set to 2 in all.js https://dxr.mozilla.org/comm-central/source/mozilla/modules/libpref/init/all.js#1914 which we seem to include somehow. In a "normal" TB session that preference is set and I assume it would be set in an xpcshell test as well. Adding it again to our all-thunderbird.js didn't make the test pass. I noticed that TB overrides some of the preferences included from all.js here: https://dxr.mozilla.org/comm-central/source/mail/app/profile/all-thunderbird.js#340 but nothing with network.auth.* gets changed. I'll see how I can do a http log.
Attached file http log (deleted) —
I followed https://developer.mozilla.org/en-US/docs/Mozilla/Debugging/HTTP_logging and set $ export NSPR_LOG_MODULES=timestamp,nsHttp:5,nsSocketTransport:5,nsStreamPump:5,nsHostResolver:5 $ export NSPR_LOG_FILE=log-jk.txt Here is the file. If you want something else, please let me know.
Flags: needinfo?(dd.mozilla)
Gentle ping: Dragana, could you please look at the log I provided so we can move forward on this. Our tree would be a lot greener if we could solve this one test failure which occurs on all platforms.
Maybe the default content policy differs? Not sure offhand where that is set.
I do not see anything in the log: 2016-06-10 07:19:43.100000 UTC - [Main Thread]: V/nsHttp Creating HttpBaseChannel @b042000 2016-06-10 07:19:43.100000 UTC - [Main Thread]: V/nsHttp Creating nsHttpChannel [this=b042000] 2016-06-10 07:19:43.106000 UTC - [Main Thread]: V/nsHttp HttpBaseChannel::Init [this=b042000] 2016-06-10 07:19:43.106000 UTC - [Main Thread]: V/nsHttp host=localhost port=50340 2016-06-10 07:19:43.106000 UTC - [Main Thread]: V/nsHttp uri=http://localhost:50340/auth 2016-06-10 07:19:43.106000 UTC - [Main Thread]: V/nsHttp nsHttpChannel::Init [this=b042000] 2016-06-10 07:19:43.143000 UTC - [Main Thread]: V/nsHttp Destroying nsHttpChannel [this=b042000] 2016-06-10 07:19:43.143000 UTC - [Main Thread]: V/nsHttp Destroying HttpBaseChannel @b042000 and probably has to do with default content policies. The policy is set in netwerk/base/NetUtil.jsm in function makeChan can you please check what it is and set the same thing in the new version of the test.
Flags: needinfo?(dd.mozilla)
I'm not sure what to look for. https://dxr.mozilla.org/mozilla-central/source/netwerk/base/NetUtil.jsm has the string "make" twice in a comment, maybe you mean: newChannel: function NetUtil_newChannel(aWhatToLoad, aOriginCharset, aBaseURI) There is also makeChan here: https://dxr.mozilla.org/mozilla-central/source/netwerk/test/unit/test_authentication.js#288 I dumped out contentPolicyType here https://dxr.mozilla.org/mozilla-central/source/netwerk/base/NetUtil.jsm#403 and I get 1, which is not a surprise since it's called with Components.interfaces.nsIContentPolicy.TYPE_OTHER (which is 1, right?)
If you change pref: prefs.setBoolPref("network.loadinfo.skip_type_assertion", true); it should fix this if it is a policy issue.
I'm not sure where I should place this statement since the test is a M-C test that we're running as part of the C-C test suite. I added pref("network.loadinfo.skip_type_assertion", true); to our all-thunderbird.js file and it made no difference, still seeing ERROR Component returned failure code: 0x805e0006 [nsIChannel.asyncOpen2] Maybe I should let Aleth have a go, his comment #11 suggests that he knows more about that than I.
You can add: var prefs = Cc["@mozilla.org/preferences-service;1"]. getService(Ci.nsIPrefBranch); pref("network.loadinfo.skip_type_assertion", true); to the test it self. Probably it is the best to debug it and see which check is failing and returning that error. From the log probably it is failing in asyncopen already.
(In reply to Dragana Damjanovic [:dragana] from comment #16) > You can add: > var prefs = Cc["@mozilla.org/preferences-service;1"]. > getService(Ci.nsIPrefBranch); > to the test it self. This is already present: https://dxr.mozilla.org/mozilla-central/source/netwerk/test/unit/test_authentication.js#8 I added prefs.setBoolPref("network.loadinfo.skip_type_assertion", true); as you had suggested in comment #14 and it made no difference: ERROR Component returned failure code: 0x805e0006 [nsIChannel.asyncOpen2]
Yes, the failure is on NetUtil.jsm::ioService.newChannelFromURI2, called from the first test, test_noauth. The interesting thing is that all the other tests pass if you comment out the first.
Even more interestingly, if you add a function test_noop() {} to the test list at the beginning, all the tests pass ;) So there seems to be some initialization race condition.
(In reply to aleth [:aleth] from comment #19) > Even more interestingly, if you add a function test_noop() {} to the test > list at the beginning, all the tests pass ;) > > So there seems to be some initialization race condition. Bah, I missed how these tests were being run (not with the usual add_test pattern). If you comment out a test, it disables all the following ones, so things trivially pass. Sorry the confusion.
OK, a little more detailed investigation shows that the failure stack is nsHttpChannel.cpp:5426 (nsHttpChannel::AsyncOpen2) nsContentSecurityManager.cpp:424 (nsContentSecurityManager::doContentSecurityCheck) and the failing content policy is mailnews/base/src/nsMsgContentPolicy.cpp, which makes it clear why this test failure is c-c specific. The failures in that file are, starting in nsMsgContentPolicy::ShouldLoad, WARNING: NS_ENSURE_SUCCESS(rv, nullptr) failed with result 0x80070057: file mailnews/base/src/nsMsgContentPolicy.cpp, line 660 nsMsgContentPolicy::GetMsgComposeForContext : fair enough, we're not in a compose window. WARNING: NS_ENSURE_SUCCESS(rv, NS_OK) failed with result 0x80070057: file mailnews/base/src/nsMsgContentPolicy.cpp, line 281 WARNING: NS_ENSURE_TRUE(aRequestingContext) failed: file mailnews/base/src/nsMsgContentPolicy.cpp, line 786 nsMsgContentPolicy::GetOriginatingURIForContext fails out of the gate because aRequestingContext is not provided here. Looks like this code long predates the aRequestPrincipal argument, which is not made use of.
This uses the aRequestPrincipal, but only cautiously when there is no alternative, as mail code (unfortunately) seems to be doing without it, and instead implementing something similar by hand. I'm not too familiar with principals, nor with mailnews C++ style, so please check ;)
Attachment #8763304 - Flags: review?(mkmelin+mozilla)
Assignee: nobody → aleth
Status: NEW → ASSIGNED
Component: General → Security
Product: Thunderbird → MailNews Core
Version: unspecified → Trunk
Comment on attachment 8763304 [details] [diff] [review] Use aRequestPrincipal as a fallback when available in the MsgContentPolicy Review of attachment 8763304 [details] [diff] [review]: ----------------------------------------------------------------- Yeah I think this looks correct. r=mkmelin with the below addressed ::: mailnews/base/src/nsMsgContentPolicy.cpp @@ +263,5 @@ > return NS_OK; > } > > // Extract the windowtype to handle compose windows separately from mail > + if (!!aRequestingContext) { remove the !! and the style here seems to be to have { on a new line @@ +277,5 @@ > > // Find out the URI that originally initiated the set of requests for this > // context. > nsCOMPtr<nsIURI> originatorLocation; > + if (!aRequestingContext && !!aRequestPrincipal) { remove the !!, move { to new line @@ +279,5 @@ > // context. > nsCOMPtr<nsIURI> originatorLocation; > + if (!aRequestingContext && !!aRequestPrincipal) { > + // Can get the URI directly from the principal. > + aRequestPrincipal->GetURI(getter_AddRefs(originatorLocation)); I think this should be rv = ...... NS_ENSURE_SUCCESS(rv, NS_OK); @@ +281,5 @@ > + if (!aRequestingContext && !!aRequestPrincipal) { > + // Can get the URI directly from the principal. > + aRequestPrincipal->GetURI(getter_AddRefs(originatorLocation)); > + } > + else { { on new line
Attachment #8763304 - Flags: review?(mkmelin+mozilla) → review+
https://hg.mozilla.org/comm-central/rev/d0257f4d94ce63b6b3820320374b30585cbf1e16 Bug 1278782 - Use aRequestPrincipal as a fallback when available in the MsgContentPolicy. r=mkmelin
(In reply to Magnus Melin from comment #23) > and the style here seems to be to have { on a new line ah yeah, looks like someone didn't like K&R ;) > > + aRequestPrincipal->GetURI(getter_AddRefs(originatorLocation)); > > I think this should be > > rv = ...... > NS_ENSURE_SUCCESS(rv, NS_OK); Good catch, thanks.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 50.0
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: