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)
MailNews Core
Security
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.
Assignee | ||
Comment 1•8 years ago
|
||
I can't find any information on that failure code. Dragana, any ideas on what could be causing this?
Flags: needinfo?(dd.mozilla)
Comment 2•8 years ago
|
||
(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)
Assignee | ||
Comment 3•8 years ago
|
||
(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.
Comment 4•8 years ago
|
||
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
Assignee | ||
Comment 5•8 years ago
|
||
(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
Comment 6•8 years ago
|
||
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)
Comment 7•8 years ago
|
||
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)
Comment 8•8 years ago
|
||
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.
Comment 9•8 years ago
|
||
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)
Comment 10•8 years ago
|
||
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.
Assignee | ||
Comment 11•8 years ago
|
||
Maybe the default content policy differs? Not sure offhand where that is set.
Comment 12•8 years ago
|
||
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)
Comment 13•8 years ago
|
||
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?)
Comment 14•8 years ago
|
||
If you change pref:
prefs.setBoolPref("network.loadinfo.skip_type_assertion", true);
it should fix this if it is a policy issue.
Comment 15•8 years ago
|
||
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.
Comment 16•8 years ago
|
||
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.
Comment 17•8 years ago
|
||
(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]
Assignee | ||
Comment 18•8 years ago
|
||
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.
Assignee | ||
Comment 19•8 years ago
|
||
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.
Assignee | ||
Comment 20•8 years ago
|
||
(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.
Assignee | ||
Comment 21•8 years ago
|
||
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.
Assignee | ||
Comment 22•8 years ago
|
||
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 | ||
Updated•8 years ago
|
Assignee: nobody → aleth
Status: NEW → ASSIGNED
Assignee | ||
Updated•8 years ago
|
Component: General → Security
Product: Thunderbird → MailNews Core
Version: unspecified → Trunk
Comment 23•8 years ago
|
||
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+
Assignee | ||
Comment 24•8 years ago
|
||
https://hg.mozilla.org/comm-central/rev/d0257f4d94ce63b6b3820320374b30585cbf1e16
Bug 1278782 - Use aRequestPrincipal as a fallback when available in the MsgContentPolicy. r=mkmelin
Assignee | ||
Comment 25•8 years ago
|
||
(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.
Assignee | ||
Updated•8 years ago
|
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.
Description
•