jonhoo / rust-imap

IMAP client library for Rust
Apache License 2.0
477 stars 80 forks source link

Support BYE response status #210

Closed piotrwalkusz1 closed 3 years ago

piotrwalkusz1 commented 3 years ago

This example code:

fn main() {
    print!("{}", fetch_inbox_top().unwrap().unwrap());
}

fn fetch_inbox_top() -> imap::error::Result<Option<String>> {
    let domain = "imap.wp.pl";
    let tls = native_tls::TlsConnector::builder().build().unwrap();

    // we pass in the domain twice to check that the server's TLS
    // certificate is valid for the domain we're connecting to.
    let client = imap::connect((domain, 993), domain, &tls).unwrap();

    // the client we have here is unauthenticated.
    // to do anything useful with the e-mails, we need to log in
    let mut imap_session = client
        .login("<USERNAME>", "<PASSWORD>")
        .map_err(|e| e.0)?;

    // we want to fetch the first email in the INBOX mailbox
    imap_session.select("INBOX")?;

    // fetch message number 1 in this mailbox, along with its RFC822 field.
    // RFC 822 dictates the format of the body of e-mails
    let messages = imap_session.fetch("1", "RFC822")?;
    let message = if let Some(m) = messages.iter().next() {
        m
    } else {
        return Ok(None);
    };

    // extract the message's body
    let body = message.body().expect("message did not have a body!");
    let body = std::str::from_utf8(body)
        .expect("message was not valid utf-8")
        .to_string();

    // be nice to the server and log out
    imap_session.logout()?;

    Ok(Some(body))
}

throws exception on the statement imap_session.logout()?;:

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Parse(Invalid([97, 52, 32, 66, 89, 69, 32, 73, 77, 65, 80, 52, 114, 101, 118, 49, 32, 83, 101, 114, 118, 101, 114, 32, 108, 111, 103, 103, 105, 110, 103, 32, 111, 117, 116, 13, 10]))'

After decoding content of Parse(Invalid([...])) as ASCII:

a4 BYE IMAP4rev1 Server logging out

It looks like the BYE response status is not supported.

jonhoo commented 3 years ago

That certainly seems unintended! Does this happen with the latest alpha? My guess here is that the logout command generates a BYE response, which is technically an error response, but in the specific case of logout it isn't. Can you verify that that is indeed where the error occurs by changing .logout()? to .logout().unwrap(), and then seeing that the panic points to that line? Perhaps we should special-case handling BYE in logout @mordak?

mordak commented 3 years ago

Hm. I cannot reproduce on master, but it could also just be a difference between servers. I am working with Dovecot.

@piotrwalkusz1 : Can you enable debugging on the session? It's just adding imap_session.debug = true; before the imap_session.logout()?; line in your example there. When I do this, I get:

C: a4 LOGOUT
S: * BYE Logging out
S: a4 OK Logout completed (0.001 + 0.000 secs).

And this doesn't throw an error for me - it returns normally. But a different server could give something different.

@jonhoo We could certainly add a check for BYE in logout() specifically. That should be easy.

piotrwalkusz1 commented 3 years ago

Today I confirmed that the error occurs on version "3.0.0-alpha.4" on line imap_session.logout(). I run this code:

fn main() {
    print!("{}", fetch_inbox_top().unwrap().unwrap());
}

fn fetch_inbox_top() -> imap::error::Result<Option<String>> {

    let client = imap::ClientBuilder::new("imap.wp.pl", 993).native_tls()?;

    // the client we have here is unauthenticated.
    // to do anything useful with the e-mails, we need to log in
    let mut imap_session = client
        .login("<USERNAME>", "<PASSWORD>")
        .map_err(|e| e.0)?;

    // we want to fetch the first email in the INBOX mailbox
    imap_session.select("INBOX")?;

    // fetch message number 1 in this mailbox, along with its RFC822 field.
    // RFC 822 dictates the format of the body of e-mails
    let messages = imap_session.fetch("1", "RFC822")?;
    let message = if let Some(m) = messages.iter().next() {
        m
    } else {
        return Ok(None);
    };

    // extract the message's body
    let body = message.body().expect("message did not have a body!");
    let body = std::str::from_utf8(body)
        .expect("message was not valid utf-8")
        .to_string();

    // be nice to the server and log out
    imap_session.debug = true;
    imap_session.logout().unwrap();

    Ok(Some(body))
}

and get:

C: a4 LOGOUT
S: a4 BYE IMAP4rev1 Server logging out
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Parse(Invalid([97, 52, 32, 66, 89, 69, 32, 73, 77, 65, 80, 52, 114, 101, 118, 49, 32, 83, 101, 114, 118, 101, 114, 32, 108, 111, 103, 103, 105, 110, 103, 32, 111, 117, 116, 13, 10]))', src/main.rs:35:27
stack backtrace:
   0: rust_begin_unwind
             at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/panicking.rs:493:5
   1: core::panicking::panic_fmt
             at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/core/src/panicking.rs:92:14
   2: core::result::unwrap_failed
             at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/core/src/result.rs:1355:5
   3: core::result::Result<T,E>::unwrap
             at /home/piotr/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/result.rs:1037:23
   4: imap_test::fetch_inbox_top
             at ./src/main.rs:35:5
   5: imap_test::main
             at ./src/main.rs:2:18
   6: core::ops::function::FnOnce::call_once
             at /home/piotr/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

Line 35 is:

imap_session.logout().unwrap();
jonhoo commented 3 years ago

Ah, the difference seems to be that in your case @mordak, the response is untagged:

S: * BYE Logging out

Whereas in @piotrwalkusz1's case, it's tagged:

S: a4 BYE IMAP4rev1 Server logging out
mordak commented 3 years ago

Hmm. That behaviour is explicitly against the RFC, which says the BYE MUST be untagged.

But special casing this is kind of easy, so I don't see why we can't do it.

@piotrwalkusz1 Do you know what server this is? We can comment it so future us remembers why we have the special case.

piotrwalkusz1 commented 3 years ago

https://www.wp.pl/ is a popular Polish email server. Unfortunately, I don't know what software they use.