1351966 - Assertion failure: !NS_IsAboutBlank(origin) (The inner URI for about:blank must be moz-safe-about:blank), at mozilla/caps/ContentPrincipal.cpp:144

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Jorg K (CEST = GMT+2)]

Assertion failure: !NS_IsAboutBlank(origin) (The inner URI for about:blank must be moz-safe-about:blank), at c:/builds/moz2_slave/tb-c-cen-w32-d-000000000000000/build/mozilla/caps/ContentPrincipal.cpp:144

Comes from bug 1340710 part 5:
https://hg.mozilla.org/mozilla-central/rev/69abc17ea32f#l1.12

Ehsan, could you please advise me on how to adapt Thunderbird to this change.

Comment 1

[(no longer active)]

The assertion here is trying to ensure that we never end up with a principal constructed for an about:blank URI, which seems to be what's happening here.  But to tell why that's happening, we need to know where that principal is coming from.  Do you have a call stack, or some more information  which can help explain where the principal in question has been created from?  I suspect we must be in the process of creating a principal here, i.e., from <http://searchfox.org/mozilla-central/rev/72fe012899a1b27d34838ab463ad1ae5b116d76b/caps/BasePrincipal.cpp#426> calling <http://searchfox.org/mozilla-central/rev/72fe012899a1b27d34838ab463ad1ae5b116d76b/caps/BasePrincipal.cpp#379> which takes us to <http://searchfox.org/mozilla-central/source/caps/ContentPrincipal.cpp#143> where the assertion is, so we should probably look at who is calling createCodebaseURI().  If the argument being passed to it is "about:blank", then that's the problem and it needs to be fixed (in that situation the code probably wants to create a null principal instead).  But let's discuss more.  :-)

(BTW I'm at a work week this week, apologies in advance if I lag in responding...)

Comment 2

[Jorg K (CEST = GMT+2)]

Thanks for the reply, Ehsan. I have the bug on the radar, so I'll dig thought it some more. Clearing NI for now.

Comment 3

[Jorg K (CEST = GMT+2)]

I've dug a bit though the logs and saw that this test fails:
mozmake SOLO_TEST=composition/test-blocked-content.js mozmill-one

When run manually that also conveniently prints a stack dump:
#01: mozilla::BasePrincipal::CreateCodebasePrincipal (c:\mozilla-source\comm-central\mozilla\caps\baseprincipal.cpp:380)
#02: nsScriptSecurityManager::GetChannelURIPrincipal (c:\mozilla-source\comm-central\mozilla\caps\nsscriptsecuritymanager.cpp:409)
#03: mozilla::net::nsChannelClassifier::StartInternal (c:\mozilla-source\comm-central\mozilla\netwerk\base\nschannelclassifier.cpp:394)
#04: mozilla::net::nsChannelClassifier::Start (c:\mozilla-source\comm-central\mozilla\netwerk\base\nschannelclassifier.cpp:311)
#05: nsBaseChannel::ClassifyURI (c:\mozilla-source\comm-central\mozilla\netwerk\base\nsbasechannel.cpp:319)
#06: nsBaseChannel::AsyncOpen (c:\mozilla-source\comm-central\mozilla\netwerk\base\nsbasechannel.cpp:696)
#07: nsBaseChannel::AsyncOpen2 (c:\mozilla-source\comm-central\mozilla\netwerk\base\nsbasechannel.cpp:708)
#08: imgLoader::LoadImage (c:\mozilla-source\comm-central\mozilla\image\imgloader.cpp:2248)
#09: nsContentUtils::LoadImage (c:\mozilla-source\comm-central\mozilla\dom\base\nscontentutils.cpp:3429)
#10: nsImageLoadingContent::LoadImage (c:\mozilla-source\comm-central\mozilla\dom\base\nsimageloadingcontent.cpp:893)
#11: nsImageLoadingContent::LoadImage (c:\mozilla-source\comm-central\mozilla\dom\base\nsimageloadingcontent.cpp:769)
#12: mozilla::dom::HTMLImageElement::SetAttr (c:\mozilla-source\comm-central\mozilla\dom\html\htmlimageelement.cpp:531)
#13: nsGenericHTMLElement::SetAttrHelper (c:\mozilla-source\comm-central\mozilla\dom\html\nsgenerichtmlelement.cpp:1590)
#14: nsMsgComposeAndSend::ProcessMultipartRelated (c:\mozilla-source\comm-central\mailnews\compose\src\nsmsgsend.cpp:1870)
#15: nsMsgComposeAndSend::HackAttachments (c:\mozilla-source\comm-central\mailnews\compose\src\nsmsgsend.cpp:2394)
#16: nsMsgComposeAndSend::Init (c:\mozilla-source\comm-central\mailnews\compose\src\nsmsgsend.cpp:3155)
#17: nsMsgComposeAndSend::CreateAndSendMessage (c:\mozilla-source\comm-central\mailnews\compose\src\nsmsgsend.cpp:4056)
#18: nsMsgCompose::SendMsgToServer (c:\mozilla-source\comm-central\mailnews\compose\src\nsmsgcompose.cpp:1267)
#19: nsMsgCompose::SendMsg (c:\mozilla-source\comm-central\mailnews\compose\src\nsmsgcompose.cpp:1467)

In all of mailnews/compose/src I can't see any reference to "about:blank". Also, I've surveyed the use of principals in mailnews recently, and we mostly use mull principals and on occasions also a system principal.

The MOZ_ASSERT() can also be triggered by hand easily:
Create a new message, drag any image into it, say, the little JK PNG right here on BMO, then "Send later", <CTRL><SHIFT><ENTER>. That crashes immediately. Oh, just (auto-)saving crashes, too, since that also goes through SendMsg().

The code that fails is this:
  nsCOMPtr<nsIURI> origin = NS_GetInnermostURI(aURI);
  if (!origin) {
    return NS_ERROR_FAILURE;
  }

  MOZ_ASSERT(!NS_IsAboutBlank(origin),
             "The inner URI for about:blank must be moz-safe-about:blank");

So somehow it gets an "innermost URI", no idea what that is, and decides that this is no good.

What I can tell you is that the URL of Thunderbirds compose window is about:blank (seen with DOM inspector). So what may be happening is that some image (nsContentUtils::LoadImage, see stack) is processed and that somehow retrieves the "innermost URI" and hits the URI of the compose window, although I would have expected that to be "outermost".

The compose window is created here:
https://dxr.mozilla.org/comm-central/rev/bee0697a1d3709df1db982f8320e7cac4a0b62c5/mail/components/compose/content/MsgComposeCommands.js#2726
editorElement.webNavigation.loadURI("about:blank", 0, null, null, null);

So maybe we change that to: moz-safe-about:blank, right?

Comment 4

[Jorg K (CEST = GMT+2)]

OK, editorElement.webNavigation.loadURI("moz-safe-about:blank", 0, null, null, null); didn't work at all. So next I tried:

     // Load empty page to create the editor.
-    editorElement.webNavigation.loadURI("about:blank", 0, null, null, null);
+    editorElement.webNavigation.loadURI("about:blank", // uri string
+                         0,                            // load flags
+                         null,                         // referrer
+                         null,                         // post-data stream
+                         null,                         // headers
+                         null,                         // base URI
+                         Services.scriptSecurityManager.createNullPrincipal({})
+                         );

at
https://dxr.mozilla.org/comm-central/rev/bee0697a1d3709df1db982f8320e7cac4a0b62c5/mail/components/compose/content/MsgComposeCommands.js#2726

No success, it crashes just the same on the same MOZ_ASSERT().

May I suggest that there is a problem in M-C code that somehow in the sequence of calls (comment #3) something goes wrong with the principal.

In reply to Ehsan from comment #1:
> we should probably look at who is calling createCodebaseURI()
Right there in M-C code.

Comment 5

[Jorg K (CEST = GMT+2)]

Ehsan, I was just working on bug 1347598 where certain embedded images (with cid:) don't work when they have a crossorigin="anonymous" attribute. I have a test case with two images, one with and one without that attribute.

With this test case currently no image shows. So I looked at other messages with embedded images and also no images show. So Thunderbird, being a program to show HTML (text+imaged) is pretty busted right now.

Comment 6

[Boris Zbarsky [:bzbarsky]]

> So maybe we change that to: moz-safe-about:blank, right?

No.  If you NS_NewURI("about:blank"), you will get a thing that is an outer URI wrapping an inner "moz-safe-about:blank" URI.

If you have an "about:blank" after NS_GetInnermostURI then someone went out of their way to create an about:blank URI via NOT calling NS_NewURI or equivalent.  webNavigation.loadURI should not be doing anything of the sort.

Now some questions:

1)  In your stack, what is the value passed to HTMLImageElement::SetAttr (frame 12)?  What attribute is being set?

2)  In your stack, what URI is passed to imgLoader::LoadImage (frame 8)?  What concrete class is it?

Comment 7

[Jorg K (CEST = GMT+2)]

Boris, I was thinking of you ;-) You're watching from the distance.

1) src = u"cid:[e-mail]"
   So during send, we replace data: URIs with cid:
   BTW, that's the stuff that the embedded images use that don't work right now.
2) [mozilla::net::nsSimpleURI] = {mRefCnt={mValue=0x0000000000000009 }
   _mOwningThread={mThread=0x0000000000911800 } mScheme={...} ...}
   No idea how to get the spec in the Windows debugger. I might need to add a print.
   I guess it's the stuff above shown in src.

Comment 8

[Jorg K (CEST = GMT+2)]

2) Sorry, wrong, this is from the mScheme
   mozilla::detail::nsCStringRepr = {mData=0x0000000015622ba8 "about" mLength=0x00000005 mFlags=0x00000005 }
   and mPath:
   mozilla::detail::nsCStringRepr = {mData=0x0000000015622bb8 "blank" mLength=0x00000005 mFlags=0x00000005 }
   So: about:blank.

Comment 9

[Boris Zbarsky [:bzbarsky]]

> 1) src = u"cid:[e-mail]"

OK.  That seems reasonable, back in frame 12.  But then by frame 8 we have about:blank.  How did that happen?  What were the things passed to frames 9, 10, 11?

Most simply, once you're in frame 12, can you breakpoint on the nsSimpleURI constructor and see who's calling it and with what args?  nsSimpleURI for the "cid:stuff" string would make sense, but how did it end up with "about:blank"?

Comment 10

[Jorg K (CEST = GMT+2)]

Set attribute calls LoadImage at HTMLImageElement.cpp:529 with
mData = 0x0000000008792308 u"cid:[e-mail]"

Inside nsImageLoadingContent::LoadImage() at nsImageLoadingContent.cpp:732 I see a
nsresult rv = StringToURI(aNewURI, doc, getter_AddRefs(imageURI));
The imageURI that comes out there seems to be about:blank.

So it looks to me that the StringToURI() turns the cid: into about:blank.

cid: isn't a real scheme, or is it?

Anyway, 2 AM here now, time to leave it until some other time.

Comment 11

[Jorg K (CEST = GMT+2)]

OK, from nsImageLoadingContent::LoadImage():

  // Parse the URI string to get image URI
  nsCOMPtr<nsIURI> imageURI;
  nsresult rv = StringToURI(aNewURI, doc, getter_AddRefs(imageURI));
  nsAutoCString u; imageURI->GetSpec(u);
  printf("=== nsImageLoadingContent::LoadImage: %s %s\n", NS_ConvertUTF16toUTF8(aNewURI).get(), u.get());
  NS_ENSURE_SUCCESS(rv, rv);

gives:
=== nsImageLoadingContent::LoadImage: cid:[e-mail] about:blank

So that answers where the "about:blank" comes from. This is the code path of the crash from comment #3.

As noted previously, messages containing embedded cid: images silently don't display those at all, no crash. I haven't tracked down why they get lost. I hope that addressing the problem here will also fix the missing display.

Comment 12

[Boris Zbarsky [:bzbarsky]]

> cid: isn't a real scheme, or is it?

It is in Thunderbird, in the sense of having a protocol handler.  That StringToURI ends up calling into http://searchfox.org/comm-central/source/mailnews/base/src/nsCidProtocolHandler.cpp#38 (aka nsCidProtocolHandler::NewURI) which does:

  nsCOMPtr<nsIURI> url = do_CreateInstance(NS_SIMPLEURI_CONTRACTID, &rv);
  rv = url->SetSpec(nsDependentCString("about:blank"));

This is bogus, of course, and directly leads to the asserts you see.

Now you could do:

  nsCOMPtr<nsIURI> url;
  rv = NS_NewURI(getter_AddRefs(url), "about:blank");

to at least produce an about:blank URI in a proper way.  That would solve your MOZ_ASSERT problems.  I don't see how it would solve your "not displayed" problems, but those aren't what this bug is about.

Comment 13

[Jorg K (CEST = GMT+2)]

(In reply to Boris Zbarsky [:bz] (still a bit busy) (if a patch has no decent message, automatic r-) from comment #12)
> I don't see how it would solve your "not
> displayed" problems, but those aren't what this bug is about.
Thank you so much, yes, that fixes the crash, but embedded images still don't show, I'll have to chase that up separately in bug 1352701.

Comment 14

[Jorg K (CEST = GMT+2)]

Attachment: 1351966-image-crash.patch

Thanks for your contribution Boris ;-) I know you've worked on TB since the early days. I took the liberty to make you the author here.

Comment 15

[Jorg K (CEST = GMT+2)]

https://hg.mozilla.org/comm-central/rev/2f8bd4ff6cd14b4869b3bd2aa5964e386b7456d7

Comment 16

[Jorg K (CEST = GMT+2)]

Looks like I was wrong here, not caused by bug 1340710 which landed in early March. It looks more like bug 1347817.

Comment 17

[Jorg K (CEST = GMT+2)]

Sorry, I meant to change the commit message from:
Fix bogus creation of nsIURI for about:blank. r=jorgk to
Fix incorrect creation of nsIURI for about:blank. r=jorgk.
Sigh, Mercurial is forever :-(