Subject: Re: [imap5] Comments on draft-gulbrandsen-imap-move-01

From: Bron Gondwana <[email-masked]>
Date: Sat, 2 Jun 2012 00:14:09 +0200

On Thu, May 31, 2012 at 10:20:12AM -0400, Cyrus Daboo wrote:
> - §2: what is the problem with sequence MOVE and EXPUNGE? For what
> it is worth Mulberry has always preferred sequence over uids when
> online (it uses UID commands when doing disconnected play back).
> That said, mapping to uids is no big deal (and should not be a big
> deal for any other client that might use sequence in normal
> operation).

Yeah, I don't see a problem either.  I see that my implementation
supports both just fine.  Sequence based moves are handy when you're
fiddling around by hand, if nothing else.

> - §3, ¶4: states that \Deleted must not be set in the target
> mailbox. But what if the messages being moved already had \Deleted
> set prior to the move? Shouldn't we just state that the
> flag/keywords prior to the move must be set on the messages in the
> target mailbox?

That's a really good catch.  Agree with your interpretation
absolutely.  Should be copied with exactly the same flags.

The only potentially sticking point here is awful things like
non-shared \Seen state.  Cyrus (the IMAP server, yay for namespaces)
has per-user seen state.  The actual behaviour at the moment would
be that the owners' \Seen state always gets copied, nobody else's
does.  It might also make sense to copy your OWN \Seen state if you
are not the owner.

But basically it's fluff wording.  The final result shall be as if
COPY + STORE + UID EXPUNGE had happened in precisely that order.
In other words "the messages in the destination mailbox must not
show any state that would not have happened if COPY had been used
instead of MOVE".  The only difference between COPY and MOVE is the
eventual state of the source folder.
 
> - §3: As already mentioned, EXPUNGE responses need to be present.
> Might be worth pointing out that there could be a * EXPUNGE for a
> message not being moved, but which got expunged in another session.
> Actually, more complicated is the case where a message being moved
> was expunged in another session - presumably that message is not
> moved? Yet it would get reported in a * EXPUNGE, but not be part of
> the COPYUID set.

There's no other choice here.  The only rule is that the STORE and
UID EXPUNGE only happen to the messages which were successfully
copied.

Which leads to this interesting little thing.  Two sessions, S1 and S2.

S1: SELECT AFolder
S2: SELECT AFolder
S1: STORE 3 +Flags \Deleted
S2: MOVE 1:5 OtherFolder
S1: EXPUNGE

S1 will see 5 expunges, including the one it was expecting.  S2 will
see the 5 expunged it was expecting as well - but will store the message
as \Deleted in OtherFolder.

I wonder indeed if that was an intention in the spec, that \Deleted gets
explicitly CLEARED by the MOVE operation.  Which is kind of surprising
actually.  Hmm, I wonder.

Yep - COPY copies messages with \Deleted and they have \Deleted in the
destination folder as well.

OK - I take it back, this isn't surprising.  This is the natural way
it should be.  Yes, S1's expunge missed the mark - but that's the
unfortunately downside of two-stage EXPUNGE.  There is literally no
way to avoid this... whenever you do a COPY or MOVE you need to check
your destination messages to make sure you didn't pick up something
half way through being deleted.  Even S1 doing UID EXPUNGE wouldn't
avoid this.

BUT - even if there are other \Deleted messages in the folder, the
"EXPUNGE" part of MOVE never deletes them.  It's a "UID EXPUNGE" on
just the UIDs which were actually affected by this message.  Which
also gives it the dubious honour of including "ERASE".

"ERASE" of course being just the "STORE" and "UID EXPUNGE" parts of the
above command combined together atomically(ish) such that you don't
get the race condition listed above.  In fact it would be atomic in
Cyrus.  I think I'm going to implement this one now in fact, we would
use it immediately.  Er, I'm going to write a separate email for it too.

> - §4: whilst no one may implement it, there still ought to be a
> reference to the interaction with ANNOTATE. ANNOTATE §4.6 has
> detailed text on the interaction with COPY and something similar is
> needed for MOVE.

"As Above" ;)  Literally, it should be enough to say "ANNOTATions
are treated exactly as specified for COPY and for EXPUNGE".

---

From: Timo Sirainen <[email-masked]>
Date: Sat, 2 Jun 2012 01:23:30 +0300

> does.  It might also make sense to copy your OWN \Seen state if you

The owner's \Seen state gets copied with COPY to your private mailbox? I think that might be considered a minor security/privacy leak, since it allows other users to see when the owner has read the message.

---

From: Bron Gondwana <[email-masked]>
Date: Sat, 2 Jun 2012 06:05:41 +0200

> The owner's \Seen state gets copied with COPY to your private mailbox? I think that might be considered a minor security/privacy leak, since it allows other users to see when the owner has read the…

Oh, it gets stripped if you can't see it - same as everything else.

Bron.

---

From: Arnt Gulbrandsen <[email-masked]>
Date: Sat, 2 Jun 2012 17:07:12 +0200

Lovely review, thanks.

On 05/31/2012 04:20 PM, Cyrus Daboo wrote:
> - §2: what is the problem with sequence MOVE and EXPUNGE?

I wouldn't mind having MSN-based move. But if you look at the yearly 
move proposal disasters, MSNs were often mention just before the whole 
thing exploded.

So I thought, since I couldn't find any clients that both expose move in 
the UI and use COPY rather than UID COPY, why not skip the MSN variant 
in the -00 draft?

If you as client author would prefer MOVE to UID MOVE, and none of the 
server authors have a problem with implementing it, then I don't mind 
adding it to the draft.

> - §2: "atomically" - I would prefer not to use the term "atomic" given
> that we are allowing partial failures (i.e., some messages might not be
> moved, whilst others are). So in the first paragraph I suggest changing
> "atomically" to "using a single command".

The word atomic seems to be a trouble magnet. I've removed the word 
completely from the draft, and instead merely describe what's desired 
(each single message is either moved or left unchanged).

> - §3: This needs a 3501-like "formal" definition with Arguments:,
> Responses:, Results: etc. It should use the same text for describing
> preservation of flags (see note below) and internal date.

I disgree, but if you want I'll do it. For now I've only added an open 
issue to do it (time is short today and I'm still only halfway through 
your review).

> We need to
> decide whether the \Recent flag is set like COPY (probably should be).

I'm strongly in favour. Everything should be like copystoreexpunge, 
unless there is a specific and good reason to differ.

> Also, the [TRYCREATE] behavior from COPY should be re-used. Should
> probably note that this command is only valid in selected state (or
> include a comment to that effect in the formal syntax which 3501 does
> for the command-select syntax element).

It is reused already; MOVE is defined in terms of, etc. But I'll add a 
sentence like "which means that e.g. TRYCREATE blah".

> - §3, ¶2: "UID EXPUNGE" - need a reference to the UIDPLUS extension.

Added already. If I didn't misunderstand, Barry asked me to not post any 
update until the WG has been chartered.

> - §3, ¶4: states that \Deleted must not be set in the target mailbox.
> But what if the messages being moved already had \Deleted set prior to
> the move? Shouldn't we just state that the flag/keywords prior to the
> move must be set on the messages in the target mailbox?

I've had requests that it work both ways. I have no strong opinions myself.

> - §3: As already mentioned, EXPUNGE responses need to be present. Might
> be worth pointing out that there could be a * EXPUNGE for a message not
> being moved, but which got expunged in another session. Actually, more
> complicated is the case where a message being moved was expunged in
> another session - presumably that message is not moved? Yet it would get
> reported in a * EXPUNGE, but not be part of the COPYUID set.

Fixed.

> - §3: Might want to make it clear that * FETCH FLAGS is NOT sent if
> moved messages have \Deleted added as part of the "internal" server
> implementation of move.

I don't think that was at all clear, or even intended ;) But I think 
you're right, and I added it.

> - §3: Example: change "@S:" to "S:".

Fixed.

> - §3: COPYUID response in the argument is wrong - it should have three
> values: destination mailbox UIDVALIDITY, set of uids from source
> mailbox, set of uids in destination mailbox. Current example only shows
> one set of uids.

Fixed.

> reference to the interaction with ANNOTATE. ANNOTATE §4.6 has detailed
> text on the interaction with COPY and something similar is needed for MOVE.

A few paragraphs up I added a sentence stating specifically that 
extensions that extend COPY, extend MOVE in the same way. Will that do?

> - §6: "command" is not the right syntax element to extend. I would
> suggest defining a uidmove element and then use that to extend the 3501
> uid element:
>
>     uidmove =  "UID MOVE" SP set SP mailbox
>     uid     =/ uidmove

I've changed since I posted the last update. I'll leave it as it is for 
now. Who knows whether we'll have an MSN-based move.

Barry: I understood you as saying that I should hold of on posting 
updates until the WG has been formed. Was that right, or can I post an 
update? Cyrus: If I post an update soon, it'll likely contain TBD for 
some of your issues.

Arnt

---

From: Barry Leiba <[email-masked]>
Date: Sat, 2 Jun 2012 11:46:14 -0400

> Barry: I understood you as saying that I should hold of on posting updates
> until the WG has been formed. Was that right, or can I post an update?
> Cyrus: If I post an update soon, it'll likely contain TBD for some of your
> issues.

We're probably talking about a three-week wait before the working
group is chartered, barring unforeseen objections.  I somewhat prefer
that you hold updates until then, and then post
draft-ietf-imapmove-move-00, but it's not a terribly strong
preference.  If you think continued review and discussion would
benefit from posting draft-gulbrandsen-imap-move-02 before then, then
do so.

I always say that versions are cheap.  We shouldn't hold up progress
because we're hesitant to post a revision.

Barry