2088009 - IMAP connection keeps being used after a response with an unexpected tag, so every later response goes to the wrong command

Status: RESOLVED FIXED

(MailNews Core :: Networking: IMAP, defect)

Description

[Magnus Melin [:mkmelin]]

When the tagged response that ends a command doesn't carry that command's tag, nsImapServerResponseParser::response_done() treats it as fatal: resp_cond_bye() disconnects the parser and sets the state to kNonAuthenticated. But the server hasn't closed the connection, and the next command calls SetConnected(true) again in ParseIMAPServerResponse(). nsImapProtocol::ProcessCurrentURL() only checks Connected() at the end of a URL, so a mismatch in the middle of one is forgotten, and the connection goes on with each response taken as the one to the previous command.

Whatever causes a mismatch, the connection can't be trusted after it. It should be closed and the URL failed, so that the next URL gets a new connection and a failed discovery doesn't remove folders.

Comment 1

[Magnus Melin [:mkmelin]]

Attachment: Bug 2088009 - Drop an IMAP connection once a response has another command's tag. r=#thunderbird-back-end-reviewers

A tagged response that doesn't match the command disconnects the parser, but
the next command reconnected it, and ProcessCurrentURL() only checks the
connection at the end of a URL. So the connection went on with every response
taken as the one to the previous command. With bug 2075517 the parser ignores
most untagged responses while unauthenticated, which it also is after such a
response, so folder discovery saw no folders and removed the unverified ones.

ParseIMAPandCheckForNewMail() now closes the connection and fails the URL as
soon as a response has an unexpected tag. MailboxDiscoveryFinished() then
doesn't call DiscoveryDone(), and the next URL uses a new connection. A BYE is
handled as before, as the server closes the connection itself.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/528dc2df8edc
Drop an IMAP connection once a response has another command's tag. r=edicharry

Comment 3

[Magnus Melin [:mkmelin]]

Comment on attachment 9672396 [details]
Bug 2088009 - Drop an IMAP connection once a response has another command's tag. r=#thunderbird-back-end-reviewers

Uplift Approval Request

• Please state case for uplift consideration and ensure bug severity is set: Regression
• User impact if declined: It's a companion bug to bug 2087454. That one fixed the visible symptom we know of.
  This one may be causing unknown issues. We may take it or not. I think taking it would be slightly safer.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Daily?: Yes
• Has the fix been verified in Beta?: No
• Needs manual test from QA?: No
• If yes, steps to reproduce:
• List of other uplifts needed: 2087454 (already landed)
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky):
• Does the fix cause any migrations to be skipped?: No
• String changes made/needed: none

Comment 4

[Corey Bryant]

Comment on attachment 9672396 [details]
Bug 2088009 - Drop an IMAP connection once a response has another command's tag. r=#thunderbird-back-end-reviewers

[Triage Comment]
Sorry, this missed the final beta.