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

Fetch and misbehaving IMAP server #630

Open
agcom opened this issue Jul 24, 2024 · 3 comments
Open

Fetch and misbehaving IMAP server #630

agcom opened this issue Jul 24, 2024 · 3 comments

Comments

@agcom
Copy link
Contributor

agcom commented Jul 24, 2024

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
Copy link
Contributor Author

agcom commented Jul 24, 2024

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

  • I traced the application and indeed it encountered an integer overflow (error by strconv.ParseUint), but the returned error is kinda irrelevant (expected number, got ")"). 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)?
  • 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? Of course handling all anomalies would require hundreds lines of codes, but, the least is to end the misbehaved command and move onto next ones.

@emersion
Copy link
Owner

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
Copy link
Owner

Related: #612

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

No branches or pull requests

2 participants