Subject: [quicwg/base-drafts] Introduce HTTP_REQUEST_REJECTED (#2325)

From: Mike Bishop <[email-masked]>
Date: Wed, 09 Jan 2019 19:17:23 +0000 (UTC)

Fixes #2106.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Introduce HTTP_REQUEST_REJECTED

-- File Changes --

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

-- Patch Links --

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

---

From: Dmitri Tikhonov <[email-masked]>
Date: Wed, 09 Jan 2019 20:50:26 +0000 (UTC)

dtikhonov commented on this pull request.

>  client and server to agree on which requests were accepted prior to the
 connection shutdown.  This identifier MAY be lower than the stream limit
 identified by a QUIC MAX_STREAM_ID frame, and MAY be zero if no requests were
 processed.  Servers SHOULD NOT increase the QUIC MAX_STREAM_ID limit after
 sending a GOAWAY frame.
 
+<<<<<<< 64585a30a8ddc7a766080499cd44cd5d4206fe1d

Conflict

-- 
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/2325#pullrequestreview-190895378

---

From: David Schinazi <[email-masked]>
Date: Wed, 09 Jan 2019 21:13:10 +0000 (UTC)

This change sounds good to me, but to make sure I understand: is the only purpose of this change to split HTTP_REQUEST_CANCELLED in two; client to server stays HTTP_REQUEST_CANCELLED and server to client now becomes HTTP_REQUEST_REJECTED? And is the goal to add clarity when parsing connection logs later?

-- 
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/2325#issuecomment-452837699

---

From: Mike Bishop <[email-masked]>
Date: Wed, 09 Jan 2019 21:18:25 +0000 (UTC)

@MikeBishop pushed 1 commit.

27ce67d8b2377ee86968bad15e7ba05f432a128a  Missed a merge conflict

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2325/files/5e85285597355961c4b1c002a6bd77938e3715cb..27ce67d8b2377ee86968bad15e7ba05f432a128a

---

From: Mike Bishop <[email-masked]>
Date: Wed, 09 Jan 2019 13:40:39 -0800

@DavidSchinazi, basically.  In the previous text, CANCELLED had specific semantics when sent server->client.  However, with the change to make RESET_STREAM reflect back the error code in STOP_SENDING, the client->server STOP_SENDING(CANCELLED) would provoke a RESET_STREAM(CANCELLED) in the server->client direction with that error code that didn't necessarily match those semantics.

Hence the split -- one code has specific semantics when sent by a server, and clients are instructed to use the other code.

(I would prohibit clients from sending it, but if the server sends a STOP_SENDING, the client would reflect it back and violate the prohibition, so....)

-- 
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/2325#issuecomment-452872516

---

From: Martin Thomson <[email-masked]>
Date: Wed, 09 Jan 2019 13:58:18 -0800

martinthomson approved this pull request.

A few suggestions, but this is a good change.

> @@ -963,20 +963,20 @@ for each header field.
 
 ### Request Cancellation
 
-Either client or server can cancel requests by aborting the stream (QUIC
-RESET_STREAM and/or STOP_SENDING frames, as appropriate) with an error code of
+Clients can cancel requests by aborting the stream (QUIC RESET_STREAM and/or
+STOP_SENDING frames, as appropriate) with an error code of
 HTTP_REQUEST_CANCELLED ({{http-error-codes}}).  When the client cancels a
 response, it indicates that this response is no longer of interest.
 Implementations SHOULD cancel requests by aborting both directions of a stream.

Missing from either this paragraph or the next:  A server that receives a STOP_SENDING with the HTTP_REQUEST_CANCELLED error code MAY use the HTTP_REQUEST_REJECTED error code in response to indicate that no processing occurred.

> @@ -1867,8 +1871,11 @@ FRAME_SIZE_ERROR (0x6):
 : HTTP_MALFORMED_FRAME error codes defined in {{http-error-codes}}.
 
 REFUSED_STREAM (0x7):
-: Not applicable, since QUIC handles stream management.  Would provoke a
-  STREAM_ID_ERROR from the QUIC layer.
+: When used to indicate that a request was not processed, HTTP_REQUEST_REJECTED
+  in {{http-error-codes}}.
+: When used to indicate that an active stream limit was exceeded, not applicable

I had a little trouble parsing this bit.  Maybe reorder the sentences:

> HTTP_REQUEST_REJECTED (in {{http-error-codes}}) is used to indicate when requests were not processed.  Otherwise, not applicable because QUIC handles stream management.  A STREAM_ID_ERROR at the QUIC layer is used for streams that are improperly opened.

-- 
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/2325#pullrequestreview-190956397

---

From: Mike Bishop <[email-masked]>
Date: Wed, 09 Jan 2019 22:08:18 +0000 (UTC)

b756df2628d40aa91c5afd2ae92e8f193c1936a8  Martin's edits

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2325/files/27ce67d8b2377ee86968bad15e7ba05f432a128a..b756df2628d40aa91c5afd2ae92e8f193c1936a8

---

From: afrind <[email-masked]>
Date: Thu, 10 Jan 2019 08:33:10 -0800

afrind commented on this pull request.

>  ({{request-cancellation}}).  Clients SHOULD reset any streams above this ID with
-the error code HTTP_REQUEST_CANCELLED.  Servers MAY also cancel requests on
+the error code HTTP_REQUEST_REJECTED.  Servers MAY also reject requests on

Why have the client send REJECTED in this case?

-- 
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/2325#pullrequestreview-191297248

---

From: David Schinazi <[email-masked]>
Date: Thu, 10 Jan 2019 09:13:45 -0800

DavidSchinazi commented on this pull request.

 ### Request Cancellation

Wouldn't that defeat the purpose of this PR? If a server receives STOP_SENDING with HTTP_REQUEST_CANCELLED, it doesn't reject the request, it acts on the client's cancellation. In that case I think it would make more sense for the server to use HTTP_REQUEST_CANCELLED.

-- 
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/2325#discussion_r246846343

---

From: Mike Bishop <[email-masked]>
Date: Thu, 10 Jan 2019 09:31:19 -0800

MikeBishop commented on this pull request.

Because GOAWAY already tells the client that the server didn't act on these higher streams; this is effectively the client's agreement that these streams were rejected by GOAWAY.  (There's probably also a STOP_SENDING en route with the same 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/2325#discussion_r246852731

---

From: Mike Bishop <[email-masked]>
Date: Thu, 10 Jan 2019 09:54:31 -0800

### Request Cancellation

>From `-transport`:
> An endpoint SHOULD copy the error code from the STOP_SENDING frame, but MAY use any application error code.

_CANCELLED would be the default behavior; this text merely says that _REJECTED is also acceptable.  (And the transport draft says that the server MAY use any error code, so this is really just a restatement.)

The subtle difference would be an indication of whether the cancelled request had begun to process (and therefore might have had side-effects) before the cancellation took place.

-- 
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/2325#discussion_r246860205

---

From: Mike Bishop <[email-masked]>
Date: Thu, 10 Jan 2019 10:01:17 -0800

5fbe7f5959653fdd7ca8c28313783f2e81ab780d  Leftover > instead of >=

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2325/files/b756df2628d40aa91c5afd2ae92e8f193c1936a8..5fbe7f5959653fdd7ca8c28313783f2e81ab780d

---

From: Martin Thomson <[email-masked]>
Date: Thu, 10 Jan 2019 13:31:58 -0800

martinthomson commented on this pull request.

 ### Request Cancellation

Right. The trick here is that a cancelled request can't be automatically retried.  A rejected one can be.  So using _REJECTED is preferable.

-- 
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/2325#discussion_r246928366

---

From: David Schinazi <[email-masked]>
Date: Thu, 10 Jan 2019 14:07:14 -0800

### Request Cancellation

Ah, I see. I agree. Hmmm not sure how to clarify the text so others aren't confused the same way I was though.

-- 
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/2325#discussion_r246938562

---

From: afrind <[email-masked]>
Date: Thu, 10 Jan 2019 15:31:48 -0800

This seems weird.  I think I prefer the client sending CANCELLED.

-- 
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/2325#discussion_r246959697

---

From: Martin Thomson <[email-masked]>
Date: Thu, 10 Jan 2019 15:36:34 -0800

Oh, I missed that.  Yes.  The client CAN'T send _REJECTED.  Because if the server echoes it (as they are allowed to), then the client might conclude that nothing was processed.  We need a prohibition against clients using _REJECTED.

> Clients MUST NOT use HTTP_REQUEST_REJECTED with STOP_SENDING.

-- 
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/2325#discussion_r246960609

---

From: Mike Bishop <[email-masked]>
Date: Thu, 10 Jan 2019 16:12:08 -0800

It's not that the client can't, because those streams *haven't* been processed, per GOAWAY.  But yes, we should add a prohibition on its use in other cases at least.

-- 
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/2325#discussion_r246967388

---

From: Mike Bishop <[email-masked]>
Date: Tue, 15 Jan 2019 16:10:42 -0800

8adb75ed0483cb720412837879363a236ecc37c9  Clients don't reject, they cancel

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2325/files/5fbe7f5959653fdd7ca8c28313783f2e81ab780d..8adb75ed0483cb720412837879363a236ecc37c9

---

From: Mike Bishop <[email-masked]>
Date: Tue, 15 Jan 2019 16:12:24 -0800

Merged #2325 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/2325#event-2076115953