Re: [quicwg/base-drafts] Unidirectional Streams (#643)

[ekr <[e-mail]>]

ekr commented on this pull request.

This seems like a good start

>  # Streams: QUIC's Data Structuring Abstraction {#streams}
 
-Streams in QUIC provide a lightweight, ordered, and bidirectional byte-stream
-abstraction modeled closely on HTTP/2 streams {{?RFC7540}}.
+Streams in QUIC provide a lightweight, ordered, and unidirectional byte-stream.

nit: "and" is unnecessary here.

>  
-Stream ID 0 (0x0) is reserved for the cryptographic handshake.  Stream 0 MUST
-NOT be used for application data, and is the first client-initiated stream.
+Stream ID 0 (0x0) in both directions is reserved for the cryptographic

Nit: "0x0" is unnecessary here. People know what 0 is.

>  
-Stream ID 0 (0x0) is reserved for the cryptographic handshake.  Stream 0 MUST
-NOT be used for application data, and is the first client-initiated stream.
+Stream ID 0 (0x0) in both directions is reserved for the cryptographic
+handshake.  Both client and server send cryptographic handshake messages on
+their stream 0, which is bound into a bidirectional channel.  Stream 0 MUST NOT

I'm not sure you need to say "bound into a bidirectional channel". you just send on one and receive on the other.

> @@ -2337,13 +2338,21 @@ Stream IDs are usually encoded as a 32-bit integer, though the STREAM frame
 stream ID are zero.
 
 
-## Life of a Stream
+## Stream States
+
+Endpoints do not coordinate the creation of streams; they are created
+unilaterally by either endpoint.

by the endpoint which will be sending on them. I know you say that below.

> @@ -2337,13 +2338,21 @@ Stream IDs are usually encoded as a 32-bit integer, though the STREAM frame
 stream ID are zero.
 
 
-## Life of a Stream
+## Stream States
+
+Endpoints do not coordinate the creation of streams; they are created
+unilaterally by either endpoint.
+
+A stream sender controls the state of its streams; the state of a stream is only
+affected by frames that are sent by the sending endpoint.  However,

Isn't this going to be wrong if you are going to allow receiver RST?

> @@ -2337,13 +2338,21 @@ Stream IDs are usually encoded as a 32-bit integer, though the STREAM frame
 stream ID are zero.
 
 
-## Life of a Stream
+## Stream States
+
+Endpoints do not coordinate the creation of streams; they are created
+unilaterally by either endpoint.
+
+A stream sender controls the state of its streams; the state of a stream is only
+affected by frames that are sent by the sending endpoint.  However,

And in any case, isn't the window part of the state?

>                              |        |
                             +--------+
+                                 |
+                      send STREAM with FIN flag
+                         send RST_STREAM
+                                 |
+                                 v
+                            +--------+
+                            |        |
+                            | closed |
+                            |        |
+                            +--------+
+~~~

This seems like it's a bug in the current state machine too, but there's clearly a difference between FIN and RST because the sender still has to retransmit when it has sent FIN. I guess you could claim that these are substates, but now that you've simplified this piece of the state machine, I think adding them explicitly here would make sense.

>  
 
-### idle
+### The "idle" State

I think I would write separate state descriptions for each side. Let me know if you want help.

> -described in {{stream-id}}.  A RST_STREAM frame, or a STREAM frame with the FIN
-flag set also causes a stream to become "half-closed".
-
-An endpoint might receive MAX_STREAM_DATA or STREAM_BLOCKED frames on
-peer-initiated streams that are "idle" if there is loss or reordering of
-packets.  Receiving these frames also causes the stream to become "open".
-
-An endpoint MUST NOT send a STREAM or RST_STREAM frame for a stream ID that is
-higher than the peers advertised maximum stream ID (see
-{{frame-max-stream-id}}).
-
+Sending a STREAM or RST_STREAM frame causes the identified stream to become
+"open" for a sending endpoint.  New streams use the next available stream
+identifier, as described in {{stream-id}}.  An endpoint MUST NOT send a STREAM
+or RST_STREAM frame for a stream ID that is higher than the peers advertised
+maximum stream ID (see {{frame-max-stream-id}}).

Can you say what the other side does in this case?

>  
-### half-closed (remote) {#state-hc-remote}
+A stream becomes "closed" at the receiving endpoint when all data is received
+and a STREAM frame with a FIN flag is received or when a RST_STREAM frame is
+received.

I think it would be helpful to reorder these, i.e., 
"A stream becomes "closed" at the receiving endpoint 
when an RST_STREAM frame is received. when all data is received
and a STREAM frame with a FIN flag is received"

Because as-is it is ambiguous if you just read this section in isolation


> -
-### closed {#state-closed}
-
-The "closed" state is the terminal state for a stream.
-
-Once a stream reaches this state, no frames can be sent that mention the stream.
-Reordering might cause frames to be received after closing, see
-{{state-hc-remote}}.
+An endpoint could continue receiving frames for the stream even after the stream
+is closed for sending if packets are reordered.  At the receiving endpoint,
+retransmissions of data in STREAM frames, or retransmissions of STREAM_BLOCKED
+and RST_STREAM frames might arrive after the stream becomes "closed".  At the
+sending endpoint, MAX_STREAM_DATA frames might be received after the stream
+becomes "closed".  Frames received on a "closed" stream SHOULD be discarded.  An
+endpoint MAY choose to limit the period over which it ignores frames and treat
+frames that arrive after this time as being in error.

This graf is a bit confusing, because you first say that you should discard frames on a closed stream and then you say that you may instead ignore them only after some time. Why would I want to do that? What would I do if I didn't ignore them?

> -treated as a connection error of type HTTP_RST_CONTROL_STREAM.  When a message
-control stream terminates cleanly, if the last frame on the stream was
-truncated, this MUST be treated as a connection error (see HTTP_MALFORMED_* in
-{{http-error-codes}}).
-
-Pairs of streams must be utilized sequentially, with no gaps.  The data stream
-is opened at the same time as the message control stream is opened and is closed
-after transferring the body.  The data stream is closed immediately after
-sending the request headers if there is no body.
+the stream management. An HTTP request/response consumes a pair of streams in
+each direction.
+
+Aside from the connection control stream, the beginning of each stream starts
+with a short stream header.  This stream header is used to identify the type of
+stream and to link the stream to another stream as necessary.  See
+{{stream-header}} for more details on the stream header.

Nit: I would put this cite after the first sentence.

> -sending the request headers if there is no body.
+the stream management. An HTTP request/response consumes a pair of streams in
+each direction.
+
+Aside from the connection control stream, the beginning of each stream starts
+with a short stream header.  This stream header is used to identify the type of
+stream and to link the stream to another stream as necessary.  See
+{{stream-header}} for more details on the stream header.
+
+\[Editor's Note: the following is clearly busted.  Requests and responses can't
+reliably be cancelled as a result of this change.  We will need QPACK/QCRAM for
+this to work.] Request, response and push streams contain HPACK data which
+manipulates connection-level state.  Therefore, these streams MUST NOT be closed
+with a stream-level error.
+
+Streams must be opened sequentially, with no gaps.  Data streams SHOULD be

Isn't this already a QUIC invariant?

> -sending the request headers if there is no body.
+the stream management. An HTTP request/response consumes a pair of streams in
+each direction.
+
+Aside from the connection control stream, the beginning of each stream starts
+with a short stream header.  This stream header is used to identify the type of
+stream and to link the stream to another stream as necessary.  See
+{{stream-header}} for more details on the stream header.
+
+\[Editor's Note: the following is clearly busted.  Requests and responses can't
+reliably be cancelled as a result of this change.  We will need QPACK/QCRAM for
+this to work.] Request, response and push streams contain HPACK data which
+manipulates connection-level state.  Therefore, these streams MUST NOT be closed
+with a stream-level error.
+
+Streams must be opened sequentially, with no gaps.  Data streams SHOULD be

"New streams use the next available stream"

> +~~~~~
+
+After the stream header, response streams contain a sequence of frames (see
+{{http-framing-layer}}).
+
+If a response stream header identifies a stream that is not a request stream, or
+it references a request stream that already has an associated response stream,
+the connection MUST be terminated with an HTTP_UNMATCHED_RESPONSE error.
+
+
+### Data Streams {#stream-data}
+
+Data streams are opened by both client and server.  Data streams carry the
+payload body of requests or responses.
+
+The stream header for a response stream contains the type octet, which is set to

I think you mean "data stream"

> +|   Type (8)    |
++-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+|                        Stream ID (32)                         |
++-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+~~~~~
+
+After the stream header, data streams contain the octets of the payload body.
+Data streams do not carry frames.
+
+Data streams MUST reference a request or response stream.  The referenced stream
+MUST include a HAS_BODY frame.  If a data stream is received that references a
+stream of a different type, the referenced stream does not include a HAS_BODY
+frame, or another data stream already references the identified steam, then the
+connection MUST be terminated with a HTTP_UNMATCHED_BODY error.  Note that due
+to reordering of packets, the referenced stream or HAS_BODY frame might not have
+been received when the data stream is opened.

So, the implementation here is:

- If has_body frame received, then process stream
- Else, block until either

  (a) has_body frame received, in which case process stream
  (b) fin/rst received in which case send error

> -
-An HTTP request/response exchange fully consumes a pair of streams. After
-sending a request, a client closes the streams for sending; after sending a
-response, the server closes its streams for sending and the QUIC streams are
-fully closed.
+response may contain zero or more header blocks on the response stream
+containing the message headers of informational (1xx) HTTP responses (see
+{{!RFC7230}}, Section 3.2 and {{!RFC7231}}, Section 6.2).
+
+A message that contains a payload body includes a HAS_BODY frame immediately
+after the first header block.  This indicates that another stream will include
+the payload body.  A data stream is opened to carry the payload body.  The data
+stream MUST be closed immediately after sending the body.  If the message does
+not contain a body, a data stream MUST NOT be opened and the HAS_BODY message
+MUST NOT be sent.  The "chunked" transfer encoding defined in Section 4.1 of
+{{!RFC7230}} MUST NOT be used.

I wonder if it would make more sense to have the pointer to the body stream here rather than a back pointer for the dependency

> @@ -401,6 +538,17 @@ All frames have the following format:
 
 ## Frame Definitions {#frames}
 
+### HAS_BODY {#frame-has-body}
+
+The HAS_BODY frame (type=0x0) indicates that the HTTP message contains a payload
+body.  It is sent after the initial header block on a request, response, or push
+stream.
+
+The HAS_BODY frame has no flags and is always empty.  A HAS_BODY frame with a
+non-empty payload or flags that are set MUST be treated as a
+HTTP_MALFORMED_HAS_BODY error.

Might it be easier to just put this field in the header block

> +
+  Request Stream/Push ID:
+  : A 32-bit stream ID or Push ID that identifies the request.  If the PUSH flag
+    is set, this refers to a Push ID included in a PUSH_PROMISE frame (see
+    {{frame-push-promise}}); if the PUSH flag is cleared, this refers to a
+    request by its stream ID.
+
+  Error Code:
+  : A 32-bit error code indicating why the response is not desired.
+
+A CANCEL_REQUEST frame always contains an 8 octet payload.  A server MUST treat
+a CANCEL_REQUEST frame payload that is other than 8 octets in length, or a
+CANCEL_REQUEST frame that identifies a stream that doesn't carry a valid request
+as an HTTP_MALFORMED_CANCEL_REQUEST error.  Note however that due to packet
+reordering, a CANCEL_REQUEST frame could arrive before the request stream that
+it references is opened.

This is weird text. Isn't it always the case that if you receive an unexpected frame structure you are sad?

> +ID MUST be treated as a HTTP_MALFORMED_PRIORITY error.
+
+The length of a PRIORITY frame is 9 octets.  A PRIORITY frame with any other
+length MUST be treated as a connection error of type HTTP_MALFORMED_PRIORITY.
+
+
+### CANCEL_REQUEST {#frame-cancel-request}
+
+The CANCEL_REQUEST frame (type=0x3) is used to request cancellation of a request
+prior to the response stream being created.  This frame can be used to cancel
+both outstanding requests initiated by a client and server push requests that
+are initiated with a PUSH_PROMISE.
+
+The frame identifies a client-initiated request by the stream ID that carried
+the request; a server push request is identified by Push ID (see
+{{frame-push-promise}}).

Don't you mean "request ID" that's what you are using for dependencies

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/643#pullrequestreview-45794120