Subject: [quicwg/base-drafts] Specify behavior on incomplete requests (#1643)

From: Mike Bishop <[email-masked]>
Date: Wed, 08 Aug 2018 17:00:36 -0700

Fixes #1596 (and an oversight I caught in passing) by specifying server behavior if it reaches the end of readable data on a stream and hasn't parsed a full HTTP request yet.  There are a couple cases here, and I think this generically covers all the interesting ones.

- **Client sends FIN early.**  Server sees truncated request, but non-error end of stream.
- **Client sends RST_STREAM part-way through request.**  Server sees truncated request with read error.
- **Client sends a RST_STREAM (but not a STOP_SENDING) after sending a fully-formed request.**
    - **...and packet loss occurred.**  No retransmissions, so same as sending it part-way through request.
    - **...after the server has read the request and generated / begun generating a response.**  The server probably never realizes and responds merrily.  If you didn't want it, you should have sent a STOP_SENDING.
    - **...but before the server has read all the data out of the buffer.**  Depends on implementation; transport might discard unread data or deliver all the data anyway.  Either way, this is identical to one of the two previous cases.

We already discuss cancelling requests, so I added a suggestion that clients go ahead and include a STOP_SENDING if they reset their request stream.
You can view, comment on, or merge this pull request online at:

  https://github.com/quicwg/base-drafts/pull/1643

-- Commit Summary --

  * HTTP_EARLY_RESPONSE not defined (Whoops\!)
  * Define HTTP_INCOMPLETE_REQUEST
  * Recommend bidirectional cancellation for clients

-- File Changes --

    M draft-ietf-quic-http.md (30)

-- Patch Links --

https://github.com/quicwg/base-drafts/pull/1643.patch
https://github.com/quicwg/base-drafts/pull/1643.diff

-- 
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/1643

---

From: Martin Thomson <[email-masked]>
Date: Wed, 08 Aug 2018 19:25:17 -0700

martinthomson commented on this pull request.

Something about the special rules here seems just wrong.  I have an alternative approach for this that I think is neater.

> @@ -317,8 +318,13 @@ client abort transmission of a request without error by triggering a QUIC
 STOP_SENDING with error code HTTP_EARLY_RESPONSE, sending a complete response,
 and cleanly closing its streams. Clients MUST NOT discard complete responses as
 a result of having their request terminated abruptly, though clients can always
-discard responses at their discretion for other reasons.  Servers MUST NOT
-abort a response in progress as a result of receiving a solicited RST_STREAM.
+discard responses at their discretion for other reasons.
+
+Servers MUST NOT abort a response in progress as a result of receiving a
+solicited RST_STREAM (i.e. with error code STOPPING).  If the request stream

missing comma after "i.e."

> @@ -317,8 +318,13 @@ client abort transmission of a request without error by triggering a QUIC
 STOP_SENDING with error code HTTP_EARLY_RESPONSE, sending a complete response,
 and cleanly closing its streams. Clients MUST NOT discard complete responses as
 a result of having their request terminated abruptly, though clients can always
-discard responses at their discretion for other reasons.  Servers MUST NOT
-abort a response in progress as a result of receiving a solicited RST_STREAM.
+discard responses at their discretion for other reasons.
+
+Servers MUST NOT abort a response in progress as a result of receiving a

Perhaps the way to do this is to say:

Cancellation of a request stream does not prevent a server from sending a response.  A client that does not want a response SHOULD send a STOP_SENDING frame.  A server does not need to take receipt of a RST_STREAM as a signal that it cannot provide a response, though it might be unable to do so.  If a stream is reset by the client and the server is unable to respond as a result, it can treat that as a stream error of type HTTP_INCOMPLETE_REQUEST.

This is a different structure - it says that any STOP_SENDING and RST_STREAM on the request side of a stream only cancel the request and not the response.  But I think that this is neater - it avoids all of the coupling you have going on here.

-- 
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/1643#pullrequestreview-144651478

---

From: Mike Bishop <[email-masked]>
Date: Thu, 09 Aug 2018 10:21:35 -0700

MikeBishop commented on this pull request.

I think that's basically what I was going for (the error at the server is that it doesn't have a complete request), but that's a clearer way to state it.

-- 
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/1643#discussion_r209012113

---

From: Mike Bishop <[email-masked]>
Date: Thu, 09 Aug 2018 10:27:42 -0700

@MikeBishop pushed 1 commit.

482badb  More separated

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1643/files/a9c73a0fd782cbd70b4822a998dc74c2b2aa54f5..482badb97213ac9ad02b08f18f1fb115c4f35291

---

From: Martin Thomson <[email-masked]>
Date: Thu, 09 Aug 2018 18:51:33 -0700

martinthomson approved this pull request.

Yeah, this is good.  Mostly just nits.  The one of fully-formed might be a deal-breaker if you don't fix it.

> @@ -317,8 +318,14 @@ client abort transmission of a request without error by triggering a QUIC
 STOP_SENDING with error code HTTP_EARLY_RESPONSE, sending a complete response,
 and cleanly closing its streams. Clients MUST NOT discard complete responses as
 a result of having their request terminated abruptly, though clients can always
-discard responses at their discretion for other reasons.  Servers MUST NOT
-abort a response in progress as a result of receiving a solicited RST_STREAM.
+discard responses at their discretion for other reasons.
+
+Changes to the state of a request stream, including receiving a RST_STREAM
+(regardless of error code), do not affect the state of the server's response.
+Servers MUST NOT abort a response in progress solely due to a state change on
+the request stream. However, if the request stream terminates without containing
+a fully-formed HTTP request, the server SHOULD abort its response with the error

s/fully-formed/usable/ perhaps.

A server can often produce a response based purely on header fields.  That's the case that we specifically want to support with STOP_SENDING (a 404 on a POST).  This implies that the message needs to be complete, which isn't necessary.

> @@ -317,8 +318,14 @@ client abort transmission of a request without error by triggering a QUIC
 STOP_SENDING with error code HTTP_EARLY_RESPONSE, sending a complete response,
 and cleanly closing its streams. Clients MUST NOT discard complete responses as
 a result of having their request terminated abruptly, though clients can always
-discard responses at their discretion for other reasons.  Servers MUST NOT
-abort a response in progress as a result of receiving a solicited RST_STREAM.
+discard responses at their discretion for other reasons.
+
+Changes to the state of a request stream, including receiving a RST_STREAM
+(regardless of error code), do not affect the state of the server's response.
+Servers MUST NOT abort a response in progress solely due to a state change on

I don't think that this is a normative requirement on reflection.  Maybe just "Servers do not abort a response..." or "Servers are not expected to abort a response..."

> @@ -317,8 +318,14 @@ client abort transmission of a request without error by triggering a QUIC
 STOP_SENDING with error code HTTP_EARLY_RESPONSE, sending a complete response,
 and cleanly closing its streams. Clients MUST NOT discard complete responses as
 a result of having their request terminated abruptly, though clients can always
-discard responses at their discretion for other reasons.  Servers MUST NOT
-abort a response in progress as a result of receiving a solicited RST_STREAM.
+discard responses at their discretion for other reasons.
+
+Changes to the state of a request stream, including receiving a RST_STREAM
+(regardless of error code), do not affect the state of the server's response.

s/(regardless of error code)/with any error code/

-- 
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/1643#pullrequestreview-145081955

---

From: Mike Bishop <[email-masked]>
Date: Fri, 10 Aug 2018 09:37:39 -0700

c777723  Usable

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1643/files/482badb97213ac9ad02b08f18f1fb115c4f35291..c77772339ed61d4b797434063ae233ac6645acb4

---

From: Mike Bishop <[email-masked]>
Date: Fri, 10 Aug 2018 16:31:55 -0700

59da706  (that too)

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1643/files/c77772339ed61d4b797434063ae233ac6645acb4..59da7060f81ef5f764d3b71ca6a02fe594adc34e

---

From: Mike Bishop <[email-masked]>
Date: Tue, 14 Aug 2018 15:14:23 -0700

Merged #1643 into master.

-- 
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/1643#event-1788941604