Subject: [Extra] Review: draft-ietf-extra-imap4rev2-11

From: "Bron Gondwana" <[email-masked]>
Date: Wed, 29 Jan 2020 23:08:55 +1100

Sorry about the delay in doing this! I was supposed to be doing the shepherding write-up, and so I figured I should read it properly.

 I've had a full read through over some very many days (I admit, I skimmed through the ABNF) and I have comments. It's long, and hopefully comprehensive - though I'm sure I missed things.

*2.1 Link level
*

Is it appropriate to mention TLS on 993 here? It's mentioned deep within the document, but not here.

*2.2.1 Client protocol*
*
*
*"A different tag is generated by the client for each command".* There's no "MUST" on this, and certainly I've seen clients which don't. Do we want to make that more strict in rev2? It would be a bit of a pain on the commandline where I normally use ".", but it's also annoying as a server author that you can't rely on tag uniqueness.

Maybe the client SHOULD generate a different tag for every command, but a server MUST accept tag reuse (could restrict concurrency, a tag can only be reused after it's been "returned")

*2.3.1.1 UID Attribute
*
*
*
*"The next unique identifier value MUST NOT change unless new messages are added to the mailbox". * I would very much like that first MUST NOT to be weakened to "SHOULD NOT". My reasoning is that you could have a virtual mailbox which used a global sequence number per message. It would behave to clients as if a message had been delivered and then immediately expunged. I'm pretty sure clients don't care - they'll fetch the range, discover that nothing new has appeared, and stay happy. The second MUST is obviously correct, that the UIDNEXT must increase when messages are delivered.

Both UID and UIDVALIDITY say "unsigned 32-bit value" - but strictly (as seen later in the ABNF) they aren't allowed to be zero. It would be good to say "non-zero" when each is first mentioned, but I don't care strongly about this one.

*2.3.2 Flags Message Attribute
*

formatting issue: $Forwarded is duplicated 3 times and $MDNSent is duplicated twice in the plain-text format.

Should this section also reference https://www.iana.org/assignments/imap-jmap-keywords/imap-jmap-keywords.xhtml ?

*5.1 Mailbox Naming
*

This doesn't solve one of my naming bugbears. The spec says things like *"servers MAY accept a de-normalized UTF-8 mailbox name and conver it to Net-Unicode prior to mailbox creation." -*** **there is no clear way to know what the SELECTable name will be after creating a mailbox.

If I was designing this myself from scratch I would say that "SELECT of the same octets that were passed to CREATE MUST open that same mailbox" or something to that effect. Since we're messing with how mailbox naming works somewhat for imap4rev2, I'd be keen to add this restriction on server behaviour so that clients don't fall into a "CREATE -> ALREADY EXISTS / SELECT -> DOESN'T EXIST" loop.

It still doesn't mean that you can reliably tell the correct octets for the matching mailbox from the LIST response though :(

*6.3.1 ENABLE command
*

The example shows a conversation with an imap4rev1 only server.

The examples don't clearly show that only the newly enabled responses are correct, e.g. the sequence:

C1 ENABLE CONDSTORE
* ENABLED CONDSTORE
C1 OK foo
C2 ENABLE CONDSTORE QRESYNC
* ENABLED QRESYNC
C2 OK foo

The second doesn't say "ENABLED CONDSTORE" because it was already enabled earlier on this connection.

(AKA: clients MUST remember what they have enabled)

*6.3.2 SELECT command
*

The [UIDNEXT <n>] response is duplicated with two different pieces of text.

*When deselecting a selected mailbox, the server MUST return an untagged OK response with the "[CLOSED]" response code when the currently selected mailbox is closed (see Paragraph 10). *- I'm not sure where "Paragraph 10" is.

The last paragraph of this section has the word "deprecated" spelled as "depractated".

I also saw somewhere that the server can unilaterally send a "* [CLOSED]" response at any point if it's had to close the mailbox (e.g. it got deleted, or IO errors). I'm not sure if that requires opt-in to IMAP4REV2, since otherwise clients will not be aware to watch for it. This may need to be clarified. Current behaviour of our server right now is to drop the connection entirely, which is ugly - but it's rare and all clients already need to handle it cleanly, so it guarantees no messed up data model for the clients.

*6.3.4 and 6.3.5 CREATE and DELETE
*

Can we make it illegal to "DELETE" a mailbox with children rather than make it a \NoSelect mailbox pretty pretty please? I'm just looking at some patches to deal with Outlook misbehaving with this precise situation, and it's all horrible. Because of mailboxId requirements for JMAP we've been forced to keep these things around as "intermediate" mailboxes anyway, and it's been the source of tons of bugs.

*6.3.6 RENAME *

I thought we had agreed to strongly recommend against clients using RENAME INBOX Other, but maybe I've forgotten. I don't see it in the minutes, so maybe it's just my wishful thinking.

We definitely have an option in Cyrus IMAP to make subscriptions follow RENAME. It's one of the warts (largely due to Thunderbird showing greyed out folders and confusing people).

*6.3.7 SUBSCRIBE
*

Speak of the devil. Lots of sites violate the MUST NOT unilaterally remove rule. Should we weaken it to SHOULD? The justification for the MUST is "the server site might choose to do X" - well, if the server site is doing that, then maybe they SHOULDN'T unilaterally remove the subscriptions, but otherwise it's a better user experience and what people want.

(also: by enabling a non-default break the rules config, it's what our server does!)

*6.3.8 UNSUBSCRIBE*

*This command returns a tagged OK response only if the unsubscription is successful. *- this is not clear on what happens if the mailbox is not currently subscribed - does it silently succeed, or is there an expected error?

For what it's worth, our server just says "OK Completed" even if it's already been unsubscrbed - and likewise for subscribe if it's already subscribed it just say "yep, all good".

. create INBOX.foo
. OK [MAILBOXID (7eca8f55-e233-4458-9d48-af854fb4b279)] Completed
. subscribe INBOX.foo
. OK Completed
. unsubscribe INBOX.foo
. OK Completed
. unsubscribe INBOX.foo
. OK Completed
. subscribe INBOX.foo
. OK Completed
. subscribe INBOX.foo
. OK Completed

*6.3.9 LIST
*

There's a weird line wrapping near the top of page 44 where a paragraph is split in half:

   The reference and mailbox name arguments are interpreted into a
   canonical form that represents an unambiguous left-to-right
   hierarchy.  The returned mailbox names will be in the interpreted
   form,

   that we call "canonical LIST pattern" later in this document.  To
   define the term "canonical LIST pattern" formally: it refers to the
   canonical pattern constructed internally by the server from the
   reference and mailbox name arguments.

*6.3.9.2 LIST Return Options
*

The CHILDREN item is duplicated with two different pieces of text.

*6.3.9.7 LIST EXAMPLES
*

We've kinda half-added SPECIAL-USE, but we don't reference it, and it isn't shown in these examples.

*6.3.10 NAMESPACE
*

We've created a conflict here. The NAMESPACE command contains prefix strings for the individual namespaces, and says:

   The prefix string allows a client to do things such as automatically
   creating personal mailboxes or LISTing all available mailboxes within
   a namespace.

Which contradicts the advice we gave back in the LIST command not to use prefix. Perhaps we should change the LIST advice to be "only use the empty prefix or prefixes given in the NAMESPACE response"?

*6.3.11 STATUS
*

      Note: The STATUS command is intended to access the status of
      mailboxes other than the currently selected mailbox.  Because the
      STATUS command can cause the mailbox to be opened internally, and
      because this information is available by other means on the
      selected mailbox, the STATUS command SHOULD NOT be used on the
      currently selected mailbox.

This advice is all well and good, but it says nothing about what the server may or may not do when it gets one of these STATUS commands. This is particularly relevant in the context of LIST RETURN STATUS, where it's hard to construct list patterns which explicitly avoid the currently selected mailbox.

Likewise with the: *MUST NOT be used as a "check for new messages in the selected mailbox" *assertion. Plenty of clients will do a STATUS run across folders and if they're doing LIST RETURN STATUS they're gonna find out about the new messages if the server returns it. Clients can not rely on it working is a more honest representation of what happens.

I'd like to see that IMAP4REV2 servers MUST implement STATUS of the currently selected mailbox. Yes, it's a special codepath - but if you're already testing if it's the currently open mailbox then you're at exactly the right place to do the right thing:

 /* use the index status if we can so we get the 'alive' Recent count */
 if (!strcmpsafe(mbentry->name, index_mboxname(imapd_index)) && imapd_index->mailbox)
 return index_status(imapd_index, sd);

 /* fall back to generic lookup */
 return status_lookup_mbentry(mbentry, imapd_userid, statusitems, sd);

*6.4.4 SEARCH
*

   For example,
   the criteria DELETED FROM "SMITH" SINCE 1-Feb-1994 refers to all
   deleted messages from Smith that were placed in the mailbox since
   February 1, 1994.

That's not strictly true - SAVEDATE would be "placed in the mailbox". The wording should say "with INTERNALDATE greater than February 1, 1994" or so.

*6.4.4.1 SAVE Result
*

I'm not sure I understand the interaction here with closing and re-opening a mailbox. I would expect that closing a mailbox would wipe the SAVED result, and that trying to use a SAVED result when there isn't one should return an error.

Speaking of which, Example 5 is wrong: It says that the second search command leads to the variable containing no messages, but the second search command did not contain the RETURN SAVE option, so it should not have altered the variable.

*7.1 Status Responses
*

Using 'o' as a tag in the ALREADYEXISTS item is confusing, it looks like an attempt to do dot points!

*7.2.3 LIST Response
*

This is the interesting bit for SPECIAL-USE - it should reference that here - as well as referencing the registry since it reproduces some of it inline.

https://www.iana.org/assignments/imap-mailbox-name-attributes/imap-mailbox-name-attributes.xhtml

*7.4.2 FETCH Response
*

I remember past bugs with UID in "unsolicited" FLAGS responses. Do we need to require that including UID only happen after ENABLE IMAP4REV2 (as it is for ENABLE QRESYNC)?

*Appendix C: Changes
*

Right now it's still stated as "the planned changes" and there's still some TODO in there (some of which I think could be well handled by referencing the registries rather than replicating them inline).

--
 Bron Gondwana, CEO, Fastmail Pty Ltd
 [email-masked]

---

From: Timo Sirainen <[email-masked]>
Date: Wed, 29 Jan 2020 16:20:58 +0200

On 29. Jan 2020, at 14.08, Bron Gondwana <[email-masked]> wrote:
> 
> 2.2.1 Client protocol
> 
> "A different tag is generated by the client for each command".  There's no "MUST" on this, and certainly I've seen clients which don't.  Do we want to make that more strict in rev2?  It would be a bit of a pain on the commandline where I normally use ".", but it's also annoying as a server author that you can't rely on tag uniqueness.
> 

I haven't seen this as a problem, so I don't really care either way.

> 2.3.1.1 UID Attribute
> 
> "The next unique identifier value MUST NOT change unless new messages are added to the mailbox".  I would very much like that first MUST NOT to be weakened to "SHOULD NOT".  My reasoning is that you could have a virtual mailbox which used a global sequence number per message.  It would behave to clients as if a message had been delivered and then immediately expunged.  I'm pretty sure clients don't care - they'll fetch the range, discover that nothing new has appeared, and stay happy.  The second MUST is obviously correct, that the UIDNEXT must increase when messages are delivered.

Agreed.

> 5.1 Mailbox Naming
> 
> This doesn't solve one of my naming bugbears.  The spec says things like "servers MAY accept a de-normalized UTF-8 mailbox name and conver it to Net-Unicode prior to mailbox creation." - there is no clear way to know what the SELECTable name will be after creating a mailbox.
> 

Is it too late to add a new response code? Something like:

* OK [CREATED] TAG "a123" NAME "Denormalized name"

> 6.3.2 SELECT command

Yeah, I'd prefer just BYE instead of unilateral CLOSED. I don't think clients are going to implement it correctly since it almost never happens, so nobody tests that code path.

> 6.3.4 and 6.3.5 CREATE and DELETE
> 

\NoSelect seems to be causing confusion in general, and Dovecot nowadays has options that makes it try to avoid creating them. But I'm not sure if it needs to be made illegal in the RFC itself. I think your server implementation can just decide to reject DELETE if it has children.

It sounds like your issues are with \NoSelect mailboxes in general, not just this specific way of creating them with DELETE? For example CREATE a/b could create "a" se \NoSelect also.

> 6.3.6 RENAME 

Dovecot doesn't allow RENAME INBOX in some configurations. I don't remember people complaining.

> We definitely have an option in Cyrus IMAP to make subscriptions follow RENAME.  It's one of the warts (largely due to Thunderbird showing greyed out folders and confusing people).

We haven't needed to do this yet. I think the \NoSelect folders have been more of a reason for Thunderbird confusion.

> 6.3.7 SUBSCRIBE

The original use case for not auto-removing a subscription is rather ancient already. I'd be fine with changing subscriptions to follow deletes and renames in the rev2 mode (but better not change behavior in rev1 mode).

> 6.3.8 UNSUBSCRIBE
> 
> This command returns a tagged OK response only if the unsubscription is successful. - this is not clear on what happens if the mailbox is not currently subscribed - does it silently succeed, or is there an expected error?
> 

Same with Dovecot. I think should keep the OK behavior, since that's what is commonly been done.

> 6.3.10 NAMESPACE

I think you're mixing "namespace prefixes" with "list references"? I don't think the above sentence advocates using LIST reference parameter, just using the namespace prefix for listing e.g. LIST "" ns-prefix/*

Anyway, I see lots and lots of text is still preserved for the LIST reference parameter explanation, even though it shouldn't be used. Couldn't we just decide to get rid of it entirely in rev2 and say the reference MUST be empty? Or say only that is is implementation-defined and refer to IMAP4rev1 if anyone wants to see how it used to be implemented?

> 6.3.11 STATUS
> Likewise with the: MUST NOT be used as a "check for new messages in the selected mailbox" assertion.  Plenty of clients will do a STATUS run across folders and if they're doing LIST RETURN STATUS they're gonna find out about the new messages if the server returns it.  Clients can not rely on it working is a more honest representation of what happens.
> 

Also this is the only way to get the current UIDNEXT of the selected mailbox (since it might be higher than last message's UID).

The main ambiguity though is whether STATUS on the current mailbox is expected to check for new changes. Dovecot implements it in a way that it doesn't check for changes. I think there used to be even a good reason for that.

> 6.4.4.1 SAVE Result
> 
> I'm not sure I understand the interaction here with closing and re-opening a mailbox.  I would expect that closing a mailbox would wipe the SAVED result,

I think it practically says that, since it says that SELECT/EXAMINE resets it to empty sequence. It's not really relevant whether it happens when the mailbox is closed or when the next mailbox is opened, because you can't use it in non-selected state anyway..

> and that trying to use a SAVED result when there isn't one should return an error.

It says that the default is an empty sequence. So it won't result in an error, just a non-matching set. I'm fine with this behavior, especially since it's easier to implement and I don't really see much of a downside.

> Speaking of which, Example 5 is wrong: It says that the second search command leads to the variable containing no messages, but the second search command did not contain the RETURN SAVE option, so…

Right. And even if it did contain RETURN SAVE option it still shouldn't have altered the variable, because:

"""
   Any of the following SEARCH commands MUST NOT change the search
   result variable:

      a SEARCH command that caused the server to return the BAD tagged
      response,
"""

> 7.4.2 FETCH Response

Might be useful to require UID for any untagged FETCH reply. It can simplify client behavior.

---

From: "Bron Gondwana" <[email-masked]>
Date: Thu, 30 Jan 2020 08:52:25 +1100

On Thu, Jan 30, 2020, at 01:20, Timo Sirainen wrote:
>> **

I don't care either way, it's more "if I was writing a server it wouldn't be clear to me that a client might just send . every time".

>> *2.3.1.1 UID Attribute*
>> **
>> *5.1 Mailbox Naming*

Presumable the name would be an astring. If we did this, I'd want to say "if OBJECTID is supported, MAILBOXID goes into this response as well".

>> *6.3.2 SELECT command*
>> *6.3.4 and 6.3.5 CREATE and DELETE*
>> 

That's indeed what happens now in our server:

. list "" a*
* LIST (\HasNoChildren) "/" a/b/c
. OK Completed (0.001 secs 3 calls)
. list "" a/%
* LIST (\Noselect \HasChildren) "/" a/b
. OK Completed (0.001 secs 3 calls)
. list "" a
* LIST (\Noselect \HasChildren) "/" a
. OK Completed (0.000 secs 3 calls)

> Dovecot doesn't allow RENAME INBOX in some configurations. I don't remember people complaining.

Excellent. I'm keen to turn it off on our server. I should probably scan the logs and see how often it gets used!

>> *6.3.7 SUBSCRIBE*
>> *6.3.10 NAMESPACE*
> Anyway, I see lots and lots of text is still preserved for the LIST reference parameter explanation, even though it shouldn't be used. Couldn't we just decide to get rid of it entirely in rev2 and…

There was some discussion about this - I'll leave it for Barry/Alexey to reply on this one, since they've thought about it more than I have.

>> *6.3.11 STATUS*
> The main ambiguity though is whether STATUS on the current mailbox is expected to check for new changes. Dovecot implements it in a way that it doesn't check for changes. I think there used to be e…

Well.. if it doesn't check for changes presumably it can't tell you the correct UIDNEXT? Either that or it will have an inconsistent response for the selected folder.

>> *6.4.4.1 SAVE Result*

Right - so that whole section is pointless. This text:

   If the server decides to send a new UIDVALIDITY value while the
   mailbox is opened, this causes resetting of the search variable to
   the empty sequence.

If any SELECT resets the variable.

> It says that the default is an empty sequence. So it won't result in an error, just a non-matching set. I'm fine with this behavior, especially since it's easier to implement and I don't really see…

Yep, fair enough. I haven't implemented $ yet, but it will be easy.

>> *7.4.2 FETCH Response*

We definitely changed from doing it always because some clients couldn't handle it - but that was 10 years ago, maybe those clients don't exist any more. Sadly I don't remember which clients it was.

Cheers,

Bron.

---

From: Timo Sirainen <[email-masked]>
Date: Thu, 30 Jan 2020 10:25:58 +0200

On 29. Jan 2020, at 23.52, Bron Gondwana <[email-masked]> wrote:

>>> 5.1 Mailbox Naming
>>> 
>>> This doesn't solve one of my naming bugbears.  The spec says things like "servers MAY accept a de-normalized UTF-8 mailbox name and conver it to Net-Unicode prior to mailbox creation.." - there is no clear way to know what the SELECTable name will be after creating a mailbox.
>>> 

Yeah, would be nice.

> . OK Completed (0.000 secs 3 calls)

This is also server-specific whether the "a" would be listed as part of "a*". Or in Dovecot case it's mailbox format specific - it happens with some formats but not with others.

> Well.. if it doesn't check for changes presumably it can't tell you the correct UIDNEXT? Either that or it will have an inconsistent response for the selected folder.

I think arguably it's even more correct that the UIDNEXT tells what it is in the current IMAP session state rather than the refreshed state. (Although I wouldn't want to require that in the RFC.) So for example:

a status inbox (uidnext)
* STATUS inbox (UIDNEXT 10)
a OK [CLIENTBUG] Status on selected mailbox completed (0.001 + 0.000 secs).

## deliver new mail

b status inbox (uidnext)
* STATUS inbox (UIDNEXT 10)
b OK [CLIENTBUG] Status on selected mailbox completed (0.001 + 0.000 secs).
c noop
* 2 EXISTS
* 2 RECENT
c OK NOOP completed (0.001 + 0.000 secs).
d status inbox (uidnext)
* STATUS inbox (UIDNEXT 11)
d OK [CLIENTBUG] Status on selected mailbox completed (0.001 + 0.000 secs).

I'll probably remove that [CLIENTBUG]..

> If any SELECT resets the variable.

I'm not sure I understand your point. This text is pointless? The first paragraph discussing SELECT resetting the sequence is pointless? They seem somewhat complementary to me. Actually I think it would most make sense to not allow IMAP server to change UIDVALIDITY while a mailbox is selected. I highly doubt there are many clients that can handle it correctly.

>>> 7.4.2 FETCH Response
>>> 
>>> I remember past bugs with UID in "unsolicited" FLAGS responses..  Do we need to require that including UID only happen after ENABLE IMAP4REV2 (as it is for ENABLE QRESYNC)?
>> 

This behavior can be enabled only after ENABLE IMAP4rev2 so it wouldn't matter if some old clients couldn't handle it.