search

emersion / go-imap

📥 An IMAP library for clients and servers
MIT License
2.02k stars 288 forks source link

[v2] Fallback MOVE by COPY+STORE+EXPUNGE doesn't return DestUIDs #623

Open Vovan-VE opened 1 week ago

Vovan-VE commented 1 week ago

v2.0.0-beta.3

data, err := client.Move(uidset, to).Wait()

The resulting MoveData.DestUIDs gets fulfilled only when real MOVE executed. https://github.com/emersion/go-imap/blob/ee36cf4658d87d2230b6bde0b0c2177b6b4eff96/imapclient/client.go#L848-L852

In case of fallback workflow COPY+STORE+EXPUNGE it remains nil.

Vovan-VE commented 1 week ago

Or wait, it's impossible by RFCs spec?

emersion commented 1 week ago

Does #624 help?

Note, these fields are only populated if the server supports UIDPLUS.

Vovan-VE commented 1 week ago

I'll try it and report later. Thanks!

Vovan-VE commented 1 week ago

I just figured out that COPY+STORE+EXPUNGE should already work fine without the PR, if underlying IMAP server supports needed features. Sorry, my fail. I'll check more details better next time. And so DestUIDs will remein empty only when it's really nothing to put there.

However the PR itself should fix independent COPY command, isn't it?

emersion commented 1 week ago

The patch is supposed to fix the case where the server does not support MOVE, but does support UIDPLUS. There is nothing we can populate the fields with when the server does not support UIDPLUS.

Vovan-VE commented 1 week ago

Yes, right. I mean your current version without patch already should work fine as far as possible, even when it's MOVE under the hood, because Move().Wait() returns MoveData anyway. However the patch is usefull to fix independent COPY command, when CopyData is returned.

emersion commented 1 week ago

There's one detail though: this function handles untagged responses only. Tagged responses are handled elsewhere: https://github.com/emersion/go-imap/blob/ee36cf4658d87d2230b6bde0b0c2177b6b4eff96/imapclient/client.go#L711

The RFC specifies that COPY results in a tagged COPYUID response, and MOVE results in an untagged COPYUID response.

Vovan-VE commented 5 days ago

I gathered CAPABILITIES from underlying accounts, and unfortunely I have no "test subjects" without MOVE support at the moment, so I cannot actually check if the PR does anything.