Subject: [Jmap] AD review of draft-ietf-jmap-mdn-13

From: Barry Leiba <[email-masked]>
Date: Fri, 24 Jul 2020 00:34:23 -0400

Thanks for the good work on this document, and I’m sorry I couldn’t get to
reviewing it more quickly.  Here’s my AD review; please let me know if you
have questions or disagreements with any of it.  I’ll set the state to
“revised I-D needed”, and will request last call after a suitable revision
is posted.

— Abstract —
It’s a small thing, and if you want to leave it as is, it’s OK, but I think
the Abstract is unnecessarily long, and doesn’t need to include background
that you might reasonably want in the Introduction.  I would just make the
entire Abstract this:

NEW
This document specifies a data model for handling Message Disposition
Notifications (MDNs, RFC 8098) in the JSON Meta Application Protocol (JMAP,
RFCs 8620 and 8621).
END

— Section 1 —

   and aims to provide a consistent interface to different
   data types.

It doesn’t aim: it does.  Make it, “and provides...”.

Please expand “MDN” on its first use in the Introduction.

Throughout, when you use “MDN” as a plural, please make it “MDNs”,
reserving “MDN” for the singular.

Throughout, when you use “an email” to mean “an email message”, please use
the longer version instead.  Also look for things such as “an existing sent
mail”, which should be “an existing sent message”, and so on.

      Client might want to display detailed
       information about a received MDN.

You need an article here: “A client” or “The client”.

— Section 1.2 —

   Keywords being case insensitive in IMAP but JSON being case
   sensitive, the "$mdnsent" keyword MUST always be used in lowercase.

I find this awkward; would you mind changing it thus?:

NEW
   Because keywords are case-insensitive in IMAP but case-sensitive
   in JMAP, the "$mdnsent" keyword MUST always be used in lowercase.
END

— Section 1.3 —
You never say explicitly that you’re defining a new capability, and you
probably should.  I would append to the first paragraph, ‘This defines a
new capability, "urn:ietf:params:jmap:mdn".’

— Section 2 —
What are the asterisks in “*MDN*” and “*Disposition*” for?

— Section 2.1 —

      always be a backreference to the creation id

What’s a “backreference”?  Do you mean “a backward reference”?

— Section 2.2 —

   The "forEmailId" property can be null or missing if the
   "originalMessageId" property is missing, not referencing an existing
   email or if the server cannot efficiently calculate the related email
   (for example if several emails get the same "Message-Id" header).

This list isn’t parallel in structure.  Please try this (which also
corrects the “email message” issue):

NEW
   The "forEmailId" property can be null or missing if the
   "originalMessageId" property is missing or does not refer to an
   existing message, or if the server cannot efficiently calculate the
   related message (for example, if several messages get the same
   "Message-Id" header).
END

— Section 3.1 —

             "reportingUA": "linagora.com; OpenPaaS",

           "originalMessageId": "<[email-masked]>"

Please make sure your examples all use example domain names, and not
actual, valid names.

— Section 4.2 —

   The following subsection register one new error code

Make it “registers”.  And, really, why have this one sentence and a
subsection?  Why not just put the subsection right here?

— Section 4.2.1 —

   Description: The message has the "$mdnsent" keyword already set.  The
   client MUST NOT try again to send an MDN for this message.

This contradicts Section 2.1, which says this:

   The client SHOULD NOT issue an MDN/send request if the message has
   the "$mdnsent" keyword set.

— Section 6.2 —
8174 has to be normative, just as 2119 does.

Barry

---

From: "Bron Gondwana" <[email-masked]>
Date: Fri, 24 Jul 2020 14:42:26 +1000

>    the "$mdnsent" keyword set.

The way I read this is "there's a race condition here and your client may not have done an immediate fetch to know if it's already happened, but if it's told that there was a $mdnsent flag then it MUST not keep trying".

So it SHOULD check the keywords in advance (ideally with a recent refresh) but that's not as important as not retrying infinitely.

Bron.

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

---

From: Barry Leiba <[email-masked]>
Date: Fri, 24 Jul 2020 09:51:02 -0400

Thanks for the quick response, Raphael.

> They were also in RFC8620 and RFC8621 when defining objects. I kept them
> for consistency. Should I change that?

No; if it's consistent with the other docs, leave it there, and we'll
see if anyone else (including the RFC Editor) brings it up.

> I agree with Bron on this point. If the client is not aware of the
> keyword, it may issue a new request. When it is aware the keyword is
> set, it must not try to.

I didn't see a response from Bron; perhaps he dropped me off the
distribution list.

I think this is an important inconsistency that needs to be fixed, as
these, as worded, are directly in conflict.

If there's a nuance here, the nuance needs to be made clearer by
rewording one or the other, or both.

Barry