Subject: [quicwg/base-drafts] all HTTP/3 protocol violations should lead to connection errors (#2510)

From: Kazuho Oku <[email-masked]>
Date: Fri, 08 Mar 2019 23:21:03 -0800

In reply to https://github.com/quicwg/base-drafts/pull/2508#issuecomment-471151993:
> @kazuho, that's a good question. Previously, the philosophy has been that connection errors are ones which screw up the state so badly you can't trust the connection to proceed correctly. It's a reasonable, but not bulletproof, argument that a peer which doesn't obey the protocol has the same implication -- you can't trust the connection.
> 
> If you see other protocol violations which are stream-local errors, please file issues and we'll discuss.

I did a quick search for "stream error" and found a few that discusses about protocol violation. This PR changes "connection error."
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * all protocol violations should lead to connection errors

-- File Changes --

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

-- Patch Links --

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

---

From: ianswett <[email-masked]>
Date: Sat, 09 Mar 2019 01:03:18 -0800

I wrote an almost identical PR #2512 I'll now close, but I think we want to fix line 331 and 630 as I do in my PR?  On those lines, we didn't specify what type of error it was.

-- 
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/2510#issuecomment-471159921

---

From: ianswett <[email-masked]>
Date: Sat, 09 Mar 2019 01:05:03 -0800

Fixes #2511 

-- 
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/2510#issuecomment-471160032

---

From: Kazuho Oku <[email-masked]>
Date: Sat, 09 Mar 2019 01:37:33 -0800

@kazuho pushed 1 commit.

8e4d28f9532de0451b360999a78d841f8d8b695f  clarify as connection error

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2510/files/851d56e53f867c299df660ebc8a33719583032df..8e4d28f9532de0451b360999a78d841f8d8b695f

---

From: Lucas Pardue <[email-masked]>
Date: Sat, 09 Mar 2019 02:19:17 -0800

@kazuho there are also some stream errors in QPACK you might want to address at the same time

-- 
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/2510#issuecomment-471164606

---

From: Kazuho Oku <[email-masked]>
Date: Sat, 09 Mar 2019 13:19:30 -0800

@kazuho pushed 1 commit.

243ae5cbb724200dac778457cd14968cb8430182  clarify that receiving an unknown stream type is not a _connection_ error

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2510/files/8e4d28f9532de0451b360999a78d841f8d8b695f..243ae5cbb724200dac778457cd14968cb8430182

---

From: Kazuho Oku <[email-masked]>
Date: Sat, 09 Mar 2019 13:25:43 -0800

@kazuho pushed 1 commit.

a7c4c591557a71c4dc765d7c74a4e3add55b6883  all QPACK errors become connection errors

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2510/files/243ae5cbb724200dac778457cd14968cb8430182..a7c4c591557a71c4dc765d7c74a4e3add55b6883

---

From: Kazuho Oku <[email-masked]>
Date: Sat, 09 Mar 2019 13:39:33 -0800

@ianswett @LPardue Thank you for the comments. Addressed in the commits above.

-- 
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/2510#issuecomment-471224439

---

From: Kazuho Oku <[email-masked]>
Date: Sat, 09 Mar 2019 14:23:03 -0800

@kazuho pushed 1 commit.

93d355877768113b1e37bd780b75605fae964b5f  clients MUST adhere `SETTINGS_MAX_HEADER_LIST_SIZE`, not doing so is a connection error

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2510/files/a7c4c591557a71c4dc765d7c74a4e3add55b6883..93d355877768113b1e37bd780b75605fae964b5f

---

From: Kazuho Oku <[email-masked]>
Date: Sat, 09 Mar 2019 14:27:07 -0800

@kazuho pushed 1 commit.

604b08d127094b9b3a8b6ad7919054b62ef98e6b  s/message header/header list/g

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2510/files/93d355877768113b1e37bd780b75605fae964b5f..604b08d127094b9b3a8b6ad7919054b62ef98e6b

---

From: ianswett <[email-masked]>
Date: Sat, 09 Mar 2019 18:39:03 -0800

ianswett approved this pull request.

LGTM

-- 
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/2510#pullrequestreview-212584932

---

From: Dmitri Tikhonov <[email-masked]>
Date: Mon, 11 Mar 2019 05:52:34 -0700

>  An endpoint MUST NOT send a header list that exceeds the peer-advised limit.

How can this be justified? This is a departure from RFC 7540, which states in two places that this setting is only advisory.

-- 
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/2510#issuecomment-471526344

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 11 Mar 2019 13:16:19 -0700

@dtikhonov 
> How can this be justified? This is a departure from RFC 7540, which states in two places that this setting is only advisory.

Yeah. My read of HTTP/3 is that it is already different from HTTP/2 this respect. It is no more clarifies as "advisory," no there is a mention that a client can exceed the limit (a text that exists in RFC 7540).

Regarding the ideal behavior, I prefer being strict here, because it would give us the most predictive behavior at a minimal cost (it's trivial to check the size, isn't it?). Think it from an end-user perspective. Without the restriction, we might see one user-agent telling you that the request is too large, and the other processing the same request. That'd be confusing.

-- 
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/2510#issuecomment-471710008

---

From: Dmitri Tikhonov <[email-masked]>
Date: Mon, 11 Mar 2019 13:28:10 -0700

The clients already differ in many, many other ways.  The size check is trivial, but that's not the problem.  The problem is the risk of breaking existing web applications.  The current HTTP/2 browser behavior is to advertise a limit, but to process much larger header lists.  We don't want to end up where *"HTTP/3 broke my app!"* is a thing.

-- 
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/2510#issuecomment-471716253

---

From: MikkelFJ <[email-masked]>
Date: Mon, 11 Mar 2019 13:31:08 -0700

Since HTTP/3 is not a thing yet, in browsers, the advice for browsers could be to advertise a larger realistic limit.

-- 
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/2510#issuecomment-471717314

---

From: Lucas Pardue <[email-masked]>
Date: Mon, 11 Mar 2019 13:41:58 -0700

H2 discusses max header list size in the same breath as header block sizes *and* individual header field sizes. The concepts are conflated (perhaps for good reason). 

H3 changes some things. For instance there are no CONTINUATIONS and the header list cannot span more than on HEADERS frame. 

Furthermore, QPACK header blocks dont modify table state, so there are fewer dangers in refusing to process them (or not?)

There is more to this than meets the eye and I dont think we can unpick the problem space as part of this change. Suggest topic is discussed on list or at Prague 104.

-- 
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/2510#issuecomment-471721126

---

From: Dmitri Tikhonov <[email-masked]>
Date: Mon, 11 Mar 2019 14:23:06 -0700

> I dont think we can unpick the problem space as part of this change

I agree.  Let's separate the "all protocol violations => connection error" and the "max header list size is no longer advisory" issues.

-- 
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/2510#issuecomment-471735393

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 11 Mar 2019 14:58:13 -0700

@kazuho pushed 1 commit.

08b7622a8fd29b243c09773f528ac525e1a7de92  revert 93d3558, 604b08d

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2510/files/604b08d127094b9b3a8b6ad7919054b62ef98e6b..08b7622a8fd29b243c09773f528ac525e1a7de92

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 11 Mar 2019 14:58:33 -0700

> Let's separate the "all protocol violations => connection error" and the "max header list size is no longer advisory" issues.

SGTM. Reverted the changes regarding max header list size.

-- 
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/2510#issuecomment-471752986

---

From: ianswett <[email-masked]>
Date: Mon, 11 Mar 2019 20:56:18 -0700

Thanks Kazuho, splitting that off makes sense.

-- 
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/2510#issuecomment-471847553