[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

[PATCH] docs: Clarify options that should not have data



The spec indirectly stated that NBD_OPT_LIST should be sent without
data (by virtue of the fact that NBD_REP_ERR_INVALID mentioned
non-zero data length as a reason for failure), but making zero-length
options explicit in the documentation of each option is nicer.
As an exception, make it clear that NBD_OPT_ABORT should succeed at
ending the connection rather than being ignored with an error due
to non-zero length, even though well-behaved clients won't send such
length.

This mirrors existing practice in many implementations, and is done
in a way that can be copied to NBD_OPT_STRUCTURED_REPLY and other
extension options that do not require data.

Signed-off-by: Eric Blake <eblake@redhat.com>
---

Applies to the master branch; based on
https://lists.debian.org/nbd/2017/nbd-201710/msg00020.html

 doc/proto.md | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/doc/proto.md b/doc/proto.md
index 909aa25..8a5b25f 100644
--- a/doc/proto.md
+++ b/doc/proto.md
@@ -861,6 +861,10 @@ of the newstyle negotiation.
     The client desires to abort the negotiation and terminate the
     session. The server MUST reply with `NBD_REP_ACK`.

+    The client SHOULD NOT send any additional data with the option;
+    however, a server SHOULD ignore any data sent by the client rather
+    than rejecting the request as invalid.
+
     Previous versions of this document were unclear on whether
     the server should send a reply to `NBD_OPT_ABORT`. Therefore
     the client SHOULD gracefully handle the server closing the
@@ -877,6 +881,10 @@ of the newstyle negotiation.
     list if TLS has not been negotiated, the server is operating in
     SELECTIVETLS mode, and the entry concerned is a TLS-only export.

+    The client MUST NOT send any additional data with the option, and
+    the server SHOULD reject a request that includes data with
+    `NBD_REP_ERR_INVALID`.
+
 - `NBD_OPT_PEEK_EXPORT` (4)

     Was defined by the (withdrawn) experimental `PEEK_EXPORT` extension;
@@ -886,9 +894,11 @@ of the newstyle negotiation.

     The client wishes to initiate TLS.

-    The server MUST either reply with `NBD_REP_ACK` after which
-    point the connection is upgraded to TLS, or an error reply
-    explicitly permitted by this document.
+    The client MUST NOT send any additional data with the option.  The
+    server MUST either reply with `NBD_REP_ACK` after which point the
+    connection is upgraded to TLS, or an error reply explicitly
+    permitted by this document (for example, `NBD_REP_ERR_INVALID` if
+    the client included data).

     See the section on TLS above for further details.

@@ -1145,8 +1155,10 @@ case that data is an error message string suitable for display to the user.
 * `NBD_REP_ERR_INVALID` (2^31 + 3)

     The option sent by the client is known by this server, but was
-    determined by the server to be syntactically invalid. For instance,
-    the client sent an `NBD_OPT_LIST` with nonzero data length.
+    determined by the server to be syntactically or semantically
+    invalid. For instance, the client sent an `NBD_OPT_LIST` with
+    nonzero data length, or the client sent a second
+    `NBD_OPT_STARTTLS` after TLS was already negotiated.

 * `NBD_REP_ERR_PLATFORM` (2^31 + 4)

-- 
2.13.6


Reply to: