Re: [Jmap] Benjamin Kaduk's Discuss on draft-ietf-jmap-mail-15: (with DISCUSS and COMMENT)

[Benjamin Kaduk <[e-mail]>]

On Thu, Mar 07, 2019 at 12:48:53AM -0500, Neil Jenkins wrote:
> Hi Benjamin,
> 
> Thanks for the detailed feedback again.
> 
> > ----------------------------------------------------------------------
> > DISCUSS:
> > ----------------------------------------------------------------------
> > 
> > Section 2
> > 
> > I think we need more precise language than "corresponds to" for the
> > relationship between JMAP MailboxRights and IMAP ACLs, specifically
> > because JMAP distinguishes mayRename and mayDelete but IMAP just has the
> > single 'x' ACL. (More in the COMMENT section, but the non-isomporphic
> > mapping of 'x' is the only DISCUSS-worthy part.)
> 
> I believe "corresponds to" is generally the right way of phrasing this as we do not want to mandate which underlying data model servers should use for their ACLs (e.g. if JMAP is authoritative you may have `mayRename: true` but `mayDelete: false`; in such a case you would have to not expose the `x` ACL over IMAP. If using IMAP as authoritative, then obviously the `mayRename` and `mayDelete` values would always be the same over JMAP.)
> 
> However, I've updated the spec to make this a little more obvious, so it now reads:
>  * *mayRename*: `Boolean`
> The user may rename the mailbox or make it a child of another mailbox. Corresponds to IMAP ACL `x` (although this covers both rename and delete permissions).
>  * *mayDelete*: `Boolean
> `The user may delete the mailbox itself. Corresponds to IMAP ACL `x` (although this covers both rename and delete permissions).

That's enough to resolve the discuss point (and thanks for the other
changes for going from IMAP to JMAP too)

> > Section 4.1.1
> > 
> > We only describe the "\" to "$" translation for the four supported
> > system keywords, but it seems that it should be more generic (not that
> > we expect more IMAP system keywords to appear anytime soon)?
> 
> We do not expect any more system keywords like this ever. Such a keyword would require an RFC rather than just registering it in the (now shared with JMAP) IANA keywords registry, and at review time would be told to just use a normal keyword and put it in the registry. The handling is not generic, in that 2 of the 6 system keywords are obsoleted and not used in JMAP.

I think the core of my position here is from an abstract process sense, in
that IMAP retains sole change control over the IMAP system keywords, and
JMAP has to respond to them.  So, while such a new keyword would require an
RFC, it would be an IMAP RFC, and there is nothing that formally binds them
to consider JMAP as well.  (I do agree in practice that an attempt to do so
is highly likely to get directed elsewhere, of course.)  So in this formal
sense, IMAP could make a change that would leave JMAP in an inconsistent
state.

That said, since both Alexey and you are fully confident this will not be
an issue in practice, I'm not going to press the point any further.

> > Section 4.7
> > 
> >  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; if the target account already has the email the
> >  copy will be rejected with a standard "alreadyExists" error.
> > 
> > This has some security considerations that should probably be mentioned
> > in Section 9.4: when a user only has read privileges to a
> > subset of the folders in an account, this behavior can be abused as an
> > oracle to determine whether a given message exists in the inaccessible
> > portions of that account. (Similarly for /import.)
> 
> Agreed, this should be documented. I have added to the "Partial account access" security consideration:
> 
> *If the server forbids a single account from having two identical messages, or two messages with the same `Message-Id` header field, a user with write access can use the error returned trying to create/import such a message to detect whether it already exists in an inaccessible portion of the account.*

Looks good.  (And I also appreciate the loss of "any" from "any
unauthorized information".)

> > Section 4.9
> > 
> >  The following metadata properties on the Email objects will be "null"
> >  if requested:
> >  [...]
> >  o mailboxIds
> > 
> > This seems in conflict with the Section 4.1.1 text that every Email "MUST
> > belong to one or more mailboxes at all times (until it is deleted)."
> > Presumably we want a broader disclaimer in 4.1.1 rather than any changes
> > here...
> 
> Yes. This is one of those things that feels "obvious" but is hard to word! I've changed:
> 
> *An email MUST belong to one or more mailboxes at all times…*
> 
> to:
> 
> *An email in the mail store MUST belong to one or more mailboxes at all times*

That sounds great, thank you.

> > There may also be a related condition wherein an EmailSubmission object
> > refers to an Email after the Email is deleted -- I didn't (yet) see text
> > to indicate whether the emailId in the EmailSubmission is expected to
> > still be resolvable, in which case there would potentially not be an
> > associated Mailbox.
> 
> The email referenced by the EmailSubmission may be deleted, in which case the emailId property of the EmailSubmission will not resolve (and return a `notFound` error if you try to fetch them). I have clarified this in the EmailSubmission/set description which already talks about this being deleted.

Thanks.

> > ----------------------------------------------------------------------
> > COMMENT:
> > ----------------------------------------------------------------------
> > 
> > Section 1.3.1
> > 
> >  o *maxMailboxesPerEmail*: "UnsignedInt|null" The maximum number of
> >  mailboxes that can be can assigned to a single email. [...]
> > 
> > nit: My understanding (shaped solely by experience and inference) is that
> > "email" is generally used to refer to a message, whereas "email account"
> > or "email address" would be used to refer to or associate with a thing
> > that can be a container for folders.
> 
> This *is* talking about a message. I will clarify it to say "Email object" and reference section 4.

Ah!  That's also a fine thing to have a quota on, but I was misreading it.

> >  o *maxSizeMailboxName*: "UnsignedInt" The maximum length, in (UTF-8)
> >  octets, allowed for the name of a mailbox. This MUST be at least
> >  100, although it is recommended servers allow more.
> > 
> > The Unicode normalization form used by client/server could cause
> > disagreement about whether a given name is permitted, though for the
> > type of use this will get it's not clear that we need to be more
> > rigorous.
> 
> I think this is sufficient for the use case it will get. It's an indication the client should not send a name that (in the client's normalisation) is longer than this length. It does not guarantee the server will accept the name; as stated in the relevant section, the server may always apply further policy restrictions on naming and reject it.
> 
> > Section 1.3.2
> > 
> >  o *submissionExtensions*: "String[String[]]" A JMAP implementation
> >  that talks to a Submission [RFC6409] server SHOULD have a
> >  configuration setting that allows an administrator to expose a new
> >  submission EHLO capability in this field. This allows a JMAP
> > 
> > I think I'm confused by the workflow here. Suppose we have a JMAP
> > client A talking to a JMAP server B, and B is also an SMTP client that
> > talks to MTA C. This capability is supposed to be about letting B
> > expose a new EHLO capability to A, but only when C supports it?
> 
> Yes you're right, it's about B being able to easily expose new capabilities that C acquires to A without needing code changes (and without needing a new JMAP-specific specification document for the SMTP extension).
> 
> > We probably need some more text here to explain the scenario, even if my
> > guess is correct.
> 
> OK, I've tried to clarify this:
> 
> *A JMAP implementation that talks to a Submission [@!RFC6409] server SHOULD have a configuration setting that allows an administrator to modify the set of submission EHLO capabilities it may expose on this property. This allows a JMAP server to easily add access to a new submission extension without code changes. 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 (for example PIPELINING, CHUNKING, or STARTTLS).*

That's getting the right point across, I think.  I might want to qualify
the "without code changes" with some note about how that depends on the
capability being something that the JMAP layer can just pass through, but
maybe that's obvious.

> >  server to gain access to a new submission extension without code
> >  changes. 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 (for example PIPELINING,
> >  CHUNKING, or STARTTLS). Each key in the object is the _ehlo-
> >  name_, and the value is a list of _ehlo-args_. Examples of
> >  Submission extensions to include:
> > 
> > The description here could probably be reworded/reorderd for clarity,
> > since we don't start talking about what the actual map keys/values are
> > until the penultimate sentence, at present.
> 
> Agreed. I have added a description of what the property represents and how it is formed to the top:
> 
> *The set of SMTP submission extensions supported by the server, which the client may use when creating an EmailSubmission object (see section 7). Each key in the object is the *ehlo-name*, and the value is a list of *ehlo-args*.*
> 
> The extra discussion then follows from this.
> 
> > Section 1.5
> > 
> >  In addition, servers MUST support pushing state changes for a type
> >  called "EmailDelivery". [...]
> > 
> > Er, is this only servers that are implementing urn:ietf:params:jmap:mail?
> 
> Yes, it should be. I've clarified this.
> 
> > Section 2
> > 
> >  o *id*: "Id" (immutable; server-set) The id of the mailbox.
> > 
> > Since jmap-core now has all Ids as immutable and server-set, is this
> > sort of notation considered redundant in jmap-mail? (Throughout; this
> > is just the first instance.)
> 
> I think it is helpful to reiterate these attributes for consistency with the other property definitions.

Makes sense.

> >  o *sortOrder*: "UnsignedInt" (default: 0) Defines the sort order of
> >  [...]
> >  equal order SHOULD be sorted in alphabetical order by name. The
> >  sorting SHOULD take into account locale-specific character order
> >  convention.
> > 
> > I think this last SHOULD is probably not a 2119 SHOULD, and therefore
> > should just be "should".
> 
> Agreed, done.
> 
> >  o *unreadThreads*: "UnsignedInt" (server-set) An indication of the
> >  number of "unread" threads in the mailbox. For compatibility with
> >  existing implementations, the way "unread threads" is determined
> >  is not mandated in this document. The simplest solution to
> > 
> > Do we really have competing *J*MAP implementations that disagree on
> > this? Or is the idea to preserve compatibility with IMAP
> > clients or other current implementations that attempt to determine
> > threading relationships (in which case, that should probably be mentioned)?
> 
> Not JMAP implementations, but the same functionality has already been implemented for proprietary APIs in at least the two documented forms:
>  * Gmail currently does the "easy" option of just number of unique threads with unread messages in that mailbox.
>  * FastMail currently does the "right" option of counting the thread as unread even if the unread message is in another mailbox (so the number you see in the sidebar corresponds to the number of unread threads you see if you open it).
> It seemed more likely that servers would ignore specific requirements than change to match, hence this text.

Oh, definitely servers will ignore anything we try to mandate.  I'm just
wondering if the text could be more clear about what exactly we're
preserving compatibility with.  (And "no change is a perfectly acceptable
outcome.)

> >  o *myRights*: "MailboxRights" (server-set) The set of rights (ACLs)
> >  the user has in relation to this mailbox. These are backwards
> >  compatible with IMAP ACLs, as defined in [RFC4314]. A
> >  _MailboxRights_ object has the following properties:
> > 
> > I read this as saying that all of the properties must be present in the
> > object, such that omitting a property is not a permitted synonym for it
> > having a value of false. Is this reading correct?
> 
> Yes.

I've forgotten the general style of the doc at this point, but would
"mandatory properties" make sense as a way to emphasize this?

> > If so, what should a client do if the server misbehaves?
> 
> Well, that's undefined really. The server could misbehave in any number of ways, and we can't specify how the client should handle every case. In this particular instance I would probably expect implementations in weakly typed languages to treat absence the same as false and strongly typed implementations to reject the Mailbox object on load from the server as invalid. But that's just the emergent behaviour I would consider most likely, not a requirement.

Okay.

> >  * *mayReadItems*: "Boolean" If true, the user may use this
> >  [...]
> >  but not the parent mailbox, this may be "false". Corresponds
> >  to IMAP ACLs "lr".
> > 
> > "Corresponds to" is perhaps imprecise, if one is thinking about the IMAP
> > ACL as being the authoritative source of information (which not all
> > readers will!). The point being that just 'l' or just 'r' would not be
> > enough to get this, and since JMAP is specifying a slightly more
> > abstract mapping than standard IMAP rights, we should be precise about
> > the mapping, and arguably in both directions. (Not just here, but for
> > all the compound ACLs, of course. Also for IMAP ACL 'x', which has two
> > corresponding JMAP permissions.)
> 
> For mayReadItems and mayRemoveItems (which map to a combination of IMAP ACLs due to difference in API design) I have added a further clarification, like:
> 
> *Corresponds to IMAP ACLs `lr` (if mapping from IMAP, both are required for this to be `true`).*
> 
> >  * *maySetSeen*: "Boolean" The user may add or remove the "$seen"
> >  keyword to/from an email. If an email belongs to multiple
> >  mailboxes, the user may only modify "$seen" if *all* of the
> >  mailboxes have this permission. Corresponds to IMAP ACL "s".
> > 
> > nit: in the intro, we talk of "rights the user has in relation to this
> > mailbox", so it's a bit disjoint to talk of mailboxes having permissons.
> > (Here and below.)
> 
> Yes, I have rewritten to:
> 
> *If an email belongs to multiple mailboxes, the user may only modify *`*$seen*`* if they have this permission for ***all*** of the mailboxes.*
> 
> >  o *isSubscribed*: "Boolean" Has the user indicated they wish to see
> >  [...]
> >  choose to ignore this property, either entirely for ease of
> >  implementation, or just for the primary account (which is normally
> > 
> > nit: We don't really provide a formal definition of "primary account"
> > either here or in jmap-core.
> 
> Yes, this was a concept in an earlier draft that got removed. I have now updated this to read:

Ah, makes sense.

> *However, clients MAY choose to ignore this property, either entirely for ease of implementation, or just for an account where `isPersonal` is `true` (indicating it is the user's own, rather than a shared account).*
> 
> > Section 2.1
> > 
> >  Standard "/get" method. The _ids_ argument may be "null" to fetch
> > 
> > I thought I had said something on jmap-core but maybe I only thought
> > about it: my personal preference would be for section references into
> > jmap-core to provide a foundation for what the "standard method" is,
> > though I will not insist upon it.
> >  all at once.
> 
> I've added section references, so instead of just saying *Standard "/get" method* it now says:
> 
> *Standard "/get" method as described in [@!I-D.ietf-jmap-core] section 5.1.*
> 
> (and similar where the same pattern is used elsewhere).

Thank you.  I know it feels tedious and repetitive, but it helps out people
who are just looking up a single aspect without wanting to read the whole
document set.

> > Section 2.2
> > 
> > I might say explicitly that "a non-null updatedProperties response
> > argument indicates that the mailbox contents are unchanged from the old
> > state, with only the counts having changed", since these semantics are
> > potentially unexpected.
> 
> It's not the mailbox contents, it's the properties on the mailbox object. (I would presume mailbox contents refers to the messages in the mailbox, which clearly have changed if the counts have changed). I think the current text here is OK.
> 
> > Section 2.3
> > 
> >  Standard "/query" method, but with the following additional argument:
> > 
> > (Generic comment, not scoped to just this section): are we going to need
> > to draw a distinction between input and ouput arguments for any of the
> > additions we make to standard methods?
> 
> Hmm, I'll reword this to "additional request argument" just to be sure this is clear.
> 
> >  o *role*: "String|null" The Mailbox _role_ property must match the
> >  given value exactly.
> > 
> > So the client is responsible for lower-casing values from the IMAP
> > Mailbox Name Attributes registry and the server must not do a
> > case-insensitive comparison?
> 
> Yes.
> 
> >  The following properties MUST be supported for sorting:
> > 
> > Just to be pedantic, we're talking about the values of the "property"
> > property within the Comparator object, right? (This is one of those
> > annoying things at the intersection of technical specifications and
> > natural English language.)
> 
> Yes, I've tried to clarify this.
> 
> > Section 4
> > 
> >  Due to the number of properties involved, the set of _Email_
> >  properties is specified over the following three sub-sections.
> > 
> > It's probably worth a note that the subsections are for purposes of
> > document organization and are not reflected in the wire protocol
> > structure -- the properties involved are all top-level peers, across the
> > three subsections. (Assuming that's correct, of course.)
> 
> That is correct, I'll add a note.
> 
> > Section 4.1
> > 
> > I assume it was a conscious WG decision to not present a full
> > consolidated schema for Email. But I want to check, since I think (not
> > having one) that it would have helped my understanding, and I try to be
> > open to discovering the errors of my ways...
> > 
> >  o _textBody_/_htmlBody_: These provide a list of parts that should
> >  be rendered sequentially as the "body" of the message. This is a
> >  list rather than a single part as messages may have headers and/or
> >  footers appended/prepended as separate parts as they are
> >  transmitted, and some clients send text and images intended to be
> >  displayed inline in the body (or even videos and sound clips) as
> >  multiple parts rather than a single HTML part with referenced
> >  images.
> > 
> > Some guidance related to interpreting these lists and avoiding the eFail
> > (efail.de) class of attacks is probably in order -- the HTML parts
> > should get some different containers around their processing. This
> > could potentially go here, or in Section 9.2.
> 
> Yes, I have added a new subsection to the security considerations:
> 
> *Messages may consist of multiple parts to be displayed sequentially as a body. Clients MUST render each part in isolation and MUST NOT concatenate the raw text values to render. Doing so may change the overall semantics of the message. If the client or server is decrypting a PGP or S/MIME encrypted part, concatenating with other parts may leak the decrypted text to an attacker, as described in [EFAIL]* (reference to paper added).

Thank you, that's a nice summary.

> >  Because MIME allows for multiple representations of the same data
> >  (using "multipart/alternative"), there is a textBody property (which
> >  prefers a plain text representation) and an htmlBody property (which
> >  prefers an HTML representation) to accommodate the two most common
> >  client requirements. The same part may appear in both lists where
> >  there is no alternative between the two.
> > 
> > (soapbox) It's annoying when I get mime/multipart with HTML in the
> > text/plain section. Is this clause going to allow for that same sort of
> > misclassification?
> 
> If the sender puts HTML in a part marked `text/plain` then, well, the sender is broken and there's not much we can do. (Clients can attempt to sniff the content and treat as `text/html` if they want but of course there's always the slim chance that someone really did want to send a bunch of HTML code as plain text).
> 
> This is an independent issue to the htmlBody/textBody properties though. These are simply a list of part ids, and those parts have a content type which the client needs to use to determine how to render it. There is a possibility that a client doesn't read the spec and presumes it will always get plain text in the textBody array, but then again the client could ignore any part of the spec and start doing silly things. Such a mistake is likely to become obvious and be fixed at some point.

Okay, this makes sense now, whether just on a third reading or having you
explain it.

> >  Due to the number of properties involved, the set of _Email_
> >  properties is specified over the following three sub-sections.
> > 
> > nit: are we at four sub-sections now?
> 
> Yes, I'll update that.
> 
> > Section 4.1.1
> > 
> >  The IMAP "\Recent" keyword is not exposed via JMAP. The IMAP
> >  "\Deleted" keyword is also not present: IMAP uses a delete+expunge
> >  model, which JMAP does not. Any message with the "\Deleted"
> >  keyword MUST NOT be visible via JMAP (including as part of any
> >  mailbox counts). Users may add arbitrary keywords to an email.
> > 
> > IIRC, Trash gets special handling with respect to deletion in JMAP
> > commands; does it also get special treatment w.r.t \Deleted translation
> > from IMAP to JMAP?
> 
> I'm not quite sure what you're asking here. As stated in this text, if supporting IMAP and JMAP on the same server, the server MUST treat \Deleted messages the same as if they were expunged when accessing via JMAP.

It looks like I was confused, sorry.  (Maybe something about message counts
in Trash?  But even that doesn't make much sense.)

> > Section 4.1.2.2
> > 
> > This escape valve for "Any header not defined in [RFC5322] or [RFC2369]"
> > (here, et seq) seems like it might benefit from some general guidance
> > about implementations applying common sense, i.e., allowing servers the
> > ability to deny requests for a given form for such new headers in order
> > to prevent nonsense behavior.
> 
> A bit torn on this. It's not an interoperable definition but at the same time it is perhaps reasonable to change this to:
> 
> *Any header not defined in [@!RFC5322] or [@!RFC2369], except where the server has determined this is obviously nonsense.*
> 
> I'm not convinced it's worth it overall; I'll leave it for now.

That's a fine stance to take; thanks for thinking about it.

> > Section 4.1.4
> > 
> >  o *partId*: "String|null" Identifies this part uniquely within the
> >  Email. This is scoped to the _emailId_ and has no meaning outside
> >  of the JMAP Email object representation. This is "null" if, and
> >  only if, the part is of type "multipart/*".
> > 
> > The prose isn't the super-best indicator that the asterisk is
> > intended to have wildcarding behavior.
> 
> I've added to the introduction:
> 
> *In the following subsections the common notational convention for wildcards has been adopted for content types, so `foo/*` means any content type that starts with `foo/`.*
> 
> > Section 4.4.1
> > 
> > Just to double-check: the different fencepost behavior for
> > minSize/maxSize is intentional?
> 
> Yes. Ranges, and properties defining ranges, are intended to match the behaviour most prevalent in common programming languages today: inclusive start, exclusive end. This is consistently applied across the spec (e.g. vacation response from/to).

I noticed it in several places, yes, and happy to hear that it's
intentional.

> >  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 be converted to text by the server. The server MAY
> >  extend the search to any additional textual property.
> > 
> > side note: as a mail user, I like to be able to use text search to
> > conclusively determine that a given message/topic is *not* in a given
> > mailbox. This weak "SHOULD" language does not provide me that
> > guarantee, though I can see how it makes sense in the protocol design to
> > leave the flexibility for implementors, here.
> 
> Hmm, I think it's safe enough to change this to something a little stronger:
> 
> *Looks for the text in emails. The server MUST look up text in the *from*, *to*, *cc*, *bcc*, *subject* header fields of the message, and SHOULD look inside any `text/*` or other body parts that may be converted to text by the server. The server MAY extend the search to any additional textual property.*

Thanks!

> >  o When searching inside a "text/html" body part, any text considered
> >  markup rather than content SHOULD be ignored, including HTML tags
> >  and most attributes, anything inside the "<head>" tag, CSS and
> >  JavaScript. Attribute content intended for presentation to the
> >  user such as "alt" and "title" SHOULD be considered in the search.
> > 
> > This would seem to leave no reliable way for a security researcher to
> > (e.g.) search for snippets of attack javascript in received mails
> > without downloading all of them.
> 
> Perhaps, and maybe this would be a worthy extension. But the default behaviour should absolutely be as described to correspond with general user expectation.
> 
> >  o Text SHOULD be matched in a case-insensitive manner.
> > 
> > Is the server going to have a sense of the user's locale as needed for
> > fully generic case-insensitive comparison?
> 
> This will be vendor dependent, but often yes. (For example at FastMail we try to determine the prevalent language in a message in order to use the appropriate stemming algorithm for indexing it for search; the same information could be used for determining what case-sensitivity method to use).

This is just a SHOULD, so there's not really a strict need to say more, but
I can imagine some people reading this and thinking "strcasecmp()", when
the reality is a lot more complicated.  I guess maybe we could reference
PRECIS stringprep or something like that if we wanted to, but your call.

> >  o Tokens MAY be matched on a whole-word basis using stemming (so for
> >  example a text search for "bus" would match "buses" but not
> >  "business").
> > 
> > I think this is supposed to not apply to the "phrase search" two bullets
> > above, but greater clarity would be appreciated.
> 
> OK, I'll amend to:
> 
> *Tokens (not part of a phrase) MAY be …*

Thanks!

> >  o *hasKeyword* - This value MUST be considered "true" if the email
> >  has the keyword given as an additional _keyword_ property on the
> >  _Comparator_ object, or "false" otherwise.
> > 
> > I strongly suggest an explicit listing of the "additional properties as
> > reuqired for specific sort operations" of the _Comparator_ type when
> > used for Emails.
> 
> OK, I've added the following to the introductory paragraph above the list:
> 
> *The following values for the "property" field on the Comparator object SHOULD be supported for sorting. When specifying a "hasKeyword", "allInThreadHaveKeyword" or "someInThreadHaveKeyword" sort, the Comparator object MUST also have a keyword property.*

Thanks.

> > Section 4.6
> > 
> >  When emptying the trash, clients SHOULD NOT destroy emails which are
> >  also in a mailbox other than trash. For those emails, they SHOULD
> >  just remove the Trash mailbox from the email.
> > 
> > This last SHOULD seems to be duplicated in Section 2.
> 
> Thanks, I've fixed this duplication.
> 
> >  For successfully created Email objects, the _created_ response
> >  contains the _id_, _blobId_, _threadId_ and _size_ properties of the
> >  object.
> > 
> > For partial drafts, the behavior where a threadId always gets assigned
> > on first touch is perhaps interesting, as subsequent edits might cause
> > the effective threading to change, which would necessitate a new Id as
> > well (IIUC). I'm not sure if there's anything weird there that a client
> > would need to be prepared for, though. (Maybe not, given that it always
> > is going to get back an _id_ from /set, and should be using that.)
> 
> Yes, email content is immutable, so to update a draft you need to create a new email and destroy the old one. This means you will always get a new id for the message (and possibly a different thread id to before too).

Thanks for confirming my understanding.

> > Section 4.8
> > 
> >  If the blob referenced is not a valid [RFC5322] message, the server
> >  MAY modify the message to fix errors (such as removing NUL octets or
> >  fixing invalid headers). If it does this, the _blobId_ on the
> >  response MUST represent the new representation and therefore be
> >  different to the _blobId_ on the EmailImport object. Alternatively,
> >  the server MAY reject the import with an "invalidEmail" SetError.
> > 
> > In general, having more options like this can increase the fragility of
> > the ecosystem, as a client might be written to assume one behvaior and
> > then be not as portable to a different server. I'm not sure that
> > there's a clear way to mandate a single type of behavior here, but want
> > to be sure that the topic was discussed.
> 
> Yes, it was discussed. Both behaviours are reasonable and exist in equivalent IMAP implementations, so this seemed the best solution for the JMAP spec.

WFM.

> > Section 6
> > 
> >  o *email*: "String" (immutable) The "From" email address the client
> >  MUST use when creating a new message from this identity. The
> >  value MAY alternatively be of the form "*@example.com", in which
> >  case the client may use any valid email address ending in
> >  "@example.com".
> > 
> > I mostly assume this is supposed to be for a generic domain and not
> > special-casing example.com literally. Some extra text/formatting would
> > help with that.
> 
> Your interpretation is correct. I've rewritten this to:
> 
> *If the mailbox part of the address (the section before the "@") is the single character `*` (e.g. `*@example.com`) then the client may use any valid address ending in that domain (e.g. **`[e-mail]`**).*

Looks good (I had failed to come up with a good way to do this, hence the
lack of suggestion).

> > Section 7.3
> > 
> > The associated identityId is not a queriable property?
> 
> Yeh, it probably should be for consistency. I'll add this.
> 
> > Section 7.5
> > 
> >  o *onSuccessUpdateEmail*: "Id[Email]|null" A map of _EmailSubmission
> >  id_ to an object containing properties to update on the Email
> >  object referenced by the EmailSubmission if the create/update/
> >  destroy succeeds. (For references to EmailSubmission creations,
> >  this is equivalent to a back-reference so the id will be the
> >  creation id prefixed with a "#".)
> > 
> > I'm confused by the "Id[Email]" part -- we describe it as a map to "an
> > object containing properties to update", but that's not exactly what an
> > Email object is. Is this more of a PatchObject than an Email per se?
> 
> Yes, this should be `Id[PatchObject]`; I've updated this. (A patch object to update an Email of course, but that's clear from the description).
> 
> > nit: I'd also tweak the wording of the parenthetical a bit (here and
> > below), to something like "when applying to EmailSubmissions created in
> > the same "/set" invocation, ..."
> 
> Thanks, that's clearer, I'll update.
> 
> > Section 8
> > 
> > By a literal reading, fromDate and toDate are in conflict with each
> > other (when non-null). That is, the fromDate text does not admit the
> > possibility of an end to the vacation response period, and vice versa.
> 
> OK, I'll update the definitions to clarify.
> 
> > Section 9
> > 
> > I'd consider adding another sentence like "Additional considerations
> > specific to the data types and functionality introduced by this document
> > are described in the following subsections."
> 
> Sure, done.
> 
> > Section 9.3
> > 
> > I know we don't want this to devolve into a generic discussion of the
> > flaws of email, but perhaps the envelope-from/body-from distinction is
> > worth repeating, with a note that JMAP has provisions for ACLs on
> > submission that check both.
> 
> I have added the following section to the security considerations
> 
> **Permission to send from an address***
> *
> *
> *
> *The email ecosystem has in recent years moved towards associating trust with the From address in the [@!RFC5322] message, particularly with schemes such as DMARC ([@?RFC7489]).**
> *
> *
> *
> *The set of Identity objects (see section 6) in an account lets the client know which email addresses the user has permission to send from. Each email submission is associated with an identity, and servers SHOULD reject submissions where the `From` header field of the email does not correspond to the associated identity.**
> *
> *
> *
> *The server MAY allow an exception to send an exact copy of an existing message received into the mail store to another address (otherwise known as "redirecting" or "bouncing"), although it is RECOMMENDED the server limit this to destinations the user has verified they also control.**
> *
> *
> *
> *If the user attempts to create a new Identity, the server MUST reject it with the appropriate error if the user does not have permission to use that email address to send from.**
> *
> *
> *
> *The [@!RFC5321] SMTP MAIL FROM address is often confused with the [@!RFC5322] message header. The user generally only ever sees the message header address, and this is the primary one to enforce. However the server MUST also enforce appropriate restrictions on the [@!RFC5321] MAIL FROM address to stop the user from flooding a 3rd party address with bounces and non-delivery notices.**
> *
> *
> *
> *The JMAP submission model provides separate errors for impermissible addresses in either context.*

That's quite good.

Thanks again for all the updates (and I've changed to No Objection in the
dattracker), so this should be ready for Alexey to push the button on.

-Benjamin