Subject: [quicwg/base-drafts] Don't wait for FIN to process complete messages (#2003)

From: Mike Bishop <[email-masked]>
Date: Wed, 14 Nov 2018 19:29:20 +0000 (UTC)

Fixes #1972.

While the discussion on the issue wasn't conclusive in either direction, I think the presence of CONNECT and early response means servers will need to be processing HTTP messages as they arrive, rather than waiting for the FIN as an end-of-message signal.  Also, if the message is large, the alternative risks deadlock on a large POST.

This PR expands on the "stream state change isn't part of the message framing" language to clarify that both peers MUST be able to process complete HTTP messages regardless of whether the stream has been closed or not.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * FIN is irrelevant

-- File Changes --

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

-- Patch Links --

https://github.com/quicwg/base-drafts/pull/2003.patch
https://github.com/quicwg/base-drafts/pull/2003.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/2003

---

From: Ryan Hamilton <[email-masked]>
Date: Wed, 14 Nov 2018 11:52:10 -0800

Maybe I'm not thinking about this the right way, but if a body does not have a content-length, then how is the end of the message determined, if not by the end of the stream?

-- 
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/2003#issuecomment-438795523

---

From: Mike Bishop <[email-masked]>
Date: Wed, 14 Nov 2018 20:06:41 +0000 (UTC)

That's true, and should have been obvious.  Ugh.

Let's take that point back to the issue; this PR might be DOA.

-- 
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/2003#issuecomment-438800244

---

From: Martin Thomson <[email-masked]>
Date: Wed, 14 Nov 2018 17:15:33 -0800

martinthomson requested changes on this pull request.

 A response MAY consist of multiple messages when and only when one or more
 informational responses (1xx, see {{!RFC7231}}, Section 6.2) precede a final
 response to the same request.  Non-final responses do not contain a payload body
 or trailers.
 
 An HTTP request/response exchange fully consumes a bidirectional QUIC stream.
-After sending a request, a client closes the stream for sending; after sending a
-final response, the server closes the stream for sending and the QUIC stream is
-fully closed.  Requests and responses are considered complete when the
-corresponding QUIC stream is closed in the appropriate direction.
+After sending a request, a client SHOULD close the stream for sending; after
+sending a final response, the server SHOULD close the stream for sending. At
+this point, the QUIC stream is fully closed.
+
+Changes to the state of a request stream do not directly affect message
+processing.  Endpoints MUST process complete HTTP messages without relying on
+stream closure as an end-of-message signal.  For example, servers do not abort a

This is wrong.  The statement to make is:

The end of a stream (in either direction) indicates the end of a message.
Incomplete messages can be processed, and ideally endpoints do that after receiving a complete header block.

-- 
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/2003#pullrequestreview-175154084

---

From: Mike Bishop <[email-masked]>
Date: Mon, 19 Nov 2018 11:23:21 -0800

@MikeBishop pushed 1 commit.

df5d86a  Close is end, consume partials

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2003/files/7743d02e38d5f3b3654d7e328a09fbc06dd4cf51..df5d86a72301cbdb5d24024cf9706424913a05f8

---

From: Mike Bishop <[email-masked]>
Date: Mon, 19 Nov 2018 11:24:58 -0800

Updated with Martin's suggestion:  End of stream data is end of message, but you SHOULD be consuming partial messages without waiting for the end.  End of stream with a partial message is an error.

-- 
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/2003#issuecomment-440011948

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 19 Nov 2018 22:24:28 +0000 (UTC)

kazuho commented on this pull request.

I think we need to use "MUST" here. Otherwise, a peer can never notice the end of the message unless trailers are used.

Other than that, the PR looks good to me. I like the language that encourages endpoints to start processing early.

-- 
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/2003#pullrequestreview-176525139

---

From: Martin Thomson <[email-masked]>
Date: Mon, 19 Nov 2018 17:08:28 -0800

martinthomson approved this pull request.

Iff you take the suggestions.

```suggestion
After sending a request, a client MUST close the stream for sending; after
```

This is me agreeing with @kazuho.

```suggestion
sending a final response, the server MUST close the stream for sending. At
```

The nice thing is that we never really say "when".

-- 
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/2003#pullrequestreview-176566565

---

From: Mike Bishop <[email-masked]>
Date: Tue, 20 Nov 2018 18:32:18 +0000 (UTC)

eb58201  Apply suggestions from code review

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2003/files/df5d86a72301cbdb5d24024cf9706424913a05f8..eb58201482ac4ed045591927e81ae6dcf131aecc

---

From: Mike Bishop <[email-masked]>
Date: Wed, 21 Nov 2018 13:54:06 -0800

@RyanAtGoogle and @martinduke, I'd ideally like your input before I merge this.

-- 
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/2003#issuecomment-440822664

---

From: Ryan Hamilton <[email-masked]>
Date: Wed, 21 Nov 2018 14:00:02 -0800

RyanAtGoogle approved this pull request.

Looks great to me!

-- 
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/2003#pullrequestreview-177443139

---

From: martinduke <[email-masked]>
Date: Sun, 25 Nov 2018 20:37:13 -0800

Thanks, this addresses my concern. I don't see how the client MUST has any force given that there's no time limit; perhaps it ought to be a SHOULD?

-- 
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/2003#issuecomment-441516866

---

From: Martin Thomson <[email-masked]>
Date: Sun, 25 Nov 2018 20:47:46 -0800

martinthomson commented on this pull request.

 An HTTP request/response exchange fully consumes a bidirectional QUIC stream.
-After sending a request, a client closes the stream for sending; after sending a
-final response, the server closes the stream for sending and the QUIC stream is
-fully closed.  Requests and responses are considered complete when the
-corresponding QUIC stream is closed in the appropriate direction.
+After sending a request, a client MUST close the stream for sending; after
+sending a final response, the server MUST close the stream for sending. At
+this point, the QUIC stream is fully closed.

To @martinduke's comment, maybe it is enough to say "A message is terminated when the stream is closed."  Instead of all these words.

-- 
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/2003#pullrequestreview-178132467

---

From: Martin Thomson <[email-masked]>
Date: Sun, 25 Nov 2018 20:53:54 -0800

Oh, and now I realize that this is what it said before...  Not sure whether this is an improvement then.

-- 
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/2003#discussion_r236121536

---

From: Mike Bishop <[email-masked]>
Date: Mon, 26 Nov 2018 09:30:12 -0800

MikeBishop commented on this pull request.

Yeah, the original provoking issue was that he didn't FIN, the server didn't respond, and both are doing totally spec-compliant things.  With this language, it's at least clarified that while the server SHOULD process that request on receipt, it isn't required to until the stream closes, so the client needs to close promptly.

-- 
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/2003#discussion_r236352508

---

From: Mike Bishop <[email-masked]>
Date: Mon, 26 Nov 2018 17:32:59 +0000 (UTC)

You're right that a MUST with no deadline is somewhat toothless, but I don't think it's SHOULD-level.  (If we were to fully expand it, you MUST close it eventually and SHOULD do so promptly.)

I think the force is actually in the *server's* SHOULD:  While the server SHOULD process messages before you FIN, it's not obligated to.  Therefore, if you want to ensure a response, you'll need to close promptly as a practical matter.

-- 
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/2003#issuecomment-441727095

---

From: martinduke <[email-masked]>
Date: Mon, 26 Nov 2018 13:31:48 -0800

It sounds like there's a blanket rule except for one special flower - HTTP_CONNECT. So perhaps we should just call out the exception rather than dancing around it, e.g.

"With the exception of requests that contain the CONNECT method, senders use a FIN to indicate the end of an HTTP request or response.  Absent CONNECT, senders MUST NOT make the sending of FIN dependent on receipt of a response from the peer. 

Absent CONNECT, senders MUST NOT send additional data on the stream after the request or response.

However, due to the possible presence of a CONNECT method, and the opportunity to send HTTP_EARLY_DATA, servers SHOULD begin processing partial HTTP messages once enough of the message has been received to make progress.  If a client stream terminates without enough of the HTTP message to provide a complete response, the server SHOULD abort its response with the error code HTTP_INCOMPLETE_REQUEST."

This MUST strikes me as a testable condition.

-- 
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/2003#issuecomment-441805881

---

From: Mike Bishop <[email-masked]>
Date: Mon, 26 Nov 2018 14:11:17 -0800

Formally, the data transferred on the TCP connection is the body of the HTTP request, so it's not a special flower in that sense.  But it's special in that it's the only case where the server MUST process the headers before the end of the "message."

The requirement that the client MUST NOT make FIN dependent on receiving anything would be a good wording improvement.

-- 
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/2003#issuecomment-441819089

---

From: martinduke <[email-masked]>
Date: Tue, 27 Nov 2018 02:50:12 +0000 (UTC)

Alright, how about this:

"Senders use a FIN to indicate the end of an HTTP request or response. Absent a CONNECT method in the request, senders MUST NOT make the sending of FIN dependent on receipt of a response from the peer.

I'm open to making the server requirement a similar MUST -- "servers MUST NOT be dependent on a FIN to start processing..." if people think that would help avoid CONNECT deadlocks.

-- 
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/2003#issuecomment-441878674

---

From: Martin Thomson <[email-masked]>
Date: Tue, 27 Nov 2018 02:53:36 +0000 (UTC)

Not seeing how early data (or HTTP_EARLY_DATA, whatever that is) has any bearing on this.  I would drop the first particle from the second paragraph and concentrate on the recommendations: process when you can, use HTTP_INCOMPLETE_REQUEST if you need to.

I don't think we need a MUST attached to the recommendations - if you don't follow the recommendations, then it's your loss.

-- 
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/2003#issuecomment-441879268

---

From: martinduke <[email-masked]>
Date: Mon, 26 Nov 2018 19:07:51 -0800

> or HTTP_EARLY_DATA, whatever that is

Argh, I meant HTTP_EARLY_RESPONSE.

As for the final wording, I am happy with whatever you guys decide.

-- 
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/2003#issuecomment-441882433

---

From: Mike Bishop <[email-masked]>
Date: Tue, 27 Nov 2018 16:05:37 -0800

dda6b41  Add a MUST, call out CONNECT

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2003/files/eb58201482ac4ed045591927e81ae6dcf131aecc..dda6b41538737d1bee15893e3f0c7d74f2cc014d

---

From: Mike Bishop <[email-masked]>
Date: Wed, 28 Nov 2018 00:05:53 +0000 (UTC)

Did something along those lines -- see what you think.

-- 
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/2003#issuecomment-442266464

---

From: Martin Thomson <[email-masked]>
Date: Tue, 27 Nov 2018 16:55:05 -0800

WFM

-- 
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/2003#pullrequestreview-179081376

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 27 Nov 2018 17:29:00 -0800

An HTTP request/response exchange fully consumes a bidirectional QUIC stream.
-After sending a request, a client closes the stream for sending; after sending a
-final response, the server closes the stream for sending and the QUIC stream is
-fully closed.  Requests and responses are considered complete when the
-corresponding QUIC stream is closed in the appropriate direction.
+After sending a request, a client MUST close the stream for sending; clients
+MUST NOT make stream closure dependent on receiving a response to their request,
+unless using the CONNECT method (see {{the-connect-method}}). After sending a

This sentence is confusing to me, because it is unclear what "unless" applies to.

How about changing the sentence to: _Unless using the CONNECT method (see {{the-connect-method}}), a client MUST close the stream for sending after sending a request. Clients MUST NOT make stream closure dependent on receiving a response to the request._

-- 
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/2003#pullrequestreview-179088319

---

From: martinduke <[email-masked]>
Date: Tue, 27 Nov 2018 19:27:04 -0800

martinduke approved this pull request.

I like @kazuho's working slightly better, but aside from that, ship 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/2003#pullrequestreview-179110158

---

From: Mike Bishop <[email-masked]>
Date: Wed, 28 Nov 2018 10:25:55 -0800

And this highlights the confusion, because I meant the "unless" to be on the opposite clause.  :-)

With the formulation we've already got, the entire CONNECT stream is one large HTTP request message, so it doesn't make sense to say you don't need to close a stream after the request in that case.  However, stream closure most definitely depends on the response you get back to the headers of your CONNECT request.

-- 
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/2003#discussion_r237205911

---

From: Mike Bishop <[email-masked]>
Date: Wed, 28 Nov 2018 18:28:07 +0000 (UTC)

a67f74d  Which unless?

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2003/files/dda6b41538737d1bee15893e3f0c7d74f2cc014d..a67f74d5121f01445b2de13761462cc3a4f45bf3

---

From: Mike Bishop <[email-masked]>
Date: Wed, 28 Nov 2018 10:29:51 -0800

Merged #2003 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/2003#event-1993246867