Re: [Jmap] AD review of draft-ietf-jmap-mail-12

["Neil Jenkins" <[e-mail]>]

On Thu, 10 Jan 2019, at 4:34 AM, Alexey Melnikov wrote:
> The document is not clear that RFC XXXX is draft-ietf-jmap-core-XX

This is now referenced properly.
 
>  By default, the JMAP server should hide EHLO
>  capabilities that are to do with the transport mechanism and thus
>  are only relevant to the JMAP server
> 
> Is it worth having a registry for these a la Section 6.2 of RFC 8494?

I don't feel a need for one strongly, but we can add one if you think it's necessary?

>  In addition, servers MUST support a psuedo-type called
>  "EmailDelivery" in the push mechanisms. 
> 
> I think an example or pointer to an example would be useful here.

I've added the following example to the document; does this clarify it for you?

*The client has registered for push notifications just for the `EmailDelivery` type (see [@!I-D.ietf-jmap-core] for details of the push system). The user marks an email as read on another device, causing the state string for the `Email` type to change, however as nothing new was added to the store the `EmailDelivery` state does not change and nothing is pushed to the client. A new message arrives in the user's inbox, again causing the `Email` state to change. This time the `EmailDelivery` state also changes, and a StateChange object is pushed to the client with the new state string. The client may then resync to fetch the new message immediately.*

>  In Section 2:
> 
> > Servers MUST forbid sibling Mailboxes with the same name.
> What exactly is this trying to say?

I have rewritten this to say:

*There MUST NOT be two mailboxes with both the same parent and the same name.*

Is that clear? (IMAP is path based so naturally you cannot have two sibling mailboxes with the same name; but JMAP is id based so in theory you could. For IMAP compatibility, and to avoid user confusion, this is forbidden by fiat.)

>  RFC 4314 (IMAP ACL) needs to be referenced at least Informatevely (due 
> to Myrights command reference).

As Bron mentioned, this has already done following the secdir core review.

> IMAP4rev1 (RFC 3501) should be an Informative reference (due to 
> referencing the following terms: subscriptions, internal date).

Added.

> > A server MAY use heuristics to
> > determine a charset and decode the octets, or MAY replace any octet
> > or octet run with the high bit set that violates UTF-8 syntax with
> > the unicode replacement character (U+FFFD).
> 
> Do we really want to encourage the "MAY use heuristics" part? Such email 
> messages are non compliant and thus not in scope for this document!

Well, they appear in the real world so servers do need to deal with them. Using heuristics to determine what the sender intended often provides the best user experience (customers tend not to care that it was the sender that sent something broken…). However, I'm not strongly attached. Would you prefer me to replace this with the following?

*A server SHOULD replace any octet or octet run with the high bit set that violates UTF-8 syntax with the unicode replacement character (U+FFFD).**
*

> So RAW will typically have the leading space, as most generated messages 
> have a space after ":" that terminates the header field name. Is it 
> worth pointing this out to implementors?

Doesn't hurt. I have added:

*This form will typically have a leading space, as most generated messages**
*
*insert a space after the colon that terminates the header field name.*

> >5. Any [RFC6532] UTF-8 values decoded.
> Decoded from what? They are already in UTF-8!

The idea was that your implementation has a unicode string implementation that may not be UTF-8; that's just one possible encoding. You are decoding everything into your internal string format, normalising the unicode (next step) and then it will be encoded as UTF-8 when output as JSON as this is the encoding used for that.

However, I have now just removed this step as I think it just confuses the matter and the step is obvious if required.

> 
>  o Comment
> 
> RFC 5322 defines "Comments" header field (note that it is plural).

Thanks, fixed.

> Also, what about "Keywords" header field?

Sure, I've added that.

> Is it worth explicitly mentioning Content-Description header field here 
> as well?

The spec currently only has restrictions on RFC5322 and RFC2369 headers. Other headers such as this one are not explicitly forbidden so may be used with the form already. So I don't think it's necessary to mention explicitly.

> In Section 4.1.2.3:
> 
> "Any [RFC6532] UTF-8 values MUST be decoded." -- again, decoded from what?

Same as above; I have now removed.

> Is RFC 2231 encoding allowed in this and the following section?

I don't think so, as To, Cc etc. don't have parameter values. Or have I got that wrong?

> 
> In 4.1.3:
> 
> asText and asAddresses is not defined in the document. Did you mean:

This is defined at the top of page 26 in 4.1.3:

o  *:as{header-form}* This means the value is in a parsed form, where
      "{header-form}" is one of the parsed-form names specified above.
      If not given, the value is in _Raw_ form.

> In Section 4.1.4:
> 
> You need to define handling for unrecognised Content-Transfer-Encoding 
> values here.

I have added

*If the transfer encoding is unknown, it is treated as though it had no transfer-encoding.*

> "cid:" needs to Normatively reference RFC 2392. HTML needs a reference 
> as well.

Added.

> In Section 4.2 (and similar text in 4.9):
> 
> maxBodyValueBytes:
> 
>  "The server MUST ensure the truncation results in valid UTF-8 and does 
> not occur mid-codepoint." --
> 
> The document needs to make sure that this is only true for body parts 
> which are known to be textual (e.g. text/plain, text/html, XML, JSON). 
> If a body part is a JPEG image, this requirement doesn't make sense.

As Bron pointed out, bodyValue is only returned like this for text/* types.

> In Sections 4.8/4.9:
> 
> The document needs to explicitly allow EAI messages, not just RFC 5322 
> format.

I'm not entirely sure how to specify this. I have added this sentence to the introduction to each of these methods:

*The server SHOULD support messages with [@!RFC6532] EAI headers.*

Is that sufficient? (And should it be a MUST?)

> In Section 5:
> 
> HTML is now definitely a Normative reference. More details of what is 
> expected in HTML escaping, other than <mark> wrap?

Good, catch this was inadequately specified. I have rewritten it as:

*subject*: `String|null`
If text from the filter matches the subject, this is the subject of the email with the following transformations:
 1. Any instance of the following three characters MUST be replaced by an appropriate HTML entity: & (ampersand), < (less-than sign), and > (greater-than sign). Other characters MAY also be replaced with an HTML entity form.
 2. The matching words/phrases from the filter are wrapped in [@!HTML] `<mark></mark>` tags.
If the subject does not match text from the filter, this property is `null`.

*preview*: `String|null`
If text from the filter matches the plain-text or HTML body, this is the relevant section of the body (converted to plain text if originally HTML), with the same transformations as the *subject* property. It MUST NOT be bigger than 255 octets in size. If the body does not contain a match for the text from the filter, this property is `null`.

> In Section 7:
> 
> Are "MDN" and "DSN" blobIds referencing only the machine parseable part 
> or the whole multipart/report coming back? I think the document should 
> clarify this and (ideally) give some examples.

This is intended to be the whole MIME message a with a top-level content-type of multipart/report; I have added the following line to the two properties to clarify:

*The blob is the whole MIME message (with a top-level content-type of multipart/report), as received.*

> End of Section 7.5:
> 
>  "The server MAY choose to localise this string into the user's 
> preferred language, if known."
> 
> How can this be done?

This is just acknowledging the server may have some kind of out-of-band knowledge about the authenticated user and if so should use it to increase the likelihood of presenting an understandable error to the user. I think anything more is out of scope of the current specification.
 
> In Section 9.3: SMTP XCLIENT should be an Informative reference.

Done.

> In Section 10.4.4 (Registration of JMAP keyword '$answered')
> 
>  Security Considerations: A server implementing this keyword as a
>  shared keyword may disclose that a user considers the message as
>  flagged for urgent/special attention.
> 
> This looks like a cut & paste error from the text specified for $flagged.

Yes, fixed.

Thanks for the review Alexey. All fixes are pushed to the Git repo <https://github.com/jmapio/jmap/issues> for the spec and live on jmap.io <https://jmap.io/spec-mail.html>; I'll cut a new IETF draft once the remaining clarifications have been settled.

Neil.