Subject: [Jmap] Review of draft-ietf-jmap-mail-05

From: Neil Jhaveri <[email-masked]>
Date: Wed, 30 May 2018 23:25:05 -0700

Hi all, 

Here are my comments from reading draft-ietf-jmap-mail-05, which looks great so far! Apologies again if I’m re-hashing anything already extensively discussed.

> 1.1. Notational conventions
> ...
> Types signatures are given for all JSON objects in this document…
Since this document is presumably dependent upon JMAP Core, can we reference it and avoid repeating the same 4 conventions?  Something like "Type signatures are given for all JSON objects in this document. The conventions follow those in Section 1 of [RFC XXXX]”.

> 2. Mailboxes
> …
> Servers SHOULD forbid sibling Mailboxes with the same name.
Should this be be a MUST? I thought this is expressly forbidden in IMAP - RFC 3501 6.3.3. says "It is an error to attempt to create INBOX or a mailbox with a name that refers to an extant mailbox”.

> 2. Mailboxes
> ..
>    o  *sortOrder*: … SHOULD take into account locale-specific character order
>       convention..
There should only be one period after convention, not two.

> 4.1.2. Header fields
> ...
>    o  *Addresses* ("EmailAddress[]") The header is parsed as an "address-list" value, as specified in [RFC5322] section 3.4, into the "EmailAddress[]" type.  The *EmailAddress* object has the following properties:
>       *  *name*: "String|null" The _display-name_ of the [RFC5322] _mailbox_ or _group_, or "null" if none.  [...]
>       *  *email*: "String|null" The _addr-spec_ of the [RFC5322] _mailbox_, or "null" if a _group_.

It looks like Bron & N. Jenkins traded comments about this on the list in February, but the outcome didn’t seem conclusive to me, so I’ll bring this up again.

Is the current definition of EmailAddress object doesn’t seem sufficient to cover an RFC 5322 Group Address? e.g.:
To: A Group:Ed Jones <[email-masked]>,[email-masked],John <[email-masked]>;

I don’t see how this group address can get mapped into EmailAddress[] as defined currently? 

I think perhaps a *groupMembers*: “EmailAddress[]|null” property needs to be added to EmailAddress.

> 4.1.2. Header fields
> …
>    Any syntactically correct [RFC2047] encoded sections with a known encoding MUST be decoded, following the same rules as for the _Text_ form...

It looks to me like the indentation of this entire section is 1 level too low?  The same issue applies to the comment right after the *MessageIds* property definition. Ditto for *URLs*.

> 4.1.2. Header fields   
> ...
> In addition, the client may request/send properties representing individual header fields of the form:
> 
>                         header:{header-field-name}
> 
> Where "{header-field-name}" means any series of one or more printable ASCII characters (i.e. characters that have values between 33 and 126, inclusive), except colon. 

I am not really sure what the conventions are... would this be better expressed with ABNF, or is this textual explanation sufficient?

> 4.1.3. Body parts
> …
>    The following *Email* properties are specified for access to the body
>    data of the message:
> 
>    o  *bodyStructure*: "EmailBodyPart" (immutable) This is the full MIME
>       structure of the message body, including sub parts but not
>       recursing into "message/rfc822" or "message/global" parts.
When I first read this, I was a little confused by the text “including sub parts”, and thought maybe it meant that the tree of parts was flattened into a single array. If others also found this confusing, maybe it could be clarified into something like this:
This is the full MIME structure of the message body, represented as an array of the message’s top-level MIME parts, without recursing into “message/rfc822” or “message/global” parts. Note that EmailBodyParts may have subParts if they are type “multipart/*”.

>    o  *bodyValues*: "String[BodyValue]" (immutable) This is a map of...
Should this be EmailBodyValue instead of BodyValue?

> 4.1.3. Body parts
> ...
> MIME structures are arbitrary nested trees of documents, but the majority of email clients present a model of an email body...
This entire section is a solid in-depth explanation, but it comes after the property definitions, so it read a little bit confusing to me, and felt like the properties were being re-defined. I think it might be worthwhile considering an edit where the high-level explanation of MIME-tree-flattening comes before the list of Email properties, and then in each property, add more detail as to what each one is, so that it reads a bit more linearly and each property definition is a bit more self-contained. Thoughts?

> 4.1.3. Body parts
> ...
>    o  _attachedEmails_/_attachedFiles_: These provide a list of parts
>       that should be presented as "attachments" to the message.  Emails
>       are presented in a separate list so their contents may be easily
>       fetched via a back-reference with the "Email/parse" method in the
>       same request, if the client wishes to. 
Offering attachedEmails separately from attachedFiles comes at the slight trade-off of possibly degrading the user fiction of attachments being an ordered list (as we know it is in many MUAs). For instance, a user could have composed a 4-part multipart/mixed message list this:
	1: text/plain: “…. See my 2nd attachment”
	2: message/rfc822
	3: application/octet-stream
	4: application/octet-stream

If we separate them out into two different properties, it’s equally likely a MUA wishing to treated attachedEmails as file-like attachments will present attachments as [3, 4, 2] or [2, 3, 4], and in the first case, the meaning is distorted.

Is it common in any MUA to have a UI where attached e-mails are immediately displayed inline? Every one I have used displays it as a separate file requiring user interaction to open — so I personally think it may be worthwhile considering consolidating back down to a single attachedFiles property rather than having to have two properties and merge them. Thoughts?

> Pages 27 & 28 - PDF Version - extremely tiny…
The array of all properties does not wrap.

> 4.2. Email/get
> …
>    The following properties are expected to be fast to fetch in a quality implementation:
Resent-XX are commonly shown by 

> 4.4.1. Filtering
> …
>    o  *text*: "String" Looks for the text in emails.  The server SHOULD look up text in the _from_, _to_, _cc_, _bcc_, _subject_ header fields of the message, and inside any "text/*" or other body parts that may converted to text by the server.  The server MAY extend the search to any additional textual property.
I think a “be” is missing between “may” and “converted”

> 4.4.1. Filtering
> …
>    o  *attachments*: "String" Looks for the text in the attachments of the email.  Server MAY handle text extraction when possible for the different kinds of media.

Server should probably be plural?

> 4.7. Email/import
> ...
> The server MAY forbid two email objects with the same exact [RFC5322] content, or even just with the same [RFC5322] Message-ID, to coexist within an account.  In this case, it MUST reject attempts to import an email considered a duplicate with an "alreadyExists" SetError.  
Allowing a rejection purely based on a Message-ID match seems a little bit restrictive to me — it’s author-generated, so I have seen the global-uniqueness requirement violated by naively written scripts. 

Why not just force the rejection to be based on a full RFC5322 content match? Would it be that much more work for a server? Presumably Message-ID and Size are easily accessible metadata, so those can be checked fast, and then if there is a hit, an exact byte comparison can happen (the real added penalty?).

If we do tighten this up, then 4.8 Email/copy has a similar “even just with the same [RFC5322] Message-ID” clause.

> 5. Email Submission
> …
>    o  *identityId*: "String" (immutable) The id of the identity to associate with this submission.
It’s a little bit confusing that Identity is used here before it’s defined in section 6. Maybe a slight note? 

> 5. Email Submission
>   *  *mailFrom*: The email in the _Sender_ header, if present, otherwise the _From_ header, if present, and no parameters.  If multiple addresses are present in one of these headers, or there is more than one _Sender_/_From_ header, the server SHOULD reject the email as invalid but otherwise MUST take the first address in the last _Sender_/_From_ header in the [RFC5322] version of the message.  If the address found from this is not allowed by the identity associated with this submission, the _email_ property from the identity MUST be used instead.
Instead of silently swapping the user’s requested _From_ to the identity address, I'd prefer returning a "forbiddenFrom” error and give the user a chance to correct their mistake. Thoughts?

> 5. Email Submission
> … 
>          For emails relayed via an alternative to SMTP, the server MAY
>          generate a synthetic string representing the status instead.
>          If it does this, the string MUST be of the following form:
> 
>          +  A 3-digit SMTP reply code, as defined in [
> RFC5321], section
>          +  Then an SMTP Enhanced Mail System Status Code as defined in [RFC3463], with a registry defined in [RFC5248].
>          +  Then a single space character.
>          +  Then an implementation-specific information string with a human readable explanation of the response.
Not very familiar with the conventions here, but would ABNF be more appropriate to specify this? 

> 5.5 EmailSubmission/set
>    Standard _/set_ method, with the following two extra arguments:
>    o  *onSuccessUpdateEmail*:...
I think a lot of clients will want to move the message from Draft to Sent using this block. It might be worth explicitly calling that out, or even giving a full example.  Thoughts?

> 7. Search Snippets
> …
>    o  *attachments*: "String|null" If text from the filter matches the text extracted from an attachment, this is the relevant section of the attachment (converted to plain text), with matching words/phrases wrapped in "<mark></mark>" tags.  It MUST NOT be bigger than 255 octets in size.  If it does not match, this is "null".
I could imagine a MUA wanting to present to the user exactly which attachment the search string was found in, so it might be worth including an attachment ID property. Thoughts? 

> 8. Vacation Response
> The *VacationResponse* object represents the state of vacation-response related settings for an account.  It has the following properties:
I’m not too knowledgeable about prior work here, but:
1. Exchange Server offers the ability to configure a separate “Internal" and “External" vacation response — which is useful if you work in a large organization and want to have your internal reply include things like “you can contact my manager at XXX”, but you want your external client-facing response to expose less information.
2. I also think that there may need to be some mention of loop prevention — so that two accounts with vacation responses don’t continuously reply to each other. 

I am starting to think this should be deferred to a JMAP extension, so that prior work in this area can be more carefully considered (such as RFC 5230). Thoughts?

- Neil