1338794 - |make mozmill| error: NS_ERROR_NOT_AVAILABLE: Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE) [nsIMIMEService.getTypeFromURI]

Status: RESOLVED FIXED

(Thunderbird :: Message Compose Window, defect)

Description

[ISHIKAWA, Chiaki]

Attachment: t4.log

During |make mozmill| test, I see the following error three times,

JavaScript error: chrome://messenger/content/messengercompose/MsgComposeCommands.js, line 5659: NS_ERROR_NOT_AVAILABLE: Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE) [nsIMIMEService.getTypeFromURI]

The error comes from the following test:

TEST-START | /NREF-COMM-CENTRAL/comm-central/mail/test/mozmill/composition/test-forward-utf8.js | test_utf8_forwarding_from_via_folder

The first time I saw this error is late December 2016 and the last time I did not see this
error is November  2016. So the bug must have crept in sometime late November and December.

I am attaching a log of local run of this test (running DEBUG version of C-C TB).

TIA

Comment 1

[ISHIKAWA, Chiaki]

It is possible that the bug had been masked earlier by NOT EXECUTING this particular test because
earlier test in the same test directory /composition/ caused time out and running the tests in this directory was aborted.

Comment 2

[Jorg K (CEST = GMT+2)]

I've seen this, too:
  let contentType = Components.classes["@mozilla.org/mime;1"]
    .getService(Components.interfaces.nsIMIMEService)
    .getTypeFromURI(uri);

That was added by Magnus in early December. We should dump uri.spec to check what's going on.

Comment 3

[ISHIKAWA, Chiaki]

I added the following dump and got

aURL	 = mailbox:///NREF-COMM-CENTRAL/objdir-tb3/_tests/mozmill/mozmillprofile/Mail/Local%20Folders/FolderWithUTF8?number=1&part=1.2
uri	 = [xpconnect wrapped nsIURI @ 0x55a087529970 (native @ 0x55a083f05e70)]
uri.spec = mailbox:///NREF-COMM-CENTRAL/objdir-tb3/_tests/mozmill/mozmillprofile/Mail/Local%20Folders/FolderWithUTF8?number=1&part=1.2

and similar. (The last one has 1.2.2 instead of 1.2.)
My suspicion is that such an uri.spec does not have any MIME-related information at all!
Should we not go inside the e-mail's "part" to find out a bit more?

Added dump statements:
/**
 * Convert the blocked content to a data URL and swap the src to that for the
 * elements that were using it.
 * @param {String} aURL - (necko) URL to unblock
 */
function loadBlockedImage(aURL) {
  let filename;
  if (/^file:/i.test(aURL)) {
    filename = aURL.substr(aURL.lastIndexOf("/") + 1);
  }
  else {
    let fnMatch = /[?&;]filename=([^?&]+)/.exec(aURL);
    filename = (fnMatch && fnMatch[1]) || "";
  }
  filename = decodeURIComponent(filename);
  let uri = Services.io.newURI(aURL);
  // debug
  dump("filename = " + filename +"\n");
  dump("aURL     = " + aURL + "\n");
  dump("uri      = " + uri + "\n");
  dump("uri.spec = " + uri.spec + "\n");

  let contentType = Components.classes["@mozilla.org/mime;1"]
    .getService(Components.interfaces.nsIMIMEService)
    .getTypeFromURI(uri);
  if (!contentType.startsWith("image/")) {
    // Unsafe to unblock this. It would just be garbage either way.
    throw new Error("Won't unblock; URL=" + aURL +
                    ", contentType=" + contentType);
  }

Comment 4

[ISHIKAWA, Chiaki]

Attachment: log with the debug dump

log with the debug dump.

Comment 5

[ISHIKAWA, Chiaki]

Now I notice we are opening the URI as stream and reading from it several lines below.

Probably we should do the check for "Does it start with image" when we read it, and remove the offending
getTypeFromURI line once for all?

Comment 6

[Jorg K (CEST = GMT+2)]

Magnus, you code
function loadBlockedImage(aURL) {
...
  let contentType = Components.classes["@mozilla.org/mime;1"]
    .getService(Components.interfaces.nsIMIMEService)
    .getTypeFromURI(uri);
is throwing an error in some unrelated test (test-forward-utf8.js). Chiaki got us the details:

filename =
aURL	 = mailbox:///NREF-COMM-CENTRAL/objdir-tb3/_tests/mozmill/mozmillprofile/Mail/Local%20Folders/FolderWithUTF8?number=1&part=1.2
uri.spec = mailbox:///NREF-COMM-CENTRAL/objdir-tb3/_tests/mozmill/mozmillprofile/Mail/Local%20Folders/FolderWithUTF8?number=1&part=1.2
JavaScript error: chrome://messenger/content/messengercompose/MsgComposeCommands.js, line 5666: NS_ERROR_NOT_AVAILABLE: Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE) [nsIMIMEService.getTypeFromURI]

The messages used by the test (content-utf8-rel-only.eml, content-utf8-alt-rel.eml, content-utf8-rel-alt.eml) have various MIME structures and all contain a small image.

When I forward them, I get this error and the image is missing from the forwarded message. The problem is that the messages are missing a filename in the header. I'll fix this.

Comment 7

[Jorg K (CEST = GMT+2)]

Attachment: 1338794-fix-test-add-filenames.patch

Magnus, I think it's safe to assume that these hand-crafted messages do not reflect what's produced by TB which always adds a filename.

Or do you want to change your code to cater for this case?

Comment 8

[ISHIKAWA, Chiaki]

(In reply to Jorg K (GMT+1) from comment #7)
> Created attachment 8836590 [details] [diff] [review]
> 1338794-fix-test-add-filenames.patch
> 
> Magnus, I think it's safe to assume that these hand-crafted messages do not
> reflect what's produced by TB which always adds a filename.
> 
> Or do you want to change your code to cater for this case?

What happens if someone sends us an attachment WITHOUT filename ?
(I hope TB does not crash or do something funny.)

Comment 9

[Jorg K (CEST = GMT+2)]

(In reply to ISHIKAWA, Chiaki from comment #8)
> What happens if someone sends us an attachment WITHOUT filename ?
> (I hope TB does not crash or do something funny.)
Well, it will do something funny, as you can see in the test. Hence the question to Magnus:
  Or do you want to change your code to cater for this case?

Actually, no need to run the test. Insert any of the three mentioned messages into a folder and click "forward". The forwarded message will not contain the embedded image.

Comment 10

[ISHIKAWA, Chiaki]

Thank you for the clarification. In that case,
>   Or do you want to change your code to cater for this case?
seems to be a prudent way to go.
We never know what kind of warped messages the other parties will send us. Such is life.

Comment 11

[Jorg K (CEST = GMT+2)]

Comment on attachment 8836590 [details] [diff] [review]
1338794-fix-test-add-filenames.patch

Cancelling r?

We still need to fix MsgComposeCommands.js:loadBlockedImage() so it doesn't produce a JS error. Maybe in this case we don't want to correct the test e-mail after all.

I'm just not sure whether there is a way to convert an image that doesn't have a filename. Note that without content type we can't create the data URI:
  let dataURL = "data:" + contentType + ...

Magnus, what's your opinion? Would you like to take the bug or tell me what to do?

Comment 12

[Magnus Melin [:mkmelin]]

I think you can get away with just using image/png if you can't figure out what mime type it is. Not super nice, but probably the best you can do. Image decoding should be able to figure it out even with the wrong mime type, as long as it's image/.

Comment 13

[Jorg K (CEST = GMT+2)]

Attachment: 1338794-fix-mimetype.patch

Like this?

Comment 14

[Magnus Melin [:mkmelin]]

Comment on attachment 8836857 [details] [diff] [review]
1338794-fix-mimetype.patch

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

Yes, assuming it works ;)

::: mail/components/compose/content/MsgComposeCommands.js
@@ +5672,5 @@
> +      // Unsafe to unblock this. It would just be garbage either way.
> +      throw new Error("Won't unblock; URL=" + aURL +
> +                      ", contentType=" + contentType);
> +    }
> +  } else {

Style here is else on a new line

Comment 15

[Jorg K (CEST = GMT+2)]

(In reply to Magnus Melin from comment #14)
> Yes, assuming it works ;)
That depends on the definition. Yes, forwarding the test message which alerted us to the problem works again. Giving the wrong MIME type might cause problems on the recipient side.

> > +  } else {
> Style here is else on a new line
Hmm, that seems to be your personal style which isn't really used much. I also can't see this in the coding style:
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style

I guess since we have to live with your minority style you'd have to life with our majority style or send the patches yourself ;-)

Comment 16

[Jorg K (CEST = GMT+2)]

(In reply to Jorg K (GMT+1) from comment #15)
> (In reply to Magnus Melin from comment #14)
> > Yes, assuming it works ;)
> That depends on the definition. Yes, forwarding the test message which
> alerted us to the problem works again. Giving the wrong MIME type might
> cause problems on the recipient side.
OK, I modified one of the test e-mails and replaced the base64 encoded PNG with a base64 encoded JPEG. Reply and forward work now, but the data in the resulting message says PNG, for example in the attachment header: Content-Type: image/png;

Surely image viewers inspect the content of the image and don't trust any secondary information, but what we're doing here isn't really nice since we're shipping out wrong data. The original message did in fact have the right information in the attachment header, but of course that's long gone by the time we get this stuff into the compose window because there only the Mailnews URL arrives: uri.spec = mailbox:// ... /mozmillprofile/Mail/Local%20Folders/FolderWithUTF8?number=1&part=1.2

Maybe apart from landing this, we should drill open MIME a bit and let it invent a filename if there wasn't one that at least carries the extension from the Content-type header. What do you think? Of course digging around in MIME isn't pleasant. Or maybe that's a bad idea since the result would be a Mailnews URL with an invented filename which isn't present in the attachment to later access via that URL might fail.

Oh well, let's just hope there isn't too much faulty data out there there in the wild. Does Outlook attach embedded images with a proper filename in the attachment header even if the user pasted some clipboard data?

Comment 17

[ISHIKAWA, Chiaki]

TB under windows produces a autogenerated filename when I insert a graphics JPEG from cliboard into HTML e-mail.
I don't use outlook and so I don't know about it.

Comment 18

[Jorg K (CEST = GMT+2)]

I got Kent to send me an e-mail with a pasted image from the clipboard. Result:

------=_NextPart_000_00CE_01D2864A.97F56770
Content-Type: image/png;
	name="image001.png"
Content-Transfer-Encoding: base64
Content-ID: <image001.png@01D2864A.9764E740>

So I wouldn't expect too much faulty data out in the wild. Kent's message can be forwarded with no problem without fixing this bug here.

Comment 19

[Jorg K (CEST = GMT+2)]

(In reply to Magnus Melin from comment #14)
> Style here is else on a new line
Count in MsgComposeCommands.js:
|} else {|: 9
|  else {|: 10, five of which you added :-(

OK, I'll put it on a new line.

Comment 20

[Jorg K (CEST = GMT+2)]

C-C (TB 54): https://hg.mozilla.org/comm-central/rev/bc46559b2c2b72539fbe85febe552607709f6b03
C-A (TB 53): https://hg.mozilla.org/releases/comm-aurora/rev/dd13b30a9fe156c1814355e8c0ccf3635bae7650

Comment 21

[Jorg K (CEST = GMT+2)]

Beta (TB 52):
https://hg.mozilla.org/releases/comm-beta/rev/1d432eff0559d3e04bfdd0f2233c16bd5730ccab