Subject: [Gen-art] Gen-art review of draft-ietf-imapmove-command-02

From: "Moriarty, Kathleen" <[email-masked]>
Date: Tue, 20 Nov 2012 15:01:28 -0500

I am the assigned Gen-ART reviewer for this draft. For background on
Gen-ART, please see the FAQ at

<http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.

Please resolve these comments along with any other Last Call comments
you may receive.

Document:  draft-ietf-imapmove-command-02
Reviewer: Kathleen Moriarty
Review Date:
IETF LC End Date:
IESG Telechat date: (if known)

Summary:  The document needs a little more work before it is ready and should address what happens when the series of commands fails to execute (logging, revert, etc.).  This should just result in either an incomplete copy (so maybe a garbled message or extra messages being left behind (expunge operations did not occur).  This document does not cover that case and does not point to another standard that covers that question.

Major issues:  

Minor issues:  The security consideration section and the main body of the document do not explain what happens when the series of commands fails to execute or any associated risks.  It should not result in mail being deleted, but rather being left in too many places or possible errors if the copy operation did not complete (unless that is handled and just needs a refernce)?

Nits/editorial comments:
Section 3.3 third paragraph:  NOT and neither form a double negative, the phrase should use either instead of neither:
The server MUST NOT leave a message in either mailbox

In the last sentence of this same paragraph, is there a reference to what happens in the very last clause if the command fails?
   "Note that these restrictions only apply to
   individual messages and not to the command as a whole, which can fail
   partway through."

Section 3.3 paragraph 5, can you add more context or clean up the paragraph to make it easier to follow:
"Note that the server may send EXPUNGEs for other messages as well, if
   any happen to have been expunged at the same time; this is normal
   IMAP operation."

Section 3.3, paragraph 6: remove duplicate "is allowed":
Change from:
"Note that moving a message to the currently selected mailbox (that
   is, where the source and target mailboxes are the same) is allowed
   when copying the message to the currently selected mailbox is
   allowed."
TO:
"Note that moving a message to the currently selected mailbox (that
   is, where the source and target mailboxes are the same) is allowed
   when copying the message to the currently selected mailbox."

Section 4.3, last sentence:
Replace 'that' with whatever it is referring to as it is not clear to the reader.
"It can be unnecessarily difficult to process that usefully.)"

Section 6, Security Considerations, second paragraph, last sentence (change with to when):
Change from: 
 "Scanning with updated rules may
   also be required when messages are moved even with the underlying
   policy has not changed.
To:
   "Scanning with updated rules may
   also be required when messages are moved even when the underlying
   policy has not changed."

Thanks,
Kathleen

---

From: Barry Leiba <[email-masked]>
Date: Tue, 20 Nov 2012 16:26:29 -0500

Hi, Kathleen, and thanks for the review.  You left the document
shepherd and one of the editors off of the distribution list, so I'm
adding them in here.  Because of that, I'm not doing the trimming that
I'd normally do in my inline reply.

On Tue, Nov 20, 2012 at 3:01 PM, Moriarty, Kathleen
<[email-masked]> wrote:
> Summary:  The document needs a little more work before it is ready and should
> address what happens when the series of commands fails to execute (logging,
> revert, etc.).  This should just result in either an incomplete copy (so maybe a
> garbled message or extra messages being left behind (expunge operations did
> not occur).  This document does not cover that case and does not point to another
> standard that covers that question.

This appears to be a repetition of the "minor issues" below, so I'm
responding to it there.

> Minor issues:  The security consideration section and the main body of the
> document do not explain what happens when the series of commands fails to
> execute or any associated risks.  It should not result in mail being deleted, but
> rather being left in too many places or possible errors if the copy operation did
> not complete (unless that is handled and just needs a refernce)?

What "series of commands" are you referring to?  The point here is
that there's *one* command, MOVE (or UID MOVE), which makes atomic a
function that is otherwise done with three separate commands (that
cause the problems you're noting, which is why we're creating a single
command to do it).  Is that really not explained sufficiently?  I
should think that the Introduction makes this clear.

As to what state the single MOVE command leaves things in, this
paragraph from Section 3 answers the questions you appear to have:

   Each message SHOULD either be moved or unaffected.  The server MUST
   NOT leave a message in neither mailbox, and SHOULD NOT leave a
   message in both mailboxes afterwards, even if the server returns a
   tagged NO response.  Note that these restrictions only apply to
   individual messages and not to the command as a whole, which can fail
   partway through.

See below for more discussion of that.

> The server MUST NOT leave a message in either mailbox

Absolutely not!  The double negative is quite necessary and
intentional.  One possible outcome if the MOVE of a single message
should fail internally would be that the message would be in *neither*
mailbox... perhaps the server removed it from the source mailbox
before putting it into the target, and there was some internal error,
server crash, or power failure at that point.  This is saying that
such a thing MUST NOT happen.

>    individual messages and not to the command as a whole, which can fail

I don't understand what you're asking here either.  The MOVE command
can operate on a set of messages in one command.  The paragraph is
saying that it has to operate successfully (that is, as described in
the earlier part of the paragraph) on each message, but that the
command can fail in the middle, having moved some of the messages but
not others.  It doesn't have to roll back successful moves if it can't
finish the rest.  Maybe this would be clearer to you?:

   These restrictions apply to each message in the set to be moved.
   If the command fails partway through, some messages might be moved,
   while others will not have been.

>    any happen to have been expunged at the same time; this is normal

No further context is needed for anyone familiar with IMAP.  The IMAP
protocol allows the server to send EXPUNGE responses for any message
at any time, and this is just a reminder of that.  It could be
removed, and it would change nothing.  But it would be a bad idea to
repeat what's explained at length in RFC 3501.

>    when copying the message to the currently selected mailbox."

Yes, that probably got left there as the sentence was rephrased.  Thanks.

> "It can be unnecessarily difficult to process that usefully.)"

Would "that sequence" make it clearer for you?

>    also be required when messages are moved even when the underlying

Yep; thanks.

Barry

---

From: "Moriarty, Kathleen" <[email-masked]>
Date: Tue, 20 Nov 2012 16:48:36 -0500

Hi Barry,

Sure and sorry to have left off a few people, it was unintentional!

I'll explain where I was confused and think I am looking for a lot less than how you may have interpreted my comments, so this should be fairly easy to resolve.  I added my new comments with [KMM] at the start of each line.

Thanks,
Kathleen
________________________________________
From: [email-masked] [[email-masked]] On Behalf Of Barry Leiba [[email-masked]]
Sent: Tuesday, November 20, 2012 4:26 PM
To: Moriarty, Kathleen
Cc: [email-masked]; [email-masked]; [email-masked]; [email-masked]; Alexey Melnikov
Subject: Re: Gen-art review of draft-ietf-imapmove-command-02

[KMM]:  It was the last part of the last sentence that raised the confusion, "which can fail part way through."  What happens if it fails partway through?  If this is answered in another RFC, can there be a pointer added to the RFC and section?  Or do you clarify this to say that a failure partway through is not acceptable?  I was reading this last sentence as the MOVE command as a whole contained several operations and one of those operations could fail.

[KMM]:  I think the correct grammar is to say either instead of neither and it has the same meaning that you intended.  The message would not be in either mailbox.

[KMM]: Okay, now I am a bit more confused.  Can the set of operations fail?  Earlier in your response, you say it can't fail.  Then here, it says one of the operations can fail.  I am fine with either, I just want to make sure it is clear to the reader.  Is there any logging required when it fails?  If this is specified in another IMAP document, maybe a quick reference would be helpful?

[KMM]:  By context, I only meant on the wording, not references to other RFCs.  Reading through this sentence, it would be helpful to know that you are talking about unrelated expunge operations, at least I think that is what was intended and think it would be helpful to clarify that point with a few extra words.

[KMM]:  Yes, "the sequence" works once you add the word sequence.  At the start of the parens, replacing 'it' would be helpful as well.

Yep; thanks.

Barry

---

From: Barry Leiba <[email-masked]>
Date: Tue, 20 Nov 2012 17:40:42 -0500

> [KMM]:  It was the last part of the last sentence that raised the confusion,
> "which can fail part way through."  What happens if it fails partway through?
> If this is answered in another RFC, can there be a pointer added to the RFC
> and section?  Or do you clarify this to say that a failure partway through is
> not acceptable?  I was reading this last sentence as the MOVE command
> as a whole contained several operations and one of those operations could fail.

Got it.  Then this is really overlapping with this discussion, moved
up from below:

> The MOVE command
> [KMM]: Okay, now I am a bit more confused.  Can the set of operations fail?
> Earlier in your response, you say it can't fail.  Then here, it says one of the
> operations can fail.  I am fine with either, I just want to make sure it is clear
> to the reader.  Is there any logging required when it fails?  If this is specified
> in another IMAP document, maybe a quick reference would be helpful?

I don't think we ever refer to "operations", so I'm still not sure
what you're confused about, but I am sure that you're confused.  So
let's try to sort that out.

The MOVE command can move a set of messages.  If the client sends
"MOVE 1:50 Banana", that's asking the server to move the first 50
messages from the currently selected mailbox into the mailbox called
"Banana".  Let's suppose that the currently selected mailbox is called
"Apple".

The paragraph in question, so we have the context here, says this:

What that paragraph is *trying* to tell you is this:

The MOVE command has to leave each message in a reasonable state.
That means that for each message in the message set ("1:50"), either
the message should be moved, or the message should be unaffected
(sentence 1).  For each message in the message set, the server MUST
NOT make it disappear from both Apple and Banana, and SHOULD NOT leave
it in *both* Apple and Banana (sentence 2).  This stuff applies to
each message individually... it's allowable for the server to fail to
move all 50 messages, as long as the previous two sentences apply.

Do you (think you) understand *that*?

I'll propose a reformulation of that paragraph in a moment, but first...

>> Section 3.3 third paragraph:  NOT and neither form a double negative, the
>> phrase should use either instead of neither:
> [KMM]:  I think the correct grammar is to say either instead of neither
> and it has the same meaning that you intended.  The message would
> not be in either mailbox.

I think you're wrong.  "MUST NOT be in either mailbox" would mean that
it's not in Apple and it's also not in Banana.  Which is what you say
in your last comment, but it's not what we're trying to say.  There's
nothing grammatically or semantically wrong with the sentence as
written, and it says exactly what it's meant to say: that you MUST NOT
have the message be not in Apple and also not in Banana -- you have to
make sure it remains in one or the other.

Notwithstanding that it's correct, if a native English speaker can't
understand it, then it needs to be re-written (though we thought it
was nice and elegant as it is; sigh -- elegance has to give way to
understanding).

If I re-write the whole paragraph like this, is the whole thing clearer to you?:

   Because a MOVE applies to a set of messages, it might fail partway
   through the set.  Regardless of whether the command is successful in
   moving the entire set, each individual message SHOULD either be moved
   or unaffected.  The server MUST leave each message in a state where
   it is in at least one of the source or target mailboxes (no message can
   be lost or orphaned).  The server SHOULD NOT leave any message in
   both mailboxes (it would be bad for a partial failure to result in a bunch
   of duplicate messages).  This is true even if the server returns a tagged
   NO response to the command.

> [KMM]:  By context, I only meant on the wording, not references to other
> RFCs.  Reading through this sentence, it would be helpful to know that you
> are talking about unrelated expunge operations, at least I think that is what
> was intended and think it would be helpful to clarify that point with a few
> extra words.

Hm.  I'm still thinking that this is entirely unnecessary, because,
really, anyone familiar enough with IMAP to consider implementing this
will know exactly what this means.  I'll propose something, and we'll
see if the editors think it's useful to change:

OLD
   Note that the server may send EXPUNGEs for other messages as well, if
   any happen to have been expunged at the same time; this is normal
   IMAP operation.
NEW
   It is always the case in normal IMAP operation that the server might
   also send EXPUNGE responses that are not related to the MOVE, such
   as for other messages that happen to have been expunged at the same
   time.

> [KMM]:  Yes, "the sequence" works once you add the word sequence.  At the
> start of the parens, replacing 'it' would be helpful as well.

NEW
   (Sending COPYUID in the tagged OK, as described in the UIDPLUS
   specification, means that clients first receive an EXPUNGE for a
   message and afterwards COPYUID for the same message.  It can be
   unnecessarily difficult to process that sequence usefully.)

Barry

---

From: "Moriarty, Kathleen" <[email-masked]>
Date: Tue, 20 Nov 2012 18:24:18 -0500

Thank you, Barry!

I appreciate your patience on this and suggestions.  I am in the middle of some developers working on RFCs in MILE and the comments are leading me to be very specific... and I think they are obvious too.

I like all of your suggestions below.  For the second to last one, another alternate option closer to the original text for the editors to consider would be:
OLD
   Note that the server may send EXPUNGEs for other messages as well, if
   any happen to have been expunged at the same time; this is normal
   IMAP operation.
NEW
   Note that the server may send unrelated EXPUNGE messages as well, if
   any happen to have been expunged at the same time; this is normal
   IMAP operation.

Thanks,
Kathleen

BTW, I can configure a sendmail.cf by hand, but have never programmed IMAP server code.  :-)
________________________________________
From: [email-masked] [[email-masked]] On Behalf Of Barry Leiba [[email-masked]]
Sent: Tuesday, November 20, 2012 5:40 PM
To: Moriarty, Kathleen
Cc: [email-masked]; [email-masked]; [email-masked]; [email-masked]; Alexey Melnikov
Subject: Re: Gen-art review of draft-ietf-imapmove-command-02

OLD

NEW

Barry

---

From: Ned Freed <[email-masked]>
Date: Wed, 21 Nov 2012 09:42:55 -0800 (PST)

> I'd normally do in my inline reply.

Thanks.

> ...

> Yes, that probably got left there as the sentence was rephrased.  Thanks.

Removed.

> Yep; thanks.

Took me a while to realize that the reason I couldn't find the change is that
the change had already been made in the document.

Anyway: Fixed.

				Ned

---

From: Ned Freed <[email-masked]>
Date: Wed, 21 Nov 2012 10:08:30 -0800 (PST)

> > not be in either mailbox.

I'm going to have to push back just a bit here: Double negatives are *not* ipso
facto incorrect grammar. In fact the only case where they should be considered
bad grammar is when they are used incorrectly. The standard example of this is,
"I didn't see noone.", which actualy means, "I saw someone", and that's pretty
certain not to be the speaker's intent.

Far better authors than anyone party to this conversation have written things
like, "There never was no man nowhere so virtuous", so you'll have to pardon me
if I take exception to what's being asserted here.

Of course this doesn't mean the present usage isn't confusing. On that score I
also disagree - I think you're on outlier here - but I'm not sufficiently
enamoured of the present text to object to changing it.

> was nice and elegant as it is; sigh -- elegance has to give way to

This principle, taken to its logical conclusion, results in an unsupportable
state of affairs. Like it or not, any text we write is going to be confusing to
someone, somewhere, and a line has to be drawn or we'll never be able to finish
anything.

I speak as someone who has been on the receiving end of comments on the MIME
specification for 20+ years, so this is something I know a *lot* about. 

The only sensible policy is to include the possibility that the text as written
actually is quite clear, and for whatever reason the reviewer's (lack of)
understanding does not in fact indicate that there is a problem.

This point isn't going to change the outcome in the present case, but I want to
be clear that I strongly object to the general principle stated here.

>    NO response to the command.

I think this actually makes the prohibition against losing messages a bit less
clear, but it significantly improves the discussion of the "leave the message
in both mailboxes" case, so overall it's an improvement and I'm going to put it
in.

>    as for other messages that happen to have been expunged at the same

Ick. "It is always the case" is one of several circumlocutory phrases I'm trying
to stop using, so no, I don't think this is useful change to make.

>    any happen to have been expunged at the same time; this is normal

I think this is a good change to make so I'm including it.

>    unnecessarily difficult to process that sequence usefully.)

Yes, this is quite a bit better. Changed.

				Ned

---

From: "Moriarty, Kathleen" <[email-masked]>
Date: Wed, 21 Nov 2012 14:26:36 -0500

Thanks for addressing the comments and making the text more clear.  That was the main objective of the comments. 

Best regards,
Kathleen
________________________________________
From: Ned Freed [[email-masked]]
Sent: Wednesday, November 21, 2012 1:08 PM
To: Barry Leiba
Cc: Moriarty, Kathleen; [email-masked]; [email-masked]; [email-masked]; [email-masked]; Alexey Melnikov
Subject: Re: Gen-art review of draft-ietf-imapmove-command-02

                                Ned