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

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

Open
Vovan-VE opened this issue Jun 24, 2024 · 8 comments · May be fixed by #624
Open

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

Vovan-VE opened this issue Jun 24, 2024 · 8 comments · May be fixed by #624

Comments

@Vovan-VE
Copy link

v2.0.0-beta.3

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

The resulting MoveData.DestUIDs gets fulfilled only when real MOVE executed.

if cmd := findPendingCmdByType[*MoveCommand](c); cmd != nil {
cmd.data.UIDValidity = uidValidity
cmd.data.SourceUIDs = srcUIDs
cmd.data.DestUIDs = dstUIDs
}

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

@Vovan-VE
Copy link
Author

Or wait, it's impossible by RFCs spec?

emersion added a commit that referenced this issue Jun 24, 2024
In Client.Move, when falling back to COPY + STORE + EXPUNGE, we
might receive a tagged COPYUID response code. Populate MoveCommand
in that case.

Closes: #623
@emersion
Copy link
Owner

Does #624 help?

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

@Vovan-VE
Copy link
Author

I'll try it and report later. Thanks!

@Vovan-VE
Copy link
Author

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

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

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

There's one detail though: this function handles untagged responses only. Tagged responses are handled elsewhere:

case "COPYUID":

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

@Vovan-VE
Copy link
Author

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.

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

Successfully merging a pull request may close this issue.

2 participants