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

Bad tests in COPY and MOVE commands #37

Open
shikharid opened this issue May 10, 2024 · 4 comments
Open

Bad tests in COPY and MOVE commands #37

shikharid opened this issue May 10, 2024 · 4 comments

Comments

@shikharid
Copy link
Contributor

shikharid commented May 10, 2024

These following tests have expectations nowhere defined in RFCs:

Test copy command 17/24 (line 32)

  • This tests expect that UIDs assigned in the destination mailbox will be exactly in-order of msg UIDs copied from source mailbox
  • For example: If msgs with UIDs 1,2,4 are moved from A->B, equivalent UIDs in B will be 1->1, 2->2, 4->3
  • RFC3501 nowhere asks servers to behave this way and server can choose to copy these msgs in any arbitrary order
  • A mapping of 1->2, 2->1. 3->2 is also valid
  • This test can either be disabled as broken
  • Or simplified by Just copying 1 msg at a time and doing the assertions instead of copying 3 at once

Test move command 12/13 (line 44)

  • This test moves a msg in the same mailbox
  • The test expects that a new UID will be assigned to the message
  • Although the RFC 6851 does allow for moving msgs in the same mailbox
  • But it does not specify that the server cannot optimise and consider it a NOOP
  • So the assertion here is not reasonable
  • The test should probably just look for a successful completion of the MOVE command

Again, happy to raise a PR fixing these. Let me know.

@sirainen
Copy link
Contributor

COPY: I guess it's not prevented by RFCs, but I've a feeling it's not going to be nice client/user behavior. I wouldn't be surprised if it breaks some clients.

MOVE: Here I think even the RFC disagrees. It says MOVE is the same behavior as:

  1. [UID] COPY
  2. [UID] STORE +FLAGS.SILENT \DELETED
  3. UID EXPUNGE

So the 1. step already tells it to assign a new UID, because two message instances can't have the same UID.

@shikharid
Copy link
Contributor Author

shikharid commented May 10, 2024

For 2, you are right I guess.

For 1, I can't comprehend why.
The simplest implementation for a client is:

  • COPYUID <src-uids> <dest-uids>
  • Each element in src uid msg set maps with corresponding entry in dest uid set

Any other way will be very convoluted, in fact I can't see how a client may do it any other way.

@sirainen
Copy link
Contributor

For 1 I'm mainly thinking that if there are no other IMAP servers copying out-of-order, the existing client implementationa may never have tested whether it works. Anyway, I guess I could do some change in that test, but first I'd want to ask from imap-protocol mailing list whether someone else can think of a reason why the order would actually have to be preserved. IMAP RFC is sometimes hiding information in a bit random places.

@shikharid
Copy link
Contributor Author

shikharid commented May 10, 2024

Makes sense, thanks!
And I have an IMAP server copying out of order, hence the ask : )

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

No branches or pull requests

2 participants