Subject: [Jmap] Adam Roach's Discuss on draft-ietf-jmap-core-14: (with DISCUSS and COMMENT)

From: Adam Roach <[email-masked]>
Date: Wed, 20 Feb 2019 22:05:14 -0800

Adam Roach has entered the following ballot position for
draft-ietf-jmap-core-14: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-jmap-core/

----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

Thanks to the authors for a well-laid-out and easy-to-read document. Thanks also
to everyone who contributed to the completion of this work. I have three
DISCUSS-level items, and a handful of other suggestions that are largely
editorial

---------------------------------------------------------------------------

§7.3:

>  The client may add a query parameter called "types", with the value
...
>  The client may add a query parameter called "closeafter" with value
...
>  The client may add a query parameter called "ping", with a positive

This form of specification of URI parameters is not allowed -- it contravenes
the basic thesis of BCP 190; section 2.4 of which stipulates:

>  Applications MUST NOT directly specify the syntax of queries, as this
>  can cause operational difficulties for deployments that do not
>  support a particular form of a query.
...
>  Extensions MUST NOT constrain the format or semantics of queries.

Given that the base resource used to bootstrap the JMAP infrastructure already
uses URI templates, it seems that they would be a good candidate for specifying
a means of adding these parameters in a way that respects the basic principle
that URI design ownership belongs to the domain administrator.

---------------------------------------------------------------------------

§8.3:

>  If this is not feasible, servers MUST ensure this path cannot be
>  controlled by an attacker, as again it may be used to steal
>  credentials.

This seems problematic, as it would need to be honored by *all* web servers
everywhere on the entire Internet to be effective. One cannot really presume
that all website operators will be aware of JMAP. I think the only really
sensible thing to do here is to forbid clients from using "/.well-known/jmap" on
a server until they have received some external positive indication that the
server supports JMAP (via SRV or some other mechanism).

---------------------------------------------------------------------------

The reference for Server-Sent events normatively points to the WHATWG HTML
spec, which changes on a continuing basis. I don't think we've really worked
out the mechanics of citing this kind of reference normatively; and JMAP takes
the especially dangerous step of citing a specific *section* of the document,
which may well change at arbitrary and potentially frequent intervals. There
is, in fact, no guarantee that the cited "text/event-stream" mechanism will
continue to exist in future versions of the WHATWG document (cf. the <keygen>
element).

In this particular case, I think we want to reference
https://www.w3.org/TR/eventsource/ instead.

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

§1.1:

>  The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
>  "SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" in this
>  document are to be interpreted as described in [RFC2119].

Please use the boilerplate from RFC 8174.

---------------------------------------------------------------------------

§1.2:

>  Where "Id" is given as a datatype, it means a "String" of at least 1
>  and maximum 255 octets in size, and MUST only contain characters from
>  the "URL and Filename safe" Base 64 Alphabet, as defined in section 5
>  of [RFC4648].  This is the ASCII alphanumeric characters ("A-Za-
>  z0-9"), hyphen ("-"), and underscore ("_").

This is unclear as to whether "=" is allowed. It is included in the RFC 4648
"URL and Filename safe" alphabet, but not mentioned in this document's
reiteration of that alphabet's definition. If the intention is to exclude "=",
please say so explicitly.

---------------------------------------------------------------------------

§1.2 says:

>  All record ids are assigned by the server, and are immutable.

...in which no exceptions are allowed.

§1.6.3 says:

>  The id of a record is immutable, and normally assigned by the server.

...where "normally" indicates that there will be exceptions. These say different
things. Please change the inaccurate one to match the accurate one.

---------------------------------------------------------------------------

§2.1:

>       "accounts": {
>         "13824": {
>           "name": "[email-masked]",
>           "isPersonal": true,
>           "isReadOnly": false,
>           "hasDataFor": [
>             "urn:ietf:params:jmap:mail",
>             "urn:ietf:params:jmap:contacts"
>           ]
>         },

Section 1.2 offers the advice:

>  In particular, it is wise to avoid:
>
...
>
>  o  Ids starting with digits
>
>  o  Ids that contain only digits

It seems that the examples should conform to the document's advice --
implementors often pay more attention to examples than even normative text, much
less non-normative advice.

---------------------------------------------------------------------------

§3.2:

>     3.  A "String" *client id*: an arbitrary string from the client to
>         be echoed back with the responses emitted by that method call

I think the document wants to say a bit more about the scope of uniqueness for
this client ID. Also, the name is a bit confusing, as "Client ID" is most easily
interpreted as "an identifier that identifies a client." This appears to be more
of a "Method ID."

---------------------------------------------------------------------------

§5.7:

>  o  *keywords*: "String[Boolean]" (default: ) A set of keywords that

I think you're missing something after "default:" inside the parentheses.

As an aside -- and I know this is just an example, but people are likely to
follow it when designing things riding on top of JMAP -- I was initially a bit
confused about why this is "String[Boolean]" rather than "String[]",
especially since the only valid value appears to be "true". Is this to allow
for smaller patch operations? If so, text explaining this design decision
would probably be good.

---------------------------------------------------------------------------

§7.1.1:

>  It can wait until the call completes and
>  then compare if the new state string after the /set is the same as
>  was pushed in the StateChange object; if so, it does not need to
>  waste a request asking for changes it already knows.

I think this phrasing may oversimplify things a bit. For example:

 - my current cached notion of state is "A"
 - I send a /set operation
 - I get a "StateChange" that indicates that the new state is "C"
 - I get a response to the /set operation indicating that the state has changed
   from "B" to "C"

The new state after the /set is the same as was pushed in the StateChange
object; however, I'm still missing state that resulted in the change from "A" to
"B".

I think the language in this example needs to be updated to be more precise
about when such requests can be omitted.

---------------------------------------------------------------------------

§7.3:

>  When a new connection is made to the event-source endpoint, a
>  client following the server-sent events specification will send a
>  Last-Event-ID HTTP header

Nit: "...HTTP header field..."