Open Bug 1158865 Opened 5 years ago Updated 4 years ago

Thunderbird creating invalid application/applefile attachments

Categories

(MailNews Core :: MIME, defect)

x86
macOS
defect
Not set

Tracking

(Not tracked)

UNCONFIRMED

People

(Reporter: hsk, Unassigned)

Details

(Whiteboard: [patchlove][haspatch])

Attachments

(1 file, 1 obsolete file)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:37.0) Gecko/20100101 Firefox/37.0
Build ID: 2015041600

Steps to reproduce:

using recent thunderbird on recent macosx (intel-based), sent a mail message with .cadx file attached - attachment was created as "multipart/appledouble".


Actual results:

mail server software of that north-east-us-company rejects that mail message.
error message is "MIME content error: Wrong magic number in Apple MIME
attachment".


Expected results:

mail server should have accepted mail message :-)

reason of failure seems to be that intel-based macosx-thunderbird generates
application/applefile (sub-)attachment with integer fields lsb-ordered,
whereas rfc1740 (format of application/applefile attachment etc.) states:
"byte ordering in the file fields follows mc68000 conventions, most
significant byte first".  there seems to be an additional issue regarding
byte alignment, see below.

said mail server parses (for whatever reason) the application/applefile
attachment, does this according to rfc1740, thinks to see only garbage,
and rejects the message.

thunderbird source code for creation of an application/applefile
attachment is in mailnews/compose/src/nsMsgAppleEncode.cpp.  the
code assigns 4-byte-integers to a write-buffer and then bytewise
base64-encodes it.  as recent intel-based macs use little-endian byte
order, this obviously results in an application/applefile attachment
with lsb-ordered integer fields.
to solve the described error, in the assignments in lines 180 and up,
uint32 and uint16 entities should be wrapped with htonl() and htons(),
e.g.,
  head.magic = htonl(APPLEDOUBLE_MAGIC);
  head.entries = htons(NUM_ENTRIES - 1);
at least for head and entries[].  not sure about the "finder info" data
and the "dates" data, and the actual resource fork data.  but then this
kind of data seems useful (and used) on mac only, and so will be just
transfer of (binary) data from mac to mac, and they will get it right
(nowadays there may not exist notable amounts of non-lsb-macs :-)

first part of the attached "patchfile" has the proposed changes to
mailnews/compose/src/nsMsgAppleEncode.cpp.  similar changes should
probably be made to attachment decoding routines.  but the code in
mozilla/uriloader/exthandler/mac/nsDecodeAppleFile.cpp seems not to be
used at all when building thunderbird.  patch to that file is included
though.
when looking at application/applefile attachments created by
macosx-thunderbird there is another discrepancy to rfc1740.  rfc says:
  [the number-of-entries field] is an unsigned 16-bit number.  if the
  number of entries is any number other than 0, then that number of entry
  descriptors immediately follows the number of entries field.
but in the created attachment there are two random bytes following the
number-of-entries field and then come the entry descriptions.  i suspect
this is caused by
  #pragma options align=mac68k
in mailnews/compose/src/nsMsgAppleCodes.h (and as well in
mozilla/uriloader/exthandler/mac/nsDecodeAppleFile.cpp, if that file is
used in building thunderbird at all, see above).  as the pragma could cause
the "head" structure that is used as write buffer before base64-encoding
and whose last element is "int16_t entries", holding the number of
entries, to be padded to a multiple of 4byte. imho, that pragma just should
not be used here.  see second part of "patchfile".

maybe creation of appledouble attachments should be dropped anyway - or be made configurable to disable...
Attachment #8598056 - Attachment description: patchfile → patchfile - for an updated version see below
Attachment #8598056 - Attachment is obsolete: true
update to the patchfile that was submitted with the initial bug report
(In reply to hsk from comment #1)
> Created attachment 8598472 [details] [diff] [review]
> patch "applefile" attachment creation
> 
> update to the patchfile that was submitted with the initial bug report

Hi hsk.

Please see instructions on gthe correct format of a patch on https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F

Once that it is correctly formatted (it will help get a quicker review), please requesta  review to the correct person. Your patch is modifying file http://mxr.mozilla.org/comm-central/source/mailnews/compose/src/nsMsgAppleEncode.cpp, so :standard8 could be one possible person.
Whiteboard: [haspatch]
OS: Unspecified → Mac OS X
Hardware: Unspecified → x86
Summary: macosx-thunderbird creating invalid application/applefile attachments → Thunderbird creating invalid application/applefile attachments
Component: Untriaged → MIME
Product: Thunderbird → MailNews Core
(In reply to Javi Rueda from comment #2)
> (In reply to hsk from comment #1)
> > Created attachment 8598472 [details] [diff] [review]
> > patch "applefile" attachment creation
> > 
> > update to the patchfile that was submitted with the initial bug report
> 
> Hi hsk.
> 
> Please see instructions on gthe correct format of a patch on
> https://developer.mozilla.org/en-US/docs/
> Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-
> in_for_me.3F
> 
> Once that it is correctly formatted (it will help get a quicker review),
> please requesta  review to the correct person. Your patch is modifying file
> http://mxr.mozilla.org/comm-central/source/mailnews/compose/src/
> nsMsgAppleEncode.cpp, so :standard8 could be one possible person.
Flags: needinfo?(hsk)
Hi reporter.

I thought that by .cadx you meant DICOM files, those which can be opened by Ginkgo CADx, for example. I tested to send a file from http://www.osirix-viewer.com/datasets/ to myself and it worked fine. MIME type was application/dicom.

Anyway, I think that this bug should be tested with any file type which has not any assigned MIME type to confirm that file is attached as multipart/appledouble as expected when no MIME type especific to the file type had been assigned on the MIME standard RFC.
hsk seems to be gone
Flags: needinfo?(hsk) → needinfo?(leofigueres)
Whiteboard: [haspatch] → [patchlove][haspatch]
It seems that the decoding part was fixed long ago. See https://bugzilla.mozilla.org/show_bug.cgi?id=361401#c16

I am still trying to find a testcase in order to test this bug and confirm it.
This bug is on my (mental) list of bugs to follow-up, soI remove the needinfo flag.
Flags: needinfo?(leofigueres)
You need to log in before you can comment on or make changes to this bug.