emersion / go-imap

📥 An IMAP library for clients and servers
MIT License
2.1k stars 296 forks source link

Fetch and misbehaving IMAP server #630

Open agcom opened 4 months ago

agcom commented 4 months ago

I encountered a misbehaving IMAP server in which a response has a bad impossible big number:

tag UID FETCH 63 BINARY.SIZE[1]
* 1 FETCH (UID 63 BINARY.SIZE[1] 18446744073709551232)
tag OK Fetch completed (0.002 + 0.000 + 0.001 secs).

Now assume the following example application that interacts with the misbehaving IMAP server:

package main

import (
    "fmt"
    "github.com/emersion/go-imap/v2"
    "github.com/emersion/go-imap/v2/imapclient"
    "log"
    "os"
)

func main() {
    var client *imapclient.Client

    // Initialize the client; connect, authenticate, and select the mailbox...

    fetchCmd := client.Fetch(imap.UIDSetNum(imap.UID(63)), &imap.FetchOptions{
        BinarySectionSize: []*imap.FetchItemBinarySectionSize{
            {
                Part: []int{1},
            },
        },
    })

    res, err := fetchCmd.Next().Collect()
    if err != nil {
        fmt.Printf("Collect: %s.\n", err)
        os.Exit(1)
    } else {
        fmt.Println("Collect OK.")
    }

    if len(res.BinarySectionSize) == 0 {
        fmt.Println("No binary section size collected.")
    }

    err = fetchCmd.Close()
    if err != nil {
        fmt.Printf("Close fetch: %s.\n", err)
    }

    fmt.Printf("Client connection state: %s.", client.State())
}

The output of running the application is:

Collect OK.
No binary section size collected.
Close fetch: in response-data: imapwire: expected number, got ")".
Client connection state: logout.

You can see that the fetchCmd.Next().Collect() did not return an error, but the returned message buffer does not contain the requested data. Why the Collect() method won't return an error? Is it a pattern to always successfully close the fetch command before trusting the fetched objects?

agcom commented 4 months ago

I also have two other side-queries (asking for opinions):

emersion commented 3 months ago

Why the Collect() method won't return an error?

Sounds like a bug.

Is it possible to propagate the conversion error (regarding the library's design and access to setting the decoder error down the functions' call chain)?

It should be possible via Decoder.returnErr, yeah.

That number anomaly caused by the misbehaving IMAP server blows the whole client up; is it possible to add a logic that tries to recover from these anomalies instead of closing the client?

That's tricky. It's dangerous to try to recover from any error, because we might've dropped important messages and our state might become out-of-sync. We could try to special-case specifically overflow errors.

emersion commented 3 months ago

Related: https://github.com/emersion/go-imap/issues/612