emersion / go-imap

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

CAPABILITY Interferes with LOGIN Command #651

Open qjebbs opened 3 days ago

qjebbs commented 3 days ago

Backgroud:

  1. When the imapclient starts, it sends a CAPABILITY command to the server if the server greets it without CAPABILITY. The command is executed in another goroutine.
  2. Some server (imaphz.qiye.163.com here) seems to ignore new commands when the current command is still being processed.

UPDATE:

imaphz.qiye.163.com is not the one to blame, the RFC 3501 specification says:

Therefore, 
if the client sends any command other than FETCH, STORE, or SEARCH, it
MUST wait for the completion result response before sending a command
with message sequence numbers.

Consider the following code:

// conn is a net.Conn to imaphz.qiye.163.com
c = imapclient.New(conn, &imapclient.Options{
    DebugWriter: os.Stdout,
})
c.WaitGreeting()
err := c.Login(userName, password).Wait()
if  err != nil {
    return nil, fmt.Errorf("login %q: %w", userName, err)
}

The LOGIN and background spawned CAPABILITY are executed in different goroutines. The order of the commands is not guaranteed. So I see a random failure when initializing the imapclient.

  1. If the CAPABILITY command is executed first, and the LOGIN command is executed before the CAPABILITY command finishes, the server seems to ignore the LOGIN command. The command receives nothing and reports an unexpected EOF error after 60 seconds.

    * OK IMAP4 ready
    T1 CAPABILITY
    T2 LOGIN "xxx@xxx.com" "xxxx"
    * CAPABILITY IMAP4rev1 XAPPLEPUSHSERVICE XLIST SPECIAL-USE AUTH=PLAIN ID
    T1 OK completed
    --- FAIL: TestIMAPLogin (60.13s)
        imap_test.go:57: login "xxx@xxx.com": unexpected EOF
    FAIL
  2. If the LOGIN command happens to be executed first, the server will respond. (CAPABILITY command is ignored until timeout. Though the client initialized with success)

    * OK IMAP4 ready
    T1 LOGIN "xxx@xxx.com" "xxxx"
    T2 CAPABILITY
    T1 NO ERR.LOGIN.PASSERR
    T3 LOGOUT
    --- FAIL: TestIMAPLogin (0.20s)
        imap_test.go:57: login "xxx@xxx.com": imap: NO ERR.LOGIN.PASSERR

Though It's the server's fault, I can't change the server's behavior. Is there a way to fix this issue? Maybe to lock up imapclient before a background command finishes (not accept new ones)?

Workaround

UPDATE: In theory, there is no workaround to this problem

qjebbs commented 3 days ago

Realized that it not the client library's responsibility for this issue, and there exits a workaround for this library. Closing...

qjebbs commented 1 day ago

It turns out imaphz.qiye.163.com is the one that complies with the RFC 3501 specification

Therefore, 
if the client sends any command other than FETCH, STORE, or SEARCH, it
MUST wait for the completion result response before sending a command
with message sequence numbers.

We need a global lock to wait for the async, background-spawned CAPABILITY done.

emersion commented 1 day ago

This paragraph only applies to the "selected" state. If the connection isn't in the "selected" state, an untagged EXPUNGE response cannot be sent by the server.

qjebbs commented 23 hours ago

This paragraph only applies to the "selected" state. If the connection isn't in the "selected" state, an untagged EXPUNGE response cannot be sent by the server.

Sorry, I didn't read it carefully.

What's your suggestion for the situation when we work with the server like this topic? If global lock is not an option, I would say, make the background spawned CAPABILITY command synchronized and blocked?

emersion commented 21 hours ago

Does it help to call c.Caps() before c.Login()?

qjebbs commented 7 hours ago

Does it help to call c.Caps() before c.Login()?

I am afraid it doesn't.

c.Caps() is similar to what I so called workaround, by spawning another capability command and waiting. It solves half of the problem, making sure login command not being ignored.

But if it happens this capability command is ignored, the login will be delayed for 60 seconds respReadTimeout. Making background capability command synchronized and blocking will solve all the problem.