Open Bug 1609846 Opened 6 years ago Updated 4 years ago

IMAP ENVELOPE response parsing doesn't correctly handle RFC 2822 group address syntax (when mail.imap.use_envelope_cmd is set)

Categories

(MailNews Core :: Networking: IMAP, defect)

defect

Tracking

(Not tracked)

People

(Reporter: benc, Unassigned)

References

Details

(Keywords: reproducible)

(Followup from Bug 1609690)
This is pretty obscure, but I wanted to drop some breadcrumbs (hopefully I hit all the right keywords in the summary!)...

If the mail.imap.use_envelope_cmd pref is set, then TB will include the ENVELOPE data item in FETCH requests. This causes the server to send ENVELOPE data in it's response.
The addresses in the envelope response are parsed in nsImapServerResponseParser::parse_address().

And I don't think we're adhering strictly to IMAP RFC 3501.

So, for this message which has a group in the To: field:

From: Pete <pete@silly.example>
To: A Group:Chris Jones <c@a.test>,joe@where.test,John <jdoe@one.test>;
Cc: Undisclosed recipients:;
Date: Thu, 13 Feb 1969 23:32:54 -0330
Message-ID: <testabcd.1234@silly.example>

Testing.

I think most servers will parse the group into individual addresses for the ENVELOPE response, so something like:

11 UID fetch 933 (UID RFC822.SIZE FLAGS ENVELOPE BODY.PEEK[HEADER.FIELDS (Priority X-Priority References Newsgroups In-Reply-To Content-Type Reply-To)])
* 2 FETCH (UID 933 RFC822.SIZE 228 FLAGS () ENVELOPE ("Thu, 13 Feb 1969 23:32:54 -0330" "" (("Pete" NIL "pete" "silly.example")) (("Pete" NIL "pete" "silly.example")) (("Pete" NIL "pete" "silly.example")) (("Chris Jones" NIL "c" "a.test") (NIL NIL "joe" "where.test") ("John" NIL "jdoe" "one.test")) () () "" "<testabcd.1234@silly.example>") BODY[HEADER.FIELDS (PRIORITY X-PRIORITY REFERENCES NEWSGROUPS IN-REPLY-TO CONTENT-TYPE REPLY-TO)] {2}

)
11 OK UID FETCH completed

The To: field is:

(("Chris Jones" NIL "c" "a.test") (NIL NIL "joe" "where.test") ("John" NIL "jdoe" "one.test"))

And TB handles this just fine.

But the RFC suggests there's another encoding, which preserves the group name
https://tools.ietf.org/html/rfc3501#section-9

From reading the RFC, I'd guess we'd expect something like this:

(("A Group" NIL "A Group" NIL) ("Chris Jones" NIL "c" "a.test") (NIL NIL "joe" "where.test") ("John" NIL "jdoe" "one.test") (NIL NIL NIL NIL))

I could definitely have got that wrong. More research would be required, ideally capturing some examples from real servers.

Bug 1609690 occurred because someone was receiving a an envelope address with a NIL addr-mailbox from a server in the wild, which I'm assuming is a "end-of-group" marker.

Ben, I can duplicate the crash when I repair a folder with the hidden pref set. It occurs on the parse of the ENVELOPE response as you found. Also, your patch in bug 1609690 fixes it. I see this ENVELOPE response (from wireshark) causing the crash:

[Freesurfer] Three Postdoctoral or PhD positions in Stochastic Signal Processing/Computational Modelling fMRI/EEG (("Shankar Tumati" NIL "Shankar.Tumati" "theroyal.ca")) ((NIL NIL "freesurfer-bounces" "nmr.mgh.harvard.edu")) (("Freesurfer support list" NIL "freesurfer" "nmr.mgh.harvard.edu")) ((NIL NIL "Undisclosed recipients" NIL)(NIL NIL NIL NIL)) NIL NIL NIL "<CA41498B5757A04AB14837EAC5D752008105E32620@MAIL3-R.corp.rohcg.ca>") BODY[HEADER.FIELDS (PRIORITY X-PRIORITY REFERENCES NEWSGROUPS IN-REPLY-TO CONTENT-TYPE REPLY-TO TEXT)] {114}

I'm pretty sure it's the place where it says "Undisclosed recipients" highlighted above. If I add a printf like this

 if (ContinueParse()) {
      AdvanceToNextToken();
      char *mailboxName = CreateNilString();
      if (ContinueParse()) {
        AdvanceToNextToken();
        char *hostName = CreateNilString();
        AdvanceToNextToken();
        if (!mailboxName)
          printf("gds: addressLine=|%s|\n", addressLine.get());
        else
          addressLine += mailboxNam

it prints this for a single fetched header:

gds: addressLine=|Undisclosed recipients, |

In addition, only with use_envelope_cmd true, I see this assert for every message header fetched:

[114431, Main Thread] ###!!! ASSERTION: downloading hdrs for hdr we already have: 'Error', file /home/gene/mozilla/comm/mailnews/imap/src/nsImapMailFolder.cpp, line 2697
See Also: → 1609690
Keywords: reproducible
Severity: normal → S4
You need to log in before you can comment on or make changes to this bug.