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

Benjamin Kaduk <kaduk@mit.edu> Fri, 08 March 2019 19:44 UTC

Return-Path: <kaduk@mit.edu>
X-Original-To: jmap@ietfa.amsl.com
Delivered-To: jmap@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id A38EB1310B0; Fri, 8 Mar 2019 11:44:33 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2
X-Spam-Level:
X-Spam-Status: No, score=-2 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (1024-bit key) header.d=mit.edu
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id qNEgm8KrT2O5; Fri, 8 Mar 2019 11:44:27 -0800 (PST)
Received: from NAM01-BY2-obe.outbound.protection.outlook.com (mail-eopbgr810109.outbound.protection.outlook.com [40.107.81.109]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id BA6D31277DB; Fri, 8 Mar 2019 11:44:26 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mit.edu; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=eGCjnQVi9Wn+Q2dMTeGZ6RZeYJve2z60dUnMVc+0f4o=; b=s7nqLlDCKGnhFWJlgRr/Y9kjTDuDMrbhaoYIsoRR5W8j+sTUNV5IZUbSM0OG40Aby62PYtmO/nUFzda8KZdNW9jmjXTmSi2Wio9Z3mMN8zkz+i7B7R535UxXpeW5egKw1KWbygtceXnAeYChyGR3l0Pk5yxDNr43/3NS9skz6kU=
Received: from SN2PR01CA0015.prod.exchangelabs.com (2603:10b6:804:2::25) by BN6PR0101MB3171.prod.exchangelabs.com (2603:10b6:405:2f::29) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1665.19; Fri, 8 Mar 2019 19:44:24 +0000
Received: from CO1NAM03FT026.eop-NAM03.prod.protection.outlook.com (2a01:111:f400:7e48::206) by SN2PR01CA0015.outlook.office365.com (2603:10b6:804:2::25) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384) id 15.20.1686.18 via Frontend Transport; Fri, 8 Mar 2019 19:44:24 +0000
Authentication-Results: spf=pass (sender IP is 18.9.28.11) smtp.mailfrom=mit.edu; ietf.org; dkim=none (message not signed) header.d=none;ietf.org; dmarc=bestguesspass action=none header.from=mit.edu;
Received-SPF: Pass (protection.outlook.com: domain of mit.edu designates 18.9.28.11 as permitted sender) receiver=protection.outlook.com; client-ip=18.9.28.11; helo=outgoing.mit.edu;
Received: from outgoing.mit.edu (18.9.28.11) by CO1NAM03FT026.mail.protection.outlook.com (10.152.80.162) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384) id 15.20.1643.13 via Frontend Transport; Fri, 8 Mar 2019 19:44:23 +0000
Received: from kduck.mit.edu (24-107-191-124.dhcp.stls.mo.charter.com [24.107.191.124]) (authenticated bits=56) (User authenticated as kaduk@ATHENA.MIT.EDU) by outgoing.mit.edu (8.14.7/8.12.4) with ESMTP id x28JiJC4021072 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 8 Mar 2019 14:44:21 -0500
Date: Fri, 08 Mar 2019 13:44:19 -0600
From: Benjamin Kaduk <kaduk@mit.edu>
To: Neil Jenkins <neilj@fastmailteam.com>
CC: iesg <iesg@ietf.org>, draft-ietf-jmap-core@ietf.org, Bron Gondwana <brong@fastmailteam.com>, jmap-chairs@ietf.org, IETF JMAP Mailing List <jmap@ietf.org>
Message-ID: <20190308194418.GU9824@kduck.mit.edu>
References: <155072687005.20308.1288342758446844678.idtracker@ietfa.amsl.com> <ebf89939-bf68-4458-a24f-5a37090385fd@beta.fastmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <ebf89939-bf68-4458-a24f-5a37090385fd@beta.fastmail.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
X-EOPAttributedMessage: 0
X-Forefront-Antispam-Report: CIP:18.9.28.11; IPV:CAL; SCL:-1; CTRY:US; EFV:NLI; SFV:NSPM; SFS:(10019020)(396003)(136003)(346002)(39860400002)(376002)(2980300002)(40224003)(189003)(199004)(51444003)(54094003)(51914003)(8676002)(106002)(26005)(1076003)(47776003)(186003)(14444005)(305945005)(50466002)(54906003)(88552002)(58126008)(36906005)(6916009)(316002)(2870700001)(478600001)(76176011)(356004)(246002)(4326008)(104016004)(5660300002)(2906002)(86362001)(75432002)(2486003)(7696005)(8936002)(476003)(11346002)(486006)(126002)(53416004)(53946003)(956004)(33656002)(106466001)(336012)(229853002)(966005)(30864003)(6306002)(55016002)(446003)(26826003)(23676004)(6246003)(426003)(786003); DIR:OUT; SFP:1102; SCL:1; SRVR:BN6PR0101MB3171; H:outgoing.mit.edu; FPR:; SPF:Pass; LANG:en; PTR:outgoing-auth-1.mit.edu; MX:1; A:1;
X-MS-PublicTrafficType: Email
X-MS-Office365-Filtering-Correlation-Id: f765507e-b310-4313-63e0-08d6a3fe7779
X-Microsoft-Antispam: BCL:0; PCL:0; RULEID:(2390118)(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600127)(711020)(4605104)(4608103)(4709054)(2017052603328)(7153060); SRVR:BN6PR0101MB3171;
X-MS-TrafficTypeDiagnostic: BN6PR0101MB3171:
X-MS-Exchange-PUrlCount: 3
X-Microsoft-Exchange-Diagnostics: 1; BN6PR0101MB3171; 20:Y9f1+g3tv0YjFtWPys7dKzZ29Wswczss+n2oHpuTAX3Av8fMbMPBpqqSbKBcnx0Cwtf7WEJQqSsEm1rVncQLx6MQ4ObkG+96qN6bxK8fKrnWoqdFBdP95Fn4b+lbIr7pkgh5scnzKC5oyoblfhBjYj0uJmfZL4DHxoVCCK7Fy1P1M7DVGzYDRNxAQ4f8718NRUJNTYDalWOL84u2QATOxRO+Sr6yIkE+pGylaI6JYFnKs7dOeilT3etsFqQb8c+9w11zMyE47UgtMzj60sZ5sj5BY3dWyZRnECJXZ+uc6zbar4mEKCfyMNRPQL3bQ4hRRX9yPVObJrmoqqa+gnrCNrQr18c+BoDp5E3l2QQhufIZVPIdUWts2Chg27OmwpVc2XvW7x0ycJwQ3Li3jXTtn0PFSLx1QcXuvN9D/cTzFsNyVxMwdDyHs2Zm48u+o3XPS2pyiudAhu0OuDG8xBPbZFlHGsf96dYVf0b5TnrlhEjpib1g8kSEKaFCDrcjRC3s4g53r5+3c1J7gpR9hlsQT0+5xpSYdY1jV2PVmZ8t93S70WoETjy3/HBvABK8Yx7yFbTFSJE5wBka0O52ZhOHs+Jk+nWYug1ntGd2v1ZQHvM=
X-Microsoft-Antispam-PRVS: <BN6PR0101MB317192E019386774229A1D20A04D0@BN6PR0101MB3171.prod.exchangelabs.com>
X-Forefront-PRVS: 0970508454
X-Microsoft-Exchange-Diagnostics: 1;BN6PR0101MB3171;23:Y3twX+3VpVQIgO0YamTrqWbXwr8T/XxUsu4tyH9mWMcfITIUOS/OBhtO3Sgbr3uMSXO692MVgHohLmF+4f0MRdCSW63NtROOX1rKora74XAwW7nLYEbeadN2QhMwlqlS0aoCfnXzFWxTtDaGEMTjT5GJTIx5dPEkSQIx5UewV95ThFacfsY0mDvRw4eWlHqx1WwRJnVz/zANuqbkJwQygrA1lKd+DLfslUxJfwp/9uW5FpF5rTeGwLnL9WuAyx2A8ekmo/wWtkVzU7napyLPXokFKlmznEOGrmLc33mo5ygPlWGVDefYFpVioOQN2GlaNuOeNav8vg+4k6hojoxleb4G7U6VoOZ7XwTgVb/h7ibZ9m8775eRyQB8EBxyaoZl5zUFZ7vIaqY2h1+bhZqzC0Jf1FxN7OXZSsuXetseHfhbc0KmAaCrg6kG7UkbsTxwAkXVL/jYk/MGzSZeMrN9R6wFw7ExXU4NFLikROk3UA3q9QLHBCtkDrPK21XpsCT6OefzDlgkBP+mx+Lg/ba+edNZTVyJ7M3IMY0GepNX/7C9/M+uAtVVQmFgkiCM7HsKyciwssKKYOMWMaBc2zvWUclLjpYTS4j+cEr8FjtO3iTY06fZNwC/hG2/Z2NWwEw7kRmS3eD9fq+TO9gWN2TToPHIHqSlP7+b88OtC1MdkMFQuh3IMGBXDUBATfkHiIctoIGy55SnM7iziQX9d63G1sYxgwaYpB/2sgqu6Bztm7QWNnQs6iJ/mqdM5njGMp7jST0mjvVFIU8rNHu6f3Lf8w3OHxw17fG+Gsx0rdzfM9u7HV9GKgR/faNlZ84ZqkTriJpzpFTIWphYBKDdotSVIQpOJI/f/6ece5FKmqDPJDDjhiWQL7lOFa1LWgYPs86PmKl01zYsnIlObdbsEt38SHkA0B6TD0QsrWoDcpoPSnmBiPKaoECp0PtoqjnkVrjU1OE0fzUugIVUPm1TPNKINu5TxqRTVhiM84FExpB8wKNXhIy3UZKFlm4onTXU0TQStIIMhIiZiyA//TrygLipK7559Ijb6TbDa1ODINJHIzEJJmwZWdIquuWSV6WCGQFqcVQK7HULnDqeSVavctZETHoZMb7V/mcneVIKXNkKpnalNwWA8j7R3mrm7pFLU/+FnatYAyPar3Jsx3wIlq9iasPaskkVfhueB2cI4zM5BQZbXspsg6KrPb9RMVPBBeFTuxGS4jBsUIbrBpiVKqTZAgh1DgLkhZrZ3Ei9T4H8M3uvYRzxoJ8Cn5qYmdv8K1DcntdGxcgM3DDwmamAIOxeG8OfbEtKXNiOZZb6gDPDt1oCEcgo7dGi7rISbLs2TaKPcCtLmXKcNX/uxOe8KNEBgg==
X-MS-Exchange-SenderADCheck: 1
X-Microsoft-Antispam-Message-Info: kyZtxzCHJYS+nAVlsflYmSmSPvgwzyQE41z9WjzzdDSNZl7GPKZHQNPvK0Al/6pM5nlW5MBu1da8BHPRH84Q5bhUi/Gb3y5uHYbJBLHkL7iKLN1BQ2lO9t/MEgJfSQsMD5rQgNNpmBCnVO9jHPTqfs3FQsLN+Zj1cEYlkaqlQwUg/7df/mMyTucT8Ms75o6vvaLi7lSYZxVixSoNBeBmnWYuYeDjew2pnxjsEmNvtUEGXlS6lm7ltd104naHAQivl3n9XA5GRW9tbsa7lusZglxgjjMVm7ncG2MXBkGPJlKJnO03JGO9n9kFNkcDYlFmCyhxGt5S3BWKmIUDJZHGFhNZ7GLxzV+L6LcrFrdghYh7V2c7f1vi+Uef0PARHB+jCd2tWoAGpOG5W7rQhDN9AOF6IhsXbV4N9M938WMFfrQ=
X-OriginatorOrg: mit.edu
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Mar 2019 19:44:23.5083 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: f765507e-b310-4313-63e0-08d6a3fe7779
X-MS-Exchange-CrossTenant-Id: 64afd9ba-0ecf-4acf-bc36-935f6235ba8b
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=64afd9ba-0ecf-4acf-bc36-935f6235ba8b; Ip=[18.9.28.11]; Helo=[outgoing.mit.edu]
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN6PR0101MB3171
Archived-At: <https://mailarchive.ietf.org/arch/msg/jmap/metlwpNZE7hz-JiMxaPmB112UQ8>
Subject: Re: [Jmap] Benjamin Kaduk's Discuss on draft-ietf-jmap-core-14: (with DISCUSS and COMMENT)
X-BeenThere: jmap@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: JSON Message Access Protocol <jmap.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/jmap>, <mailto:jmap-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/jmap/>
List-Post: <mailto:jmap@ietf.org>
List-Help: <mailto:jmap-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/jmap>, <mailto:jmap-request@ietf.org?subject=subscribe>
X-List-Received-Date: Fri, 08 Mar 2019 19:44:35 -0000

On Thu, Feb 28, 2019 at 01:14:44AM -0500, Neil Jenkins wrote:
> Thanks for the detailed feedback Benjamin.

Forking the thread to get back to the comments; the -16 looks good w.r.t.
the discussion points.

> *Discussion points*
> 
[...]
> 
> *Comments*
> 
> >  o "String[A]" - A JSON object where the keys are all "String"s, and
> >  the values are of type "A".
> > 
> > To avoid confusion about whether String is the only possible map key
> > type (vs., e.g., Id), maybe "A[B]" notation is more appropriate?
> 
> Sure, done.
> 
> > Section 1.2
> > 
> > Do we want to give any guidance about when it is (not) advisable to use
> > very short identifiers (even the 1-2 character case could be
> > noteworthy)? (I don't know what that guidance would be, so "no" is a
> > perfectly fine answer!)
> 
> Hmm, no. I don't think it really matters; the server can use whatever scheme it likes. (For example, might have monotonically increasing integers which you prefix with a letter for the type, leading to ids like "K1", "J6" etc. in the early cases; this is a perfectly reasonable scheme to use).
> 
> > W.r.t "NIL", that is the specifc 3-character identifier, right? Or do
> > we need to worry about having it embedded as part of a larger string?
> 
> Just the exact string "NIL".

Okay.  I guess the list would even be more parallel if the text was "Ids
that are the exact three characters "NIL" [...]", but this is your call.

> > (I don't see how prefixing every ID with an alphabetical character
> > avoids the "NIL" case, unless 'N' is disallowed from being that
> > character.)
> 
> Well, yes true, but this is general advice not a normative requirement, and if you might produce the letters "IL" as a suffix, then don't choose "N" as a prefix. Even then though, it should be fine unless these same ids are shared with another protocol where this string has special significance.
> 
> > Section 1.3
> > 
> > My math degree obliges me to note that, per trichotomy, zero is not a
> > positive integer.
> 
> I've renamed this to `UnsignedInt`.

Thanks.  (To be honest, I wasn't actually expecting this to change, so it
was a pleasant surprise.)

> > Section 1.6.1
> > 
> > nit: perhaps a "user object", or some broader rewrite like "in this
> > document, the keyword 'user' is used to refer to [...]" -- the current
> > text feels like it's dehumanizing, well, humans.
> 
> I have reworded this to:
> 
> *A user is a person accessing data via JMAP. A user has a set of permissions determining the data that they can see.*

Thanks.

> > Section 1.6.2
> > 
> > nit re. "All data belong to a single account": I assume this is "each datum
> > has a map to its respective account", not "the entire set of data in the
> > system is associated with a single privileged account", as that would
> > make the account distinction rather useless.
> 
> Yes. I have removed this sentence as redundant already based on previous feedback.
> 
> > Section 1.7
> > 
> > A "MUST" requirement for TLS would seem likely to disallow HTTP/3
> > (QUIC).
> 
> My understanding is that QUIC will also use TLS <https://datatracker.ietf.org/doc/draft-ietf-quic-tls/>.

The situation there is actually a bit complicated.  My understanding is
that the current plan is to use the TLS handshake messages, but transmit
them over the QUIC record layer, as a key-agreement mechanism.  The TLS
record-protection layer would not be used at all, and so I wouldn't really
characterize that as "using TLS".

> > Does "MUST confirm with the HTTP Authentication framework" preclude
> > other authentication schemes? The RFC 7235 framework has some warts,
> > and in many webapp settings there are plausible reasons to prefer
> > app-layer authentication schemes. I guess that JMAP as being an
> > API-driven model may have a more natural mapping to the 7235 framework,
> > and I do note the remarks in the shepherd writeup that an authentication
> > scheme was removed from a previous version of this document. So mostly
> > I'm just asking if we are intending to force the 7235 framework to be
> > the one and only authentication scheme for JMAP. (Who knows, maybe this
> > will drive adoption of new HTTP Authentication Schemes or prompt renewed
> > interest in their development?)
> 
> This has been raised by other reviewers too, and I think I'm going to just cut the line and the requirement. Implementors that want/need to use something other than 7235 will almost certainly do so regardless of what we write here.

That's fair.

> >  Clients MUST understand and be able to handle standard HTTP status
> >  codes appropriately.
> > 
> > (Is there a precise definition for "standard HTTP status codes"?)
> 
> No, I've cut this line as well. This was recommended text in an early draft of BCP 56bis <https://datatracker.ietf.org/doc/draft-ietf-httpbis-bcp56bis/>, but has since been cut.
> 
> >  An authenticated client can fetch the JMAP Session object with
> >  details about the data and capabilities the server can provide as
> > 
> > nit: maybe this is "a JAMP session object" with the indefinite article?
> 
> I used the definite article here because the client is authenticated, and therefore has access to one specific JMAP session object associated with those credentials.

Would that indicate "their"/"its", then?

> > Section 2
> > 
> >  o *username*: "String" The username associated with the given
> >  credentials.
> > 
> > This seems to implicitly require that all credentialed entities have
> > user accounts, denying the possibility for some shared-credential or
> > machine-credential models. Is this intended?
> 
> Hmm, fair point. I'll change the description to:
> 
> *The username associated with the given credentials, or the empty string if none.*
> 
> >  o *state*: "String" A string representing the state of this object
> >  on the server. If the value of any other property on the session
> >  object changes, this string will change. The current value is
> >  also returned on the API Response object (see section 3.3),
> >  allowing clients to quickly determine if the session information
> >  has changed (e.g. an account has been added or removed) and so
> >  they need to refetch the object.
> > 
> > As written this sounds like a serialized version of the full object
> > (without the 'state' field, of course, to avoid recursion), but I
> > suspect this is intended to be more like a generation number or hash or
> > serialized state, as a short lookup key. Greater clarity would be
> > welcome.
> 
> Yes, the intention is for this to be a short generation/modification number or hash. While this is preferred, it is not required to be a short string for functional operation. I hesitate to put further normative restrictions on it as this may make it harder for existing server implementations of other APIs with a similar concept but longer strings to implement a JMAP interface.
> 
> I'm happy to suggest this however; I'll change "A string representing…" to "A (preferably short) string representing…".

That looks good to me.

> > Section 3.2
> > 
> >  o *using*: "String[]" The set of capabilities the client wishes to
> >  use. The client MAY include capability identifiers even if the
> >  method calls it makes do not utilise those capabilities. The
> >  server advertises the set of specifications it supports in the
> >  JMAP Session object, as keys on the _capabilities_ property.
> > 
> > Are the "capability identifiers" the same as or different from the
> > "specifications [the server] supports"?
> 
> I have updated this to stick to "capability" as the terminology; a specification (document) may define more than one capability that can be supported independently, so the two are not interchangeable.
> 
> > Is the "client id" something that could also be called a "request ID"?
> > (I note that RFC 7252 calls a similar thing a "token" but has some text
> > wishing they called it a "request ID".)
> 
> Based on feedback from another reviewer I have renamed this "method call id". (A request in JMAP is a bundle of method calls so "request id" would not be appropriate.)

Sounds good.

> > The 'prefixed with a "#"' laguage seems like it has some potential for
> > confusion; is it better to write this like "the first character of the
> > creation ID MUST be octothorpe ('#')"?
> 
> The # is not part of the creation id. References to a creation id may be used in a context where an id is expected by prefixing the creation id with a #. Since this is not a valid character in an id, it is easy to determine that this must be a creation reference. I have added a reference at this point to the /set definition where this is explained in more detail.

That will help, though I'm not sure exactly how much.
Looking at this again, I think the biggest thing that confused me on my
first read is that I didn't internalize that creation ids and
ResultReferences are different things (since they both use the magic "#"
prefix), and I was trying to reconcile them into the same bucket.
So maybe when ResultReference is introduced there could be a note about how
"the ResultReference performs a similar role to that of the creation id, in
that it allows a chained method call to refer to information not available when
the request is generated.  However, they are different types and not
interchangable; the only commonality is the octothorpe used to indicate
them".  But, maybe I'm unusual for being confused about this and it's not
worth the extra text.

> > AFAICT having finished reading the doc, these can only be created by
> > explicit client action via a "create" array (or equivalent), where the
> > array keys are the "creation id"s since the client has to use some
> > handle before the server has assigned them. It should be possible to
> > add a very brief note to that effect here.
> 
> I have added cross-references to make this a bit clearer. There is now text here that says:
> 
> *As the server processes API requests, any time it successfully creates a new record it adds to this map the creation id (see the *create* argument to "/set" in section 5.3), with the server-assigned real id as the value.*
> 
> > Given the later usage in Section 3.3, I would consider splitting the
> > *Invocation* definition into a subsection.
> 
> OK
> 
> > Section 3.3
> > 
> > (editorial) If the second element of the Invocation tuple is literally
> > "arguments for the method", do we really have enough rope to make
> > response objects like this (unless we limit ourselves to conceptually
> > "in/out" arguments with no pure-output functionality)?
> 
> I think the document as a whole makes the syntax clear enough, but if you have a suggestion for better wording here I'm happy to consider it.

I don't have any wording suggestions, so this will probably be fine as-is.

> > Section 3.5
> > 
> > I expect that deployment will see differences of opinion as to which
> > checks fall under "syntactically valid", but it's unclear whether we
> > need to attempt to forestall such debate.
> 
> I would consider this to mean something that is valid JSON and when decoded matches the type signature of the Request object. I will clarify this in the spec, although I don't think small deviations in interpretation are likely to result in significant interoperability issues.

(I agree that it's unlikely for any interoperability issues to occur, so
I'm not very worried about this.)

> > Section 3.5.1
> > 
> >  o "urn:ietf:params:jmap:error:notRequest" The request parsed as JSON
> >  but did not match the structure of the Request object.
> > 
> > "the Request object" makes me think of "the thing the client stuck in
> > the field named "Request", which is probably not the intent. Perhaps
> > the key phrase is "type signature"?
> 
> Yes, that's clearer; I have changed this.
> 
> > Section 3.5.1.1
> > 
> > Since there is special handling for "urn:ietf:params:jmap:error:limit",
> > do we want an example to show that?
> 
> I'll add a second example.

Thanks.

> > Section 3.5.2
> > 
> >  With the exception of "serverPartialFail", the externally-visible
> >  state of the server MUST NOT have changed if an error is returned at
> >  the method level.
> > 
> > nit: You don't say where in the type signature "serverPartialFail" is
> > supposed to be in order to trigger the special handling.
> 
> Sorry, I don't understand this comment. serverPartialFail is a method-level error so it would be returned like any other method-level error:
> 
> [ "error", {
>   "type": "serverPartialFail"
> }, "call-id-1" ]

I think I misparsed what this was trying to say, namely that there is a
serverPartialFail element whose state is allowed to change if there is a
method-level error.  That is, of course, nonsense; the fact that the server
returns a serverPartialFail error code indicates that the "entire" server
state is indeterminate and the client must resync.  So any response to my
comment here would be along the lines of changing the text to "with the
exception of when the 'serverPartialFail' error is returned".

> >  Further general errors MAY be defined in future RFCs. Should a
> >  client receive an error type it does not understand, it MUST treat it
> >  the same as the "serverFail" type.
> > 
> > Does this imply that "serverPartialFail" is unique in its ability to
> > partially modify state?
> 
> General errors (that could be returned by any method) are expected to be defined rarely. A partial-fail where the server commits state changes but then fails part way is also expected to be very rare (because the client has to do a full resync to recover). 
> 
> If necessary, it's more likely a future specification could define an error that partially modifies state but is only returned in very specific circumstances and when the client has opted in to that capability, so it knows the client will be able to handle it.

Okay.

> > Section 3.6
> > 
> > (Same comment about '"prefixes with "#"' as above.)
> > 
> >  If an argument object contains the
> >  same argument name in normal and referenced form (e.g. "foo" and
> >  "#foo"), the method MUST return an "invalidArguments" error.
> > 
> > This "same argument name in normal and referenced form" applies even in
> > arbitrarily nested objects, right? It seems like this could potentially
> > be expensive and/or difficult to enforce.
> 
> Arguments are not nested. Only the top-level keys in the object are arguments. The value to an argument may be an object, but the keys in there are not arguments; they are properties of that object.

Thanks for clarifying; that makes sense now.

> > Section 5.1
> > 
> >  o *ids*: "Id[]|null" The ids of the Foo objects to return. If
> >  "null" then *all* records of the data type are returned, if this
> >  is supported for that data type.
> > 
> > Maybe note that the "maxObjectsInGet" capability limit might kick in
> > here for the "null" case?
> 
> OK, done.
> 
> >  o *properties*: "String[]|null" If supplied, only the properties
> >  listed in the array are returned for each Foo object. If "null",
> >  all properties of the object are returned. The id property of the
> >  object is *always* returned, even if not explicitly requested. If
> >  an invalid property is requested, the call MUST be rejected with
> >  an "invalidArguments" error.
> > 
> > Because we're constrained to a single type here, there's no way for only
> > some (but not all) of the objects to have any given property, right?
> 
> Correct.
> 
> > Section 5.3
> > 
> >  o *ifInState*: "String|null" This is a state string as returned by
> >  the _Foo/get_ method. If supplied, the string must match the
> > 
> > So, just to double-check, this applies to the state of all objects of
> > the type in question (for the account)?
> 
> Yes.
> 
> > It might be worth reiterating, since we frequently see this sort of check against the current state at
> > a per-object granularity, in other systems.
> 
> OK, will do.
> 
> > Do the "creation id"s in the "create" array include leading "#"?
> 
> No. A creation id is of type `Id` which does not allow this character. When a creation id is referenced from elsewhere, this is signified by prefixing the id with a #.
> 
> > Section 5.4
> > 
> >  o *destroyFromIfInState*: "String|null" This argument is passed on
> >  as the "ifInState" argument to the implicit _Foo/set_ call, if
> >  made at the end of this request.
> > 
> > This is "the Implicit _Foo/set_ call that effectuates the destruction of
> > the original", right?
> 
> Yes.

I think I was asking for a little more text here, basically to reiterate
that the Foo/set call is a deletion call.

> >  o *created*: "Id[Foo]|null" A map of the creation id to an object
> >  containing any properties of the copied Foo object that are set by
> >  the server (such as the _id_ in most object types). This argument
> >  is "null" if no Foo objects were successfully copied.
> > 
> > Maybe note that the id property will also likely differ from the one
> > that was passed in in the create array, since the ids in question are
> > from different accounts?
> 
> Sure, done.
> 
> > Section 5.6
> > 
> >  o *upToId*: "Id|null" The last (highest-index) id the client
> >  currently has cached from the query results. When there are a
> > 
> > Just to double-check: semantically this is an object identifier, but the
> > ordering we care about for the "up to" part is the ordering in the query
> > results?
> 
> Yes.
> 
> >  o *removed*: "Id[]" The _id_ for every foo that was in the query
> >  results in the old state and is not in the results in the new
> >  state. If the server cannot calculate this exactly, the server
> >  MAY return extra foos in addition that may have been in the old
> >  results but are not in the new results. If the sort and filter
> >  are both only on immutable properties and an _upToId_ is supplied
> >  and exists in the results, any ids that were removed but have a
> >  higher index than _upToId_ SHOULD be omitted. If the _filter_ or
> >  _sort_ includes a mutable property, the server MUST include all
> >  foos in the current results for which this property MAY have
> >  changed.
> > 
> > I'm having a hard time understanding this "MAY have changed" text
> > (which, for one, shouldn't be using 2119 language). Taking note that
> > this is the "removed" list, this text seems to be saying that I include
> > in the "removed" list any object that *is* in the current query results,
> > but which may have had a property change since the previous state. So
> > we end up having to do a "remove + add" for this sort of property
> > change, is that right?
> 
> Yes. If the item may have moved in the list, the client needs to remove and then re-add it at its current index to ensure its query results cache is correct. I will add a sentence to explain this.

Thanks!

> (I have changed the "MAY" to "may"; this was incorrect use of 2119 language, you're right.)
> 
> > We may need more detail on the "splices in" operation, with respect to
> > the indices in the "added" array reflecting the final state, and the
> > local cached indices needing to be updated on the fly during the
> > splicing operation in order to get things inserted in the proper places.
> 
> Right. I have tried to make this a bit clearer; it now reads:
> 
> *The result of this is that if the client has a cached sparse array of foo ids corresponding to the results in the old state:**
> *
> *
> *
> *    fooIds = [ "id1", "id2", null, null, "id3", "id4", null, null, null ]**
> *
> *
> *
> *then if it ***splices out*** all ids in the removed array that it has in its cached results:**
> *
> *
> *
> *    removed = [ "id2", "id31", ... ];
>     fooIds => [ "id1", null, null, "id3", "id4", null, null, null ]**
> *
> *
> *
> *and ***splices in*** (one-by-one in order, starting with the lowest index) all of the ids in the added array:**
> *
> *
> *
> *    added = [{ id: "id5", index: 0, ... }];
>     fooIds => [ "id5", "id1", null, null, "id3", "id4", null, null, null ]**
> *
> *
> *
> *and ***truncates*** or ***extends*** to the new total length, then the results will now be in the new state.**
> *
> *
> *
> *Note: splicing in adds the item at the given index, incrementing the index of all items previously at that or a higher index. Splicing out is the inverse, removing the item and decrementing the index of every item after it in the array.*

Thanks, I think that will help make things clear.

> > Section 5.8
> > 
> >  2. It must resolve back-references to previous method results that
> >  were processed on a different server. This is a relatively
> >  simple syntactic substitution, described in section 3.6.
> > 
> > Relatively simple, yes, but does require properly parsing the JSON
> > (right?).
> 
> Yes.

The security reviewer part of me wants the text to point out that this
puts the proxy's JSON parser squarely in the attack surface, so if it is
poorly implemented that's a risk for the system as a whole.  I want to say
that you could do (1) without a full JSON parser, so to some extent it is
just the backreferences that make this a full-fledged requirement on the
proxy.

> > Section 6.3
> > 
> > Why does Blob/copy use 'blobIds' instead of 'create' like generic /copy?
> 
> Because it's a different format (just an array of ids rather than a map of creation id to object). We considered it clearer if arguments with the same name were of the same type.

Good point; I think I missed that the first time around.

> >  o *fromAccountId*: "Id" The id of the account emails were copied
> >  from.
> > 
> >  o *accountId*: "Id" The id of the account emails were copied to.
> > 
> > I assme the "emails" are copy/paste bugs.
> 
> Yes, good spot. I've fixed this.
> 
> > Section 7.2
> > 
> >  o *deviceClientId*: "String" (immutable) An id that uniquely
> >  identifies the client + device it is running on. The purpose of
> >  this is to allow clients to identify which PushSubscription
> >  objects they created even if they lose their local state, so they
> >  can revoke or update them. This string MUST be different on
> >  different devices, and be different from other vendors. It SHOULD
> > 
> > What's the first vendor that's the basis for comparison?
> 
> Sorry, I'm not quite sure what you're asking here.

I think my fundamental confusion here is about where the device ID is
coming from -- is this supposed to be something like the serial number or
IMEI number from my phone, that is a fixed and permanently bound to a
specific device (and not changeable)?  I think we delved into related
topics for Alissa's Discuss, but not quite into this aspect of things.  If
I start talking about "vendors", is there some implication that the
identity of the current device's vendor is input to the
deviceClientId-generation process (whether explicitly or implicitly via a
manufacturer prefix in some other identifier)?  So the literal answer to my
question here is something like the "first vendor" is the manufacturer of
the device being identified, or the author of the app generating this
specific deviceClientId.

Even in the current text (-16), I wonder if "a device id" is quite enough,
as opposed to "a unique identifier associated with the device where the
JMAP client is running" or similar.

> >  be easy to re-generate, not depend on persisted state. A secure
> >  hash that includes both a device id and vendor id is one way this
> >  could be achieved.
> > 
> > Easy to re-generate by whom?
> 
> The client.
> 
> > How does the client get the deviceClientId value the first time?
> 
> It generates it from its vendor app id and a device id it gets from the OS.

E.g., we don't say "gets from the OS" anywhere in the -16.

> e.g. `sha256( "com.example.mail" . $device-id )`
> 
> (where I own example.com and `$device-id` is from the OS).
> 
> >  The POST request MUST have a content type of "application/json" and
> >  contain the UTF-8 JSON encoded object as the body. The request MUST
> > 
> > (editorial) Are we back to what the JMAP server sends to the notification URL?
> 
> Yes.
> 
> >  The push subscription is tied to the credentials used to authenticate
> >  the API request that created it. Should these credentials expire or
> >  be revoked, the push subscription MUST be destroyed by the JMAP
> >  server.
> > 
> > How is the JMAP server expected to learn about credential expiry or
> > revocation?
> 
> That's implementation dependent.

I think when  I first read this I was confused about whether this was
credentials for [JMAP client to JMAP server] or [JMAP server to push
service].  But the PushSubscription object is a pure JMAP thing, so it's
the first case and the JMAP server does its own credential management, as
you note here.

Which is a long way of saying "I agree, and the current text is fine" :)

> >  When these credentials have their own expiry (i.e. it is a session
> >  with a timeout), the server SHOULD NOT set or bound the expiry time
> > 
> > (editorial) A session where?
> 
> If your authentication has the concept of a session that expires after a set amount of time, or after a set amount of idle time (rather than, say, just a BASIC username/password with no expiry time).

(seems like my same confusion as above, nothing to see here)

> > Section 7.2.1
> > 
> > I would suggest a section reference to "follows the common /get
> > semantics from Section 5.1, with the exceptions that [...]" to more
> > clearly incorporate by reference the existing text.
> 
> OK, I've added a reference.
> 
> > What nature of authorization checking is done for these get requests?
> 
> These are standard method calls that are sent like any other, and so authenticated like any other at the HTTP request level.

(The new text in the -15 addresses what I was getting at; thanks)

> > Section 7.2.3
> > 
> > Given that all of these times are going to be in the past for all
> > readers, do we want to say something about what time the client is
> > performing these operations at?
> 
> Yes, that's reasonable. I have done this (and fixed up the times in the examples to make sense relative to one another).

Thank you!

> > Section 8.1
> > 
> > Again, this (duplicated!) MUST for TLS prevents future HTTP/3 QUIC
> > usage.
> > 
> > Also, please reference RFC 7525.
> 
> I have added a reference to RFC 7525 and removed the duplication.
> 
> > Section 8.2
> > 
> > Please recommend against Basic. Also, the concept of what a "user's regular password" is seems a bit
> > underspecified.
> 
> OK. I've modified this to read:
> 
> *Use of the Basic authentication scheme is NOT RECOMMENDED. Services that choose to use this are strongly recommended to require generation of a unique "app password" via some external mechanism for each client they wish to connect.*
> 
> (I've just cut "user's regular password" since it's redundant.)

Beautiful!  (Maybe s/use this/use it/, but I'm sure the RFC Editor is on
top of it.)

> > Section 8.3
> > 
> > DNS SRV-based autodiscovery seems the only type of autodiscovery
> > available that is susceptible to the attack described here; you should
> > probably just state that explicitly.
> 
> OK, will do.
> 
> > Section 8.4
> > 
> > While true, 8529's security considerations are pretty sparse; we could
> > say more here about not overscanning, limiting string length, being
> > strict about tokenization, etc.
> 
> Do you have any suggested text?

As for any serialization format, parsers need to thoroughly check the
syntax of the supplied data.  JSON uses opening and closing tags for
several types and structures, and it is possible that the end of supplied
data will be reached when scanning for a matching closing tag; this is an
error condition and implementations need to stop scanning at the end of the
supplied data.

JSON also uses a string encoding with some escape sequences to encode
special characters within a string.  Care is needed when processing these
escape sequences to ensure that an escape sequence is fully formed before
the special processing is triggered, with special care taken when the
escape sequences appear adjacent to other (non-escaped) special characters
or the end of data (as in the previous paragraph).

If parsing JSON into a non-textual structured data format, implementations
may need to allocate storage to hold JSON string elements.  Since JSON
does not use explicit string lengths, the risk of denial of service due to
resource exhaustion is small, but implementations may still wish to place
limits on the size of allocations they are willing to make in any given
context, to avoid untrusted data causing excessive memory allocation.

> > Section 8.7
> > 
> >  and JMAP server the client MUST specify encryption keys when
> >  establishing the PushSubscription and ignore any push notification
> >  received that is not encrypted and signed with those keys.
> > 
> > There's no signing in RFC 8291; there is, however, a separate
> > authentication secret.
> 
> Right, I will update this text.
> 
> > Section 8.8
> > 
> > I would be surprised if the propsects for traffic analysis were limited
> > to just push. Even regular accesses may still be susceptible to traffic
> > analysis.
> 
> No doubt something could be gleaned. I will make this slightly more generic, although push notifications still have the clearer information leak.

Sure (the new text looks fine).

> > Section 9.4.3
> > 
> > You probably should document that recourse for non-response after 30
> > days is to request action from the IESG.
> 
> OK.

Thanks for all the updates!

I've changed my position to "No Objection" in the datatracker.

-Benjamin

P.S. I appreciate the separate "description" column for the error registry
in the -16, but the formatting of the current table is basically
unreadable.  I had to go find
https://github.com/jmapio/jmap/commit/3f4b7f83c99596a553bd81324c092516a9babbcf#diff-3004124a0560c91bfc79eb996bd81433
to get something useful to look at.