Subject: [Tsv-art] Tsvart last call review of draft-ietf-jmap-websocket-04

From: Bob Briscoe via Datatracker <[email-masked]>
Date: Thu, 19 Dec 2019 08:41:18 -0800

Reviewer: Bob Briscoe
Review result: Ready with Issues

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
[email-masked] 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.

---

From: Alexey Melnikov <[email-masked]>
Date: Thu, 19 Dec 2019 17:17:42 +0000

Hi Bob,

Thank you for your review. A few quick comments below:

On 19/12/2019 16:41, Bob Briscoe via Datatracker wrote:

  [snip]
>     through a WebSocket JMAP connection.
I think this case is not interesting, as it is effectively a separate 
API endpoint in standard JMAP anyway. So there would be just no way of 
doing this in JMAP over WebSocket.
>     NOT be transmitted over this connection.
This is a more interesting case. So saying something here would be useful.
> form of..." do not say what to do if they are not, and do not seem to

And so is this.

[snip]

Best Regards,

Alexey

---

From: Bob Briscoe <[email-masked]>
Date: Fri, 20 Dec 2019 01:48:32 +0000

Alexey,

Pls check the draft for more occurrences than the three I found.

I think your word uninteresting meant "not possible for current 
implementations to do this".

You might have misunderstood my point that defining what to do on 
receipt of an unexpected (i.e uninteresting in your words) message is a 
useful exercise to allow for extensibility. If you don't say whether to 
ignore or to abort the connection (or whatever), it becomes impossible 
to predict what pre-existing implementations will do if you want to 
extend the protocol in future.

Bob

On 19/12/2019 17:17, Alexey Melnikov wrote:
> _______________________________________________
> Tsv-art mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/tsv-art

-- 
________________________________________________________________
Bob Briscoe                               http://bobbriscoe.net/

---

From: Alexey Melnikov <[email-masked]>
Date: Fri, 20 Dec 2019 09:01:39 +0000

Hi Bob,

> On 20 Dec 2019, at 01:48, Bob Briscoe <[email-masked]> wrote:
> 
> Alexey,
> 

Right.

> I think your word uninteresting meant "not possible for current implementations to do this".

Yes. I think your general point is valid, I just wanted to point out that one of the cases you mentioned doesn’t need further considerations.
> 
> You might have misunderstood my point that defining what to do on receipt of an unexpected (i.e uninteresting in your words) message is a useful exercise to allow for extensibility. If you don't say whether to ignore or to abort the connection (or whatever), it becomes impossible to predict what pre-existing implementations will do if you want to extend the protocol in future.

No, I agree with you here.
>> I think this case is not interesting, as it is effectively a separate API endpoint in standard JMAP anyway. So there would be just no way of doing this in JMAP over WebSocket.
>>> 4.1 Handshake
>>>     Other message types MUST