Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle unsolicited "* OK Searched 91% of the mailbox, ETA 0:01" message #179

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

paulpr0
Copy link

@paulpr0 paulpr0 commented Feb 22, 2021

When fetching a large number of UIDs, rust-imap would sometimes produce an error.

Looking into it, dovecot sometimes responds with

  • OK Searched 91% of the mailbox, ETA 0:01

which is not handled when expecting a response to a Fetch message.

This is a small change to handle this case. I have made it quite explicit - it only handles an OK status with no code, as I'm not sure whether other messages which have an unknown code (but do contain a code) would be better handled by returning an error as is currently the case.


This change is Reviewable

…ce an error.

Lookig into it, dovecot sometimes responds with

* OK Searched 91% of the mailbox, ETA 0:01

which is not handled when expecting a response to a Fetch message.
This is a small change to handle this case. I have made it quite explicit
- it only handles an OK status with no code, as I'm not sure whether other messages
which have an unknown code (but do contain a code) would be better handled by returning
an error as is currently the case.
@paulpr0
Copy link
Author

paulpr0 commented Feb 22, 2021

Firstly, thank you for the rust-imap crate. I have found it very useful!

If it's any help, the IMAP server I see this message from is Dovecot v2.2.36 with a pretty vanilla setup.

@jonhoo
Copy link
Owner

jonhoo commented Feb 26, 2021

Hi! Thank you for the PR ❤️

This seems like really odd behavior. I don't think the specification allows for these kinds of random untagged Ok responses? @djc have you seen these before? Do you know if clients are expected to just blindly ignore them?

@paulpr0
Copy link
Author

paulpr0 commented Feb 26, 2021

Hi,

I don't know the IMAP specs well, but looking at the definition of the OK response suggests it's allowed (https://tools.ietf.org/html/rfc3501#section-7.1.1)

" The untagged form indicates an information-only message; "

I have had a look over the dovecot source code though as it's probably the most popular IMAP server on Linux (and the one I saw this behaviour on).

I think the function responsible for these untagged OK messages is notify_ok

The call I was seeing was from https://github.com/dovecot/core/blob/master/src/lib-storage/index/index-search.c#L1545, but there are some others too - they all seem to be sent when the server is taking time to complete an operation and is letting the client know in an unstructured way (this one just says "Hang in there.." https://github.com/dovecot/core/blob/master/src/lib-storage/index/maildir/maildir-sync.c#L259)

Hope that helps,

Paul

@jonhoo
Copy link
Owner

jonhoo commented Feb 27, 2021

Ah, that's helpful, thank you! In that case I'd be happy to land this. Unfortunately, it'll have to wait for the release of imap 3.0.0, since it's a backwards-incompatible change to add a variant to UnsolicitedResponse. I really need to get the last bits together to do that. In the meantime, I'll leave some minor nits so that this is ready to land when 3.0 rolls around!

@jonhoo jonhoo added this to the 3.0.0 milestone Feb 27, 2021
} if imap_proto::Status::Ok == status && code.is_none() => {
unsolicited
.send(UnsolicitedResponse::Uncategorised(
information.unwrap_or("").to_string(),
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, I'm surprised that the information is an Option. Judging from the spec it doesn't seem like it can be left out? @djc

status,
code,
information,
} if imap_proto::Status::Ok == status && code.is_none() => {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary for the code to None of this to be considered informational? From what I can tell, it's enough for the response to be untagged for it to be considered unsolicited — or am I missing something?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In which case we should include the code in the unsolicited response type.

@@ -277,6 +277,10 @@ pub enum UnsolicitedResponse {
/// sequence numbers 9, 8, 7, 6, and 5.
// TODO: the spec doesn't seem to say anything about when these may be received as unsolicited?
Expunge(Seq),
/// An uncategorised response - one which has no code associated with it, just a text
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's try to be a little more helpful here to potentially-confused receivers of this code. How about:

/// An unsolicited [`OK` response](https://tools.ietf.org/html/rfc3501#section-7.1.1) that
/// indicates an information-only message; the nature of the information MAY be
/// indicated by a response code.
///
/// The untagged OK is also used as one of three possible greetings
/// at connection startup.  It indicates that the connection is not
/// yet authenticated and that a `LOGIN` command is needed.
Notice(String)

@jonhoo
Copy link
Owner

jonhoo commented Apr 20, 2021

@paulpr0 Now that we're getting closer to a 3.0.0, do you think you might have some spare time to finish this up? Shouldn't be too much left. Otherwise, @mordak, this would be another good candidate to get over the finish line.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants