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

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



Thanks, applied.

It seems to me that you've been pesky enough with all these
documentation patches, that you should be punished for it with commit
access ;-)

Do you have a github account?

On Wed, Oct 18, 2017 at 02:40:12PM -0500, Eric Blake wrote:
> 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
> 
> 

-- 
Could you people please use IRC like normal people?!?

  -- Amaya Rodrigo Sastre, trying to quiet down the buzz in the DebConf 2008
     Hacklab


Reply to: