"ResponseParseError: unexpected token NUMBER (expected ATOM)" on old exchange mail servers

[hoffi]

Needed to work with an old exchange mail server. The ID command reports: ("name" "Microsoft.Exchange.Imap4.Imap4Server" "version" "15.0") which according to this microsoft document should be an Exchange 2013 server.

After i sent an UID MOVE command it failed with the mentioned ResponseParserError.
I enabled the debug logging and got the following output:

[...]
S: RUBY0003 OK [READ-WRITE] SELECT completed.
C: RUBY0004 UID MOVE 10 PROCESSING
S: [COPYUID 124 10 233]
# (net-imap crashes at this point)
S: * 1 EXPUNGE
S: * 69 EXISTS
S: RUBY0004 OK MOVE completed.

As i understand RFC 2359 there should be an OK in front of the COPYUID?

Should the parser be able to handle such incorrect responses?
I have managed to build a hack for the parser locally to get it parsed and could submit a PR for the fix after some refactoring.

I have also checked against the Exchange implementation for Microsoft 365 and this does not have the error. The ID command reports: ("name" "Microsoft.Exchange.Imap4.Imap4Server" "version" "15.20") here.

[nevans]

(Sorry for the slow reply)

Yeah, every response must start with one of:

• + (for a continuation)
• * (for an untagged response) followed by a status (OK, BAD, or NO) and then the response code [COPYUID 124 10 233] and any other (human readable, non-parsable) text.
• or a "tag" (basically an identifier atom, chosen by the client)

It boggles my mind how often servers, including the ones from the biggest software companies in the world, fail at the absolute basics of the protocol. I do think that Net::IMAP should be more resilient to buggy responses, in general. But I'm hesitant to include something this far out-of-spec into the parser... yet. I've actually been working through basic quoted string escape bugs from a certain fruit-named company's iMAP iServer, for the last week or so. Ultimately, I don't think I'll even be pushing a PR for that workaround either. 🙁

[nevans]

I do have other ideas for improving robustness:

• In some cases, the IMAP specs provides more support for "extension" grammar than we are currently taking advantage of. In v0.4.3 and v0.4.5, I added ExtensionData, UnparsedData, and UnparsedNumericResponseData, and these can be utilized much more than they currently are.
• The client should simply be more robust to errors. Currently, any exception in the receiver thread will close the connection. For errors in the basic protocol handling code, there really isn't any safe way other than dropping the connection. But, most of the time, an error in the parser only needs to crash the currently running commands, not the entire connection. However we need to be very careful with this: ignoring small errors can cascade into much bigger errors. It's easy to imagine ignored errors leading to downloading corrupted data. In the worst case, ignoring parser errors could lead to a connection continuing to work even after TLS errors or authentication errors and enable a man-in-the-middle attack.
• I'd like to officially support server-specific "quirks mode" parsing. At the very least, you should be able to supply your own parser when you instantiate a client. And we can add server-specific hacks into parser subclasses or into modules that are prepended to the parser. This is what we're doing at my workplace, and I've seen this in several open source projects that use net-imap as well. This is also commonly done in other mature IMAP client libraries written in other languages. I'd like to improve the ways that these "quirks mode" server-specific hacks are supported and tested, even including built-in server-specific hacks.
• In cases where the violations are very small or insignificant, or where they aren't too big but very common, and where the workarounds aren't too far out-of-spec, we can put workarounds directly into the parser. I'd like to avoid using this approach too often though. It makes parser code more complex, and that sometimes leads to bugs or reduced performance or makes it harder to fix bugs or add extensions for servers who are following the spec. But I have added several of these recently, and I know we'll add many more. 🙂
• And if a workaround is a significant change to the spec, but is still seen in popular servers, that's another place where I might use Net::IMAP::UnparsedData or ExtensionData or some other data structure that's explicitly marked as "buggy". I have a patch for "foolproof BODYSTRUCTURE parsing" which I'll (eventually) get around to converting into a PR... and in the worst cases it falls back to using UnparsedData (even though it is partially parsed).

So, although I'm reluctant to include it in the generic parser code, I'm curious to see what you came up with, and how you solved this for yourself. If you post something (in this ticket or in a PR), at the very least that documents the workaround for others who get to this issue via search engines.

[nevans]

Oh, although I doubt that it makes any difference for this issue, you should use RFC4315 for UIDPLUS (or RFC9051, which includes UIDPLUS). RFC2359 is obsolete (and was already obsolete for a nearly decade by 2013). But it is sometimes informative to read the obsolete RFCs, especially when dealing with very old servers. 🙂

[hoffi]

(Sorry for the slow reply)

No worries :)

So, although I'm reluctant to include it in the generic parser code, I'm curious to see what you came up with, and how you solved this for yourself. If you post something (in this ticket or in a PR), at the very least that documents the workaround for others who get to this issue via search engines.

I have got it working using these changes. But i am not sure if there are any unwanted side effects with other servers and so on.

diff --git a/lib/net/imap/response_parser.rb b/lib/net/imap/response_parser.rb
index 1aab798..d2dc08e 100644
--- a/lib/net/imap/response_parser.rb
+++ b/lib/net/imap/response_parser.rb
@@ -655,6 +655,7 @@ module Net
         resp = case lookahead!(T_PLUS, T_STAR, *TAG_TOKENS).symbol
                when T_PLUS then continue_req
                when T_STAR then response_data
+               when T_LBRA then invalid_exchange_response
                else             response_tagged
                end
         accept_spaces # QUIRKY: Ignore trailing space (MS Exchange Server?)
@@ -776,6 +777,10 @@ module Net
       alias comparator_data         response_data__unhandled
       alias message_data__converted response_data__unhandled
 
+      def invalid_exchange_response
+        UntaggedResponse.new(OK, resp_text, @str)
+      end
+
       # RFC3501 & RFC9051:
       #   response-tagged = tag SP resp-cond-state CRLF
       def response_tagged

However i do not use this parser change anymore as i have seen that it seems to work with the UID COPY command.
Basically i now use UID COPY to copy it to the new mailbox and after that i use UID STORE and UID EXPUNGE to delete the old message to mimic a MOVE that way:

@imap.select(source_mailbox)
response = @imap.uid_copy(uid_to_copy, destination_mailbox)
@imap.uid_store(uid_to_copy, '+FLAGS', [Net::IMAP::DELETED])
@imap.uid_expunge(uid_to_copy)
uid_after_copy = response.data.code.data.assigned_uids.first

Ironically the UID COPY also responds with a COPYUID, but for whatever reason it is correct in that case, so it works without any parser errors. Strange.

Here the debug log for this:

[...]
S: RUBY0009 OK [READ-WRITE] SELECT completed.
C: RUBY0010 UID COPY 58 PROCESSED
S: RUBY0010 OK [COPYUID 6017 58 295] COPY completed.
C: RUBY0011 UID STORE 58 +FLAGS (\Deleted)
S: * 1 FETCH (FLAGS (\Deleted))
S: RUBY0011 OK STORE completed.
C: RUBY0012 UID EXPUNGE 58
S: * 1 EXPUNGE
S: * 66 EXISTS
S: RUBY0012 OK EXPUNGE completed.
C: RUBY0013 LOGOUT
S: * BYE Microsoft Exchange Server 2013 IMAP4 server signing off.
S: RUBY0013 OK LOGOUT completed.

At the very least, you should be able to supply your own parser when you instantiate a client. And we can add server-specific hacks into parser subclasses or into modules that are prepended to the parser.

That sounds interesting. Also the existing hacks in the net-imap gem could be extracted into such modules and can be included by the user when needed?