[Extra] AD Review of draft-ietf-extra-imap4rev2

"Murray S. Kucherawy" <superuser@gmail.com> Mon, 04 January 2021 08:19 UTC

Return-Path: <superuser@gmail.com>
X-Original-To: extra@ietfa.amsl.com
Delivered-To: extra@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 23F2A3A0127 for <extra@ietfa.amsl.com>; Mon, 4 Jan 2021 00:19:02 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: 0.311
X-Spam-Level:
X-Spam-Status: No, score=0.311 tagged_above=-999 required=5 tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, THIS_AD=0.498, T_KAM_HTML_FONT_INVALID=0.01, URIBL_BLOCKED=0.001] autolearn=no autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id fpky17xj8-Q8 for <extra@ietfa.amsl.com>; Mon, 4 Jan 2021 00:18:59 -0800 (PST)
Received: from mail-vk1-xa29.google.com (mail-vk1-xa29.google.com [IPv6:2607:f8b0:4864:20::a29]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id CBE1F3A0121 for <extra@ietf.org>; Mon, 4 Jan 2021 00:18:58 -0800 (PST)
Received: by mail-vk1-xa29.google.com with SMTP id p128so5978188vkf.12 for <extra@ietf.org>; Mon, 04 Jan 2021 00:18:58 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:from:date:message-id:subject:to; bh=9vLC5WTtcLbboPvVHItRzQ/ZZ4hOOMiAu0RPFCbSd4M=; b=Je4/SOQd98jVfWlSpKlyC0c6ELLjAzWBm6OwFwuisJ7s5CDGWE7/N+50WIp4X9/0Pb afTlc39OQEM+C3H/wzwLbQPWOnZ3ZTdGDG/KhWQyU898goTWcDNUD8eE5gVx9EG/CMK3 +tz8HcC+NSkdh6ULNZnJJBZHOBa3S3xjYnCK97z358lhwad47eSyTkomR77RzXOEQSB9 F0q6+a13+WHc/l5nnhTyKV+nCnyE+s0JtfV9Ecu8JrLR9UHJMJvIbQQS0sJ1VawKlhBZ XVqEHPaaXjuZfood5kqOkzqSdSi5wOoIwacM10g+2VLT5LeWnv4hUVcF7O0U56Bizj5m dIMw==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:from:date:message-id:subject:to; bh=9vLC5WTtcLbboPvVHItRzQ/ZZ4hOOMiAu0RPFCbSd4M=; b=qtAsVqcwYNEytDvtX+SaNUfM0wRp2Yhhk3iKkdyRwazmQi+HKKE38a9G88iFhCdv+Q +rDmijG1bIQmm9TlVSh5NIDaCWgjCBQfPnFN0NsjE4dFVPUNzSbLXGDzgw7HNE9xRNs5 nq1iUSS8mlfQsuhmI/iKm4wf81qWrpiK1U9kM99yXCPh58mLXNCMoKE7NpS/rvGlYqUV uUqciikB8Wc9tWcYOElrkEXcqIx0+PhjykuENBctBjUYAIZqRBM7MKy/3z1rybtjU22p vuxNJ96v9W7sEjzBU8yHYmfyPQiIzWTjvTZgJ0vzG9IghifMMwtoSGHJQecPTEKn3dua xMZw==
X-Gm-Message-State: AOAM530KHLtBVSXkdOi/ZJMemx0IsjuXN1omsGjoo+Rbf2FQbIytB8dl 2kQ/ZOtei3dCea1EliTuk3s51oq08yh8E5yRslpdR5dzqyy6xw==
X-Google-Smtp-Source: ABdhPJylbayMYAUVy62Uvj3JGdiNXvbwi71G7u5igPLwxHhWMowOZnPBvdMnU9S/96tgY6sEdkzCXXkfVAvcheYAGN4=
X-Received: by 2002:a1f:30cd:: with SMTP id w196mr44929946vkw.13.1609748337042; Mon, 04 Jan 2021 00:18:57 -0800 (PST)
MIME-Version: 1.0
From: "Murray S. Kucherawy" <superuser@gmail.com>
Date: Mon, 04 Jan 2021 00:18:46 -0800
Message-ID: <CAL0qLwaLa+PuGWRrKTbpmDa_SWKT9ZQUEQ9dsPgXfUmTzcYAYw@mail.gmail.com>
To: extra@ietf.org
Content-Type: multipart/alternative; boundary="000000000000cea78a05b80ebd11"
Archived-At: <https://mailarchive.ietf.org/arch/msg/extra/hVCavR4AYimfDkf2iwjXdWw8H2A>
Subject: [Extra] AD Review of draft-ietf-extra-imap4rev2
X-BeenThere: extra@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Email mailstore and eXtensions To Revise or Amend <extra.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/extra>, <mailto:extra-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/extra/>
List-Post: <mailto:extra@ietf.org>
List-Help: <mailto:extra-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/extra>, <mailto:extra-request@ietf.org?subject=subscribe>
X-List-Received-Date: Mon, 04 Jan 2021 08:19:02 -0000

At long last. Thanks for your patience. This is a lot of material to sift
through!

This AD review is based on -22.  I’m coming at this as someone who has made
almost no direct use of IMAP as a user and certainly never implemented it,
so expect some neophyte-style feedback here.  I also (briefly) reviewed the
diff between RFC 3501 and this version.


Away we go...

Section 1.2

Should the “C:” and “S:” paragraph also mention that CRLF termination is
implicit in examples?  I realize Section 2.2 makes it clear that’s the
protocol anyway, so maybe this isn’t necessary, but I’ve seen it made
explicit in other documents so I thought I’d mention it.

Section 2.2.1

  Clients MUST follow the syntax outlined in this specification

   strictly.  It is a syntax error to send a command with missing or

   extraneous spaces or arguments.

It seems unnecessary to say this in a standards track document.  There’s a
similar paragraph in Section 2.2.2.

Section 2.2.2

   In the case of certain server data, the data MUST be recorded.

This seems misplaced; the MUST should be in the descriptions for the
specific cases where this requirement exists.

Section 2.3

Most of the subsections below here start with a clause that defines the
term, but it’s not a sentence.  This may be personal preference, but I
suggest adjusting them slightly so they’re sentences.  I’ve included an
example below where I have an adjacent suggestion and thus fixed the whole
paragraph.

Section 2.3.1.1


“STRONGLY ENCOURAGES” in all caps seems awkward.  I’m tempted to say
something about it not being valid BCP 14, etc.

Section 2.3.2

The opening sentence isn’t a sentence.  Also later, “Keyword” is introduced
without indicating that it’s one of the two possible flag types (which I
believe to be the case).  I suggest:

  A message has associated with it a list of zero or more named tokens,

   known as “flags”. A flag is set by its addition to this list, and is
cleared by its

   removal.  There are two types of flags in IMAP4rev2: system flags, and
keywords.

   A flag of either type can also be permanent or session-only.

Second paragraph: s/begin/begins/

s/in used in/in use in/

The text surrounding $Phishing goes further into human factors work than
other working groups have felt appropriate.  (DKIM and DMARC come to
mind.)  Are we sure that belongs here?

It seems strange to oblige a client detecting both $Junk and $NotJunk being
set to deal with that condition by fixing it.  The advice to ignore them is
fine, however.

It’s also curious to put a normative SHOULD on registering keywords, as
IANA activity is outside of the protocol.  It’s more typical to say you
SHOULD ignore keywords you don’t recognize, and then make a reference to
the registry.

Should this section mention whether the system flags are permanent?  \Seen,
for example, is clearly a system flag, but it could also be a session flag.

Section 2.3.3

All the SHOULDs in here make me wonder when an implementor might
legitimately choose to do something different.  Or maybe instead of “SHOULD
be”, just say “is”?

Section 4.3

4th paragraph: s/alternate/alternative/

Section 5.1

There’s a reference “see the Formal Syntax” which could probably benefit
from a section number reference as well.  This is actually something I’d
thought about many times earlier in the document up to this point, but I’m
finally saying something here; some of them were clear references like
this, while others were references to commands or flags that are “described
elsewhere in this document”.

Section 5.1.2.1

Although both “Implementer” and “Implementor” appear to be correct, you use
the latter everywhere in the document except here.  (Actually, Google docs
suggests the former.)

Section 5.2

“Sometimes, such behavior is REQUIRED.”  This isn’t a specific normative
statement, so I’d use the lowercase version here, and use BCP 14 language
when you’re presenting specific cases.

Section 5.3

Maybe my understanding is wrong, but if you do a non-blocking write while
you don’t have enough buffer space available, won’t some of what you write
be lost, resulting in gibberish at the other end?  Maybe this needs to
mention retries of stuff that couldn’t be sent (if any) when a non-blocking
write returns?

I’m tempted to suggest just removing this.  This is something you deal with
at the socket layer, and IMHO isn’t really part of IMAP itself.

Section 6.1.1

Is the stuff about XBLURDYBLOOP still appropriate given BCP 178?

Section 6.1.2


This section says NOOP can be used to reset the autologout timer (which
seems definite), but Section 5.1.4 says any command only SHOULD reset the
timer.  Do these line up?

Section 6.2

s/alternate/alternative/

Section 6.2.3


I think there’s an “or” missing in the last paragraph.

Section 6.3.1

There’s another “X” reference here (BCP 178).

This section should make it more explicit, up front, that the response
names the extensions that were successfully enabled.  Right now the lede is
buried in about the 7th paragraph.

Section 6.3.2

Where is “Paragraph 10”?

Section 6.3.4

The all-caps “UNLESS” should probably be lowercase-ized.

Section 6.3.5

Same remark about “UNLESS”.

Section 6.3.6

Same remark about “UNLESS”.

It’s weird to say “renaming INBOX is permitted” followed immediately by
“some servers don’t let you do this”.  Which is the standard for this
version of IMAP?  If it’s an implementation choice, I think that’s what
this should say.

Section 6.3.9

   The LIST command returns a subset of names from the complete set of

   all names available to the client.

Names of what?

   Clients SHOULD use the empty reference

   argument.

Why might I not do this?

    For example, here are some examples …

You can probably drop “For example,”.

Section 6.3.9.2

This section (and the previous) require that \Subscribed be computed
accurately before the response is generated.  How does this square with the
text in Section 6.3.9 that insists replies come quickly?

Section 6.3.9.5


A grammar nit, which I encountered here but it’s not the first: “whether or
not” should usually just be “whether”; the “or not” is implied.

The second paragraph also gets into the presentation of the reply.  This is
human factors stuff the IETF tends to avoid.  Are we sure it belongs here?

Section 6.3.9.6

    Servers SHOULD ONLY return …

I think “ONLY” should be lowercase here (confusion with BCP 14).

Section 6.3.9.7

   another user renaming of deleting the

s/of/or/

Section 6.3.9.8

Another grammar nit: I keep running into sentences or clauses that end with
“subscribed to”, which should really be “to which the user is subscribed”,
or maybe just “subscribed”.

Sadly I must also point out that in example 8, contrary to my earlier
point, this use of “whether or not” is correct as-is.  But the prose later
on in example 8 refers to “subscribed children”, which is a good example of
avoiding the problem I just mentioned above.  (Whee, English.)

Section 6.3.10

   The NAMESPACE command causes a single ungagged NAMESPACE …

You probably meant “untagged”, unless NAMESPACE is generally noisy.

  In this example, a server supports 2 Personal Namespaces.  …

s/2/two/

There’s a reference to X-PARAM in here that probably needs updating in
light of BCP 178.

      refer to sections Section 7
<https://tools.ietf.org/html/draft-ietf-extra-imap4rev2-22#section-7>,

      Section 7.3.1
<https://tools.ietf.org/html/draft-ietf-extra-imap4rev2-22#section-7.3.1>
for more information about …

Remove “sections” and stick an “and” between them.

Section 6.3.12

I’m not sure that 310 is the correct length for the first example.  I get
312.  Am I misunderstanding how this gets determined?  I get this by adding
the length of each line (including the empty ones) plus two for each line
representing the terminating CRLF.

Using that method, the second example also appears to be off by two.

Section 6.3.13

  Without the IDLE command a client requires to poll the server for

s/requires/needs/ (or maybe “would need”)

  responses at any time.  If the server choose to send unsolicited

s/choose/chooses/

The specific advice about reissuing IDLE every 29 minutes presumes the
auto-logout timeout is fixed at 30 minutes.  Since I think I read higher up
that this could be some other time limit, it might be a good thing to
negotiate somehow via a capability, or the continuation reply to IDLE, or
something like that.

Section 6.4.2

   The UNSELECT command frees server's resources associated with the

   selected mailbox and returns the server to the authenticated state.

I suggest changing “server’s” to “session’s”.  As written here, it could be
read to say you’re deleting the selected mailbox.

Section 6.4.4

The use of “(*)” to make a footnote that’s actually just the next paragraph
can probably be eliminated by just merging the paragraphs.


OLD:

  Individual options may contain parameters enclosed in

   parentheses (*).  If an option has parameters, they consist of atoms

   and/or strings and/or lists in a specific order.  Any options not

   defined by extensions that the server supports must be rejected with

   a BAD response.

   (*) - if an option has a mandatory parameter, which can always be

   represented as a number or a sequence-set, the option parameter does

   not need the enclosing ().  See the ABNF for more details.

NEW:

Individual options may contain parameters enclosed in parentheses.
(However, if an option has a mandatory parameter, which can always be
represented as a number or a sequence-set, the option parameter does not
need the enclosing parentheses.  See the ABNF for more details).  If an
option has parameters, they consist of atoms and/or strings and/or lists in
a specific order.  Any options not defined by extensions that the server
supports MUST be rejected with a BAD response.


(END)

In “Return number of the messages”, I think it should be “Return the number
of messages”.

The definitions of several search keys should be reviewed to ensure use of
the terms “header field” (not simply “header”) and “header field value”
appropriately, matching current convention.

Does TEXT matching ignore line breaks?  For instance, does searching for
“foo bar” match “foo<newline>bar”?  Given the text about flexible matching,
I was wondering.

Section 6.4.4.2

“as described in by Section 5.5
<https://tools.ietf.org/html/draft-ietf-extra-imap4rev2-22#section-5.5>.”
-- s/in by/in/

Section 6.4.4.4

I think the inline comments in the various examples don’t really need to be
inline.  They could just as easily be prose below each example.

Section 6.4.7

There’s a lot of SHOULD and SHOULD NOT here that make me think a discussion
of why one might do otherwise would be helpful.  There are otherwise too
many different ways in which this command could legitimately behave.

Section 6.5

Is this appropriate in light of BCP 178?

Section 7.1

Why would CONTACTADMIN be returned as part of an OK message, such as in the
example given?  I read that as “Your login was successful, but you should
contact your help desk.”

Section 7.2.2

The stuff about capability names starting with “X” should be reviewed in
light of BCP 178.

Section 7.2 and its subsections

A fair bit of this text is repeated verbatim from earlier sections and
should probably be consolidated.  I really noticed this when I hit Section
7.2.3 where, for instance, \HasChildren is defined both here and in Section
6.3.9.5, with the paragraphs in both sections being identical.  However,
now that I look earlier in 7.2.2, I see the same issue.

Section 7.4.2


In the ENVELOPE section, there are references to “header” that should be
“header field”.

Section 13.1


RFCs 2152 and 2180 are normative downrefs; please make sure the shepherd
writeup calls this out.

-MSK