Last Call Review of draft-ietf-jmap-websocket-04
review-ietf-jmap-websocket-04-tsvart-lc-briscoe-2019-12-19-00
Request | Review of | draft-ietf-jmap-websocket |
---|---|---|
Requested revision | No specific revision (document currently at 07) | |
Type | Last Call Review | |
Team | Transport Area Review Team (tsvart) | |
Deadline | 2019-12-19 | |
Requested | 2019-12-05 | |
Authors | Kenneth Murchison | |
I-D last updated | 2019-12-19 | |
Completed reviews |
Secdir Last Call review of -04
by Leif Johansson
(diff)
Genart Last Call review of -04 by Linda Dunbar (diff) Tsvart Last Call review of -04 by Bob Briscoe (diff) |
|
Assignment | Reviewer | Bob Briscoe |
State | Completed | |
Request | Last Call review on draft-ietf-jmap-websocket by Transport Area Review Team Assigned | |
Posted at | https://mailarchive.ietf.org/arch/msg/tsv-art/cqX1H_l7XFdB2mQ6DT3pGl3JqRI | |
Reviewed revision | 04 (document currently at 07) | |
Result | Ready w/issues | |
Completed | 2019-12-19 |
review-ietf-jmap-websocket-04-tsvart-lc-briscoe-2019-12-19-00
This document has been reviewed as part of the transport area review team's ongoing effort to review key IETF documents. These comments were written primarily for the transport area directors, but are copied to the document's authors and WG to allow them to address any issues raised and also to the IETF discussion list for information. When done at the time of IETF Last Call, the authors should consider this review as part of the last-call comments they receive. Please always CC tsv-art@ietf.org if you reply to or forward this review. ==Transport-specific review comments== The only transport-specific issues that I can think of would relate to holding open a connection for a long time, which increases the risk of brute-force connection hijacking and has interactions with NATs and mobility. However, they are all WebSockets issues, and adding jmap over WebSockets doesn't change any of that. ==Non-transport-related review comments== I found the document readable and comprehensible. The following is my only concern... ===Shouldn't extensibility be discussed?=== The specification is full of statements saying what peer A MUST do, but lacks statements saying what peer B MUST do if peer A doesn't do what it is supposed to. Perhaps there needs to be a default case for what to do if one peer receives a message that violates the Websockets JMAP subprotocol (or at least one peer believes it violates the version of the protocol that it supports). Examples: 4. JMAP Subprotocol Binary data MUST NOT be uploaded or downloaded through a WebSocket JMAP connection. What if they are? 4.1 Handshake Other message types MUST NOT be transmitted over this connection. What if they are? 4.2 WebSocket Messages The lists of allowed messages following "The messages MUST be in the form of..." do not say what to do if they are not, and do not seem to allow for extensibility. ===Nits=== 4.1 Handshake CURRENT: If a client receives a handshake response that does not include "jmap" in the "Sec-WebSocket-Protocol" header, ... PROPOSED: If a client receives a handshake response in which the value of the "Sec-WebSocket-Protocol" header is not "jmap", ... REASON: 'include' implies it would have been valid for the server to have sent a list of subprotocols.