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

From: Alexey Melnikov <[email-masked]>
Date: Sat, 1 Feb 2020 19:00:30 +0000

Hi,

> On 29 Jan 2020, at 14:20, Timo Sirainen <[email-masked]> wrote:
> 
>> 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.
>> 
>> 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")
> 
> I haven't seen this as a problem, so I don't really care either way.

I don’t see this as a problem and I don’t want to make the server check for reuse.

I am Ok with adding informative suggestion for clients to generate unique tags to ease in debugging issues. Would this work for people?
> 
>> 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.

I am fine with this change. Our server sometimes increment UIDNEXT during some failed COPY or APPEND, so obviously relaxing this would be better for us. 
> 
>> 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.

I agree this is a problem and happy to do what you suggest.
>> 
>> It still doesn't mean that you can reliably tell the correct octets for the matching mailbox from the LIST response though :(
> 
> Is it too late to add a new response code? Something like:
> 
> * OK [CREATED] TAG "a123" NAME "Denormalized name"

Response codes are cheap and don’t break older clients, so I am happy to add one.
> 
>> 6.3.2 SELECT command
>> 
>> 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. 

Agreed.

>> 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.
> 
> 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.

Let me think about this.
> 
>> 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.
> 
> \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.

I am happy to disallow this, or at least say that servers can either disallow delete of a mailbox with children or add \NoSelect to it.
> 
>> 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.
> 
> Dovecot doesn't allow RENAME INBOX in some configurations. I don't remember people complaining.

From the client prospective I am disagreeing with this, renaming INBOX is useful for archiving.

I am happy to say that some servers might disallow this though. Would that be Ok with people?
> 
>> 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
>> 
>> 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!)
> 
> 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).

I think I would rather weaken this to SHOULD and I would rather not require change in behaviour for servers that already comply with the MUST NOT.
> 
>> 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".
> 
> Same with Dovecot. I think should keep the OK behavior, since that's what is commonly been done.

I think returning OK if not subscribed would be a better behaviour for clients. So I would be happy to change the text.
> 
>> 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"?
> 
> 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/*

Exactly.
> 
> 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?

I kept the text because non empty reference is still allowed. I think I would rather allow it and mark the text as recommended server behaviour if non empty reference parameter is supplied. Maybe move it to a separate section for clarity?
> 
>> 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.

Good point. I think this should be allowed on the 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:
> 
> 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.

I am ambivalent on what server behaviour should be in this case. I am Ok with saying that the client can’t expect the server to check for changes.
> 
>> 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..

Exactly :-).
> 
>> 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.

Same here. That was the intent.
> 
>> 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.
> 
> 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,
> """

Ok, I will recheck the example.

> 
>> 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)?
> 
> Might be useful to require UID for any untagged FETCH reply. It can simplify client behavior.

I think we agreed to this. I think I mention this for IDLE, but didn’t say it in this section. What is the best place for this text?

Best Regards,
Alexey

---

From: Timo Sirainen <[email-masked]>
Date: Sun, 2 Feb 2020 17:16:34 +0200

On 1. Feb 2020, at 21.00, Alexey Melnikov <[email-masked]> wrote:
> 
> Hi,
> 
> On 29 Jan 2020, at 14:20, Timo Sirainen <[email-masked] <mailto:timo@sirainen..com>> wrote:
> 
>> On 29. Jan 2020, at 14.08, Bron Gondwana <[email-masked] <mailto:[email-masked]>> wrote:
>>> 
>>> 2.2.1 Client protocol
>>> 

Fine with me.

> Response codes are cheap and don’t break older clients, so I am happy to add one.

I guess an optional reply which can send both the changed name and/or mailboxid. Maybe something like:

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

Although I'm not sure if some part of it would be nicer inside parenthesis.

> I am happy to say that some servers might disallow this though. Would that be Ok with people?

Fine with me.

> I think I would rather weaken this to SHOULD and I would rather not require change in behaviour for servers that already comply with the MUST NOT.

But is the MUST NOT or SHOULD NOT behavior wanted nowadays anymore? Especially subscriptions not following RENAME is annoying for clients because they have to explicitly transfer the subscription as well. So my suggestion would be that if ENABLE IMAP4rev2 is used then subscriptions would follow RENAME/DELETE.

> I kept the text because non empty reference is still allowed. I think I would rather allow it and mark the text as recommended server behaviour if non empty reference parameter is supplied. Maybe m…

I think a separate section would be clearer.

> I think we agreed to this. I think I mention this for IDLE, but didn’t say it in this section. What is the best place for this text?

Maybe near the beginning of the 7.4.2 section?

---

From: "Bron Gondwana" <[email-masked]>
Date: Fri, 07 Feb 2020 15:03:51 +1100

Hi Alexey,

I agree with all the things Timo said. I've added a couple more comments below.

Do you want me to create github issues to track each thing, or are you happy that you know what changes are needed?

Again - sorry for the late final review and hence pushing the timeline back on this.

Cheers,

Bron.

> Response codes are cheap and don’t break older clients, so I am happy to add one.

Along with the instruction to add MAILBOXID if RFC8474 is supported, I'm good with this.

>> 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.

We still need a decision on this. I'm going to keep dropping connections regardless in this rare case, because there's a horrible long tail of clients, and debugging their broken caches if we ever confuse them is very difficult.

> I am happy to disallow this, or at least say that servers can either disallow delete of a mailbox with children or add \NoSelect to it.

Sounds good to me. Allowing the disallow is enough :)

>>> *6.3.6 RENAME *

Works for me.

>>> *6.3.7 SUBSCRIBE*

Yeah, totally. I'm fine with SHOULD, and I don't care about 

>>> *6.3.10 NAMESPACE*
> I kept the text because non empty reference is still allowed. I think I would rather allow it and mark the text as recommended server behaviour if non empty reference parameter is supplied. Maybe m…

I don't think any change is needed here, sorry for adding confusion.

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

---

From: Alexey Melnikov <[email-masked]>
Date: Fri, 7 Feb 2020 13:31:53 +0000

Hi Bron,

> I agree with all the things Timo said. I've added a couple more 
I started replying to your emails and applied some changes, but I 
haven't done all of them. I will send an email on this.
> Do you want me to create github issues to track each thing, or are you 
> happy that you know what changes are needed?
I think I am Ok for now without tracking issues in github.
> Again - sorry for the late final review and hence pushing the timeline 

No worries.

Best Regards,

Alexey

>>> 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.
>>>
>>> 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.
>>
>> Let me think about this.
>
> We still need a decision on this. I'm going to keep dropping 
> connections regardless in this rare case, because there's a horrible 
> long tail of clients, and debugging their broken caches if we ever 
> confuse them is very difficult.
>
>>> 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..
>>
>> I am happy to disallow this, or at least say that servers can either 
>> disallow delete of a mailbox with children or add \NoSelect to it.
>
>>>> *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).
>>>
>>> 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*
>>>>
>>>> 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.
>>>>
>>> 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).
>>
>> I think I would rather weaken this to SHOULD and I would rather not 
>> require change in behaviour for servers that already comply with the 
>> MUST NOT.
>
>>>> We've created a conflict here.  The NAMESPACE command contains 
>>>> prefix strings for the individual namespaces, and says:
>>>>
>>>> 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"?
>>>
>>> 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/*
>>
>> Exactly.
>>>
>>> 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?
>>
>> I kept the text because non empty reference is still allowed. I think 
>> I would rather allow it and mark the text as recommended server 
>> behaviour if non empty reference parameter is supplied. Maybe move it 
>> to a separate section for clarity?
>
> _______________________________________________
> Extra mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/extra

---

From: Alexey Melnikov <[email-masked]>
Date: Fri, 7 Feb 2020 16:57:52 +0000

Hi,

On 29 Jan 2020, at 14:20, Timo Sirainen <[email-masked] 
<mailto:timo@sirainen..com>> 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.
>>
>> 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")
>

I don’t see this as a problem and I don’t want to require servers to 
check for reuse.

I am Ok with adding informative suggestion for clients to generate 
unique tags to ease in debugging issues. Would this work for people?
>
>> *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.

I was initially fine with this change, but after rereading this sentence 
I think it is mostly Ok as is. I think a possible point to clarify is 
that if a message is added and removed in another session, the UIDNEXT 
will be updated, even if the change is not reported in the current 
session. This is perfectly fine behaviour and should also work for the 
virtual mailbox case.

>> *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 :(
>

Response codes are cheap and don’t break older clients, so I am happy to 
add one.
>
>> *6.3.2 SELECT command
>> *
>>
>> 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.

Agreed.

> 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.

Let me think about this.
>
>> *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.
>
> \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.
>
> CREATE a/b could create "a" se \NoSelect also.

I think the only safe for clients to do is to assume that deleting a 
mailbox with submailboxes is not going to work, so the client needs to 
delete each child separately.
>> RENAME INBOX Other, but maybe I've forgotten.  I don't see it in the 
>> minutes, so maybe it's just my wishful thinking.
>

 From the client prospective I am disagreeing with this, renaming INBOX 
is useful for archiving.

I am happy to say that some servers might disallow this though. Would 
that be Ok with 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.
>>

>
>> *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".
>

I think returning OK if not subscribed would be a better behaviour for 
clients. (I have lots of comments in my client asking what should be 
done if UNSUBSCRIBE fails, for example.) So I would be happy to change 
the text.
>
>> *6.3.10 NAMESPACE*
>>
>> We've created a conflict here.  The NAMESPACE command contains prefix 
>> strings for the individual namespaces, and says:
>>
>> 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"?
>

Exactly.
> 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?

I kept the text because non empty reference is still allowed. I think I 
would rather allow it and mark the text as recommended server behaviour 
if non empty reference parameter is supplied. Maybe move it to a 
separate section for clarity?
>> *6.3.11 STATUS
>> *
>> 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 

>>
>> 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:
>
> 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.

I am ambivalent on what server behaviour should be in this case. I am Ok 
with saying that the client can’t expect the server to check for changes.
>
>> 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..

Exactly :-).
> 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 it should 
>> not have altered the variable.
>
> Right. And even if it did contain RETURN SAVE option it still 
> shouldn't have altered the variable, because:
>
> """

>
>> *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)?
>

I think we agreed to this. I think I mention this for IDLE, but didn’t 
say it in this section. What is the best place for this text? At the top 
of the section describing FETCH response?

Best Regards,
Alexey

---

From: Alexey Melnikov <[email-masked]>
Date: Fri, 7 Feb 2020 17:04:16 +0000

Hi Bron,

Replying to issues raised not covered by my reply to Timo:

On 29/01/2020 12:08, Bron Gondwana wrote:
> 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.
Done.
> *2.3.2 Flags Message Attribute
> *
>
> formatting issue: $Forwarded is duplicated 3 times and $MDNSent is 
> duplicated twice in the plain-text format.

This is actually a bug in TXT rendering from XML.

  [snip]

> *6.3.1 ENABLE command
> *
>
> The example shows a conversation with an imap4rev1 only server.
Fixed.
> 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.

Actually, it is perfectly Ok for the server to return in response to C2:

* ENABLED QRESYNC CONDSTORE

The text in the document wasn't entirely clear. It was trying to say 
that the preferred server behaviour is:

L1 ENABLE A B
* ENABLED A B
L1 OK foo

L2 ENABLE C
* ENABLED C
L2 OK foo

and not

L2 ENABLE C
* ENABLED A B C
L2 OK foo

I clarified this.
>
> (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.
Again, this is a tool issue.
> /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.**
> *
I will check. I think the section reference for response codes is missing.
> The last paragraph of this section has the word "deprecated" spelled 
> as "depractated".
Fixed.
> remove rule.  Should we weaken it to SHOULD?
Done.
>   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.
>
> is successful. /- this is not clear on what happens if the mailbox is 
I agree that your server behaviour is sensible. Updated description of 
both SUBSCRIBE and UNSUBSCRIBE.
>
> . 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.
I will double check, but I think it is another rendering issue to TXT 
format (there is a comment in bettween 2 parts of the sentence.
>
> *6.3.9.2 LIST Return Options
> *
>
> The CHILDREN item is duplicated with two different pieces of text.

Again, a rendering issue. These are actually 2 separate paragraphs under 
the same item.

> *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.

Good point. Fixed.

> *6.4.4.1 SAVE Result
> wipe the SAVED result, and that trying to use a SAVED result when 
> there isn't one should return an error.
I think Timo explained this well in his response.

  [snip]

>
> *7.1 Status Responses
> *
>
> Using 'o' as a tag in the ALREADYEXISTS item is confusing, it looks 
> like an attempt to do dot points!
Changed :-).

  [snip]

Best Regards,

Alexey

---

From: "Bron Gondwana" <[email-masked]>
Date: Fri, 14 Feb 2020 10:47:15 +1100

Thanks Alexey, I see there's a -12 there now, and I'm happy with all the edits in it :)

There are still some outstanding points from this thread - are you expecting to issue a -13 for those?

Thanks,

Bron.

>> 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.
> Done.
> 
>> *2.3.2 Flags Message Attribute*
>> 
>> formatting issue: $Forwarded is duplicated 3 times and $MDNSent is duplicated twice in the plain-text format.

>> *6.3.1 ENABLE command*
>> 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
>> The second doesn't say "ENABLED CONDSTORE" because it was already enabled earlier on this connection.

> The text in the document wasn't entirely clear. It was trying to say that the preferred server behaviour is:
> 
> L1 ENABLE A B
> * ENABLED A B
> L1 OK foo
> 
> L2 ENABLE C
> * ENABLED C
> L2 OK foo
> 
> and not 
> 
> L2 ENABLE C
> * ENABLED A B C
> L2 OK foo
> 
> I clarified this.
>> 
>> *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".
> Fixed.
> 
>> 
>> *6.3.7 SUBSCRIBE*
>> 
>> Speak of the devil. Lots of sites violate the MUST NOT unilaterally remove rule. Should we weaken it to SHOULD?
> Done.
> 
>>  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.
>> 
>> *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?
>> 
> I agree that your server behaviour is sensible. Updated description of both SUBSCRIBE and UNSUBSCRIBE.
> 
>> 
>> . create INBOX.foo
>> *6.3.9 LIST*
>> There's a weird line wrapping near the top of page 44 where a paragraph is split in half:
>> 

>> 
> I will double check, but I think it is another rendering issue to TXT format (there is a comment in bettween 2 parts of the sentence.
> 
>> 
>> *6.3.9.2 LIST Return Options*
>> 
> Again, a rendering issue. These are actually 2 separate paragraphs under the same item.

* 
>> 
>> 
>> 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.
> Good point. Fixed.

> 
>> *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.

>> 
>> *7.1 Status Responses*
>> 
>> Using 'o' as a tag in the ALREADYEXISTS item is confusing, it looks like an attempt to do dot points!
> Changed :-).
> 
>  [snip]