Subject: [quicwg/base-drafts] Discuss max-header-list-size from the other side (#2774)

From: Mike Bishop <[email-masked]>
Date: Thu, 06 Jun 2019 11:28:37 -0700

Fixes #2516.  All the text currently there is correct, but it was missing the required behavior on receipt of this parameter.  That is, as agreed in London, you SHOULD NOT send a larger header than the peer's indicated limit since it will likely just error out anyway.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Discuss max-header-list-size from the other side

-- File Changes --

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

-- Patch Links --

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

---

From: afrind <[email-masked]>
Date: Thu, 06 Jun 2019 12:40:44 -0700

afrind approved this pull 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/2774#pullrequestreview-246775746

---

From: Kazuho Oku <[email-masked]>
Date: Thu, 06 Jun 2019 19:23:11 -0700

kazuho approved this pull request.

LGTM.

OTOH, I think I might have a different opinion on how the error gets reported to the client. I've filed #2775 explaining the issue.

-- 
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/2774#pullrequestreview-246893228

---

From: Mike Bishop <[email-masked]>
Date: Fri, 07 Jun 2019 09:54:35 -0700

@MikeBishop pushed 1 commit.

dd36a721e6ff100820d2cf6adfbe7a3005950d58  Import RFC7540 recommendation to 431 on oversize headers; fixes #2775

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2774/files/7ab8417b5d3c17346da74d4f82571b24a37eae41..dd36a721e6ff100820d2cf6adfbe7a3005950d58

---

From: Kazuho Oku <[email-masked]>
Date: Fri, 07 Jun 2019 12:21:56 -0700

LGTM. Thanks for importing the suggestion to use HTTP status code 431.

-- 
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/2774#pullrequestreview-247266765

---

From: Martin Thomson <[email-masked]>
Date: Mon, 10 Jun 2019 17:10:31 -0700

I don't know if this is worth addressing here, but this doesn't explicitly acknowledge the hop-by-hop nature of this limit.  That's a big part of why this is so terrible a setting.  Each endpoint advises only of the limits that it is aware of, and is not obligated to propagate limits that might apply to messages that might be forwarded to other entities.  And endpoints might choose to forward messages even knowing that they will exceed limits.

-- 
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/2774#issuecomment-500637640

---

From: Lucas Pardue <[email-masked]>
Date: Tue, 11 Jun 2019 07:20:34 -0700

I'm still confused by the purpose of this paragraph, it opens with discussion of the header (emphasis mine):

> An HTTP/3 implementation MAY impose a limit on the maximum size of **the header** it will accept on an individual HTTP message

Since the rest of the paragraph discusses SETTINGS_MAX_HEADER_LIST_SIZE it seems that the limit only applies to the sum total of all header fields in a message. It is not possible to express the maximum size of a single header field. If so, we should change the wording slightly to make this clearer.

-- 
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/2774#issuecomment-500863297

---

From: Lucas Pardue <[email-masked]>
Date: Tue, 11 Jun 2019 07:31:32 -0700

I also find it confusing that the proposed changes say in effect "A server that receives a larger header block than it is willing to handle" and "SHOULD NOT send an HTTP message header which exceeds the indicated size".

Are we talking about header blocks or header lists? I think it would help to be consistent in terminology here.

P.S pedantically it might be something "an endpoint that receives a header block which decompresses to a list that is larger than it is willing to serve" but thats a mouthful and seems more wordy than HTTP/2

-- 
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/2774#issuecomment-500868445

---

From: Mike Bishop <[email-masked]>
Date: Tue, 11 Jun 2019 16:42:05 -0700

> I'm still confused by this paragraph, it opens with discussion of the header (emphasis mine):
> 

The message header consists of a sequence header fields.  The limit is on the full size of the message header, not any one field.  There's ongoing discussion in http-core about how best to accommodate the colloquial usage of "header" to refer to an individual field.

-- 
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/2774#issuecomment-501064228

---

From: Mike Bishop <[email-masked]>
Date: Wed, 12 Jun 2019 13:42:20 -0700

@MikeBishop pushed 2 commits.

470336a199b09364360381adb02036d148a92e46  Hop-by-hop
4f5fbccc0f297a0fc43014e4ccfbd85c2ee1a76e  Header terminology

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2774/files/dd36a721e6ff100820d2cf6adfbe7a3005950d58..4f5fbccc0f297a0fc43014e4ccfbd85c2ee1a76e

---

From: Mike Bishop <[email-masked]>
Date: Wed, 12 Jun 2019 13:43:06 -0700

@LPardue, @martinthomson - see if this helps.

-- 
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/2774#issuecomment-501447752

---

From: ianswett <[email-masked]>
Date: Wed, 12 Jun 2019 13:53:40 -0700

ianswett commented on this pull request.

> -including the length of the name and value in bytes plus an overhead of 32 bytes
-for each header field.
+An HTTP/3 implementation MAY impose a limit on the maximum size of the message
+header it will accept on an individual HTTP message.  A server that receives a
+larger header field list than it is willing to handle can send an HTTP 431
+(Request Header Fields Too Large) status code {{?RFC6585}}.  A client can
+discard responses that it cannot process.  The size of a header field list is
+calculated based on the uncompressed size of header fields, including the length
+of the name and value in bytes plus an overhead of 32 bytes for each header
+field.
+
+If an implementation wishes to advise its peer of this limit, it can be conveyed
+as a number of bytes in the `SETTINGS_MAX_HEADER_LIST_SIZE` parameter. An
+implementation which has received this parameter SHOULD NOT send an HTTP message
+header which exceeds the indicated size, as this will likely produce an error
+and could disrupt the entire connection if the peer reacts negatively.  However,

nit: reacts negatively is a bit vague.  How about "could disrupt other outstanding requests if the peer chooses to close the connection in response."

-- 
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/2774#pullrequestreview-249012461

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 12 Jun 2019 16:58:36 -0700

kazuho commented on this pull request.

-for each header field.

Actually, I think we’d better delete “and could disrupt the entire connection if the peer reacts negatively” considering the fact that such behavior goes against the spirit of this setting value being an advisory one
.

-- 
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/2774#discussion_r293162457

---

From: ianswett <[email-masked]>
Date: Wed, 12 Jun 2019 18:37:13 -0700

-for each header field.

It's advisory, but I think the spirit is that eventually, this CID isn't going to route correctly.  Every deployment has this property, it's just a matter of timeframe.  

So it is advisory.  But in the same sense that GOAWAY indicates not to send new requests, this is saying "Please don't send any more packets with CIDs below this index, because it might not work."

-- 
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/2774#discussion_r293177577

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 12 Jun 2019 19:22:49 -0700

-for each header field.

@ianswett This issue is about an _HTTP_ setting. Maybe you're comment is for one of the "Retire CID" issues of the transport?

-- 
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/2774#discussion_r293184299

---

From: ianswett <[email-masked]>
Date: Wed, 12 Jun 2019 19:27:11 -0700

-for each header field.

Yes, nevermind.

-- 
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/2774#discussion_r293184901

---

From: ianswett <[email-masked]>
Date: Thu, 13 Jun 2019 09:56:06 -0700

-for each header field.

In previous discussions, I thought we concluded closing the connection if the limit was exceeded was acceptable and therefore this wasn't really 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/2774#discussion_r293480535

---

From: Mike Bishop <[email-masked]>
Date: Thu, 13 Jun 2019 10:53:19 -0700

MikeBishop commented on this pull request.

-for each header field.

It is advisory, in that you're being told in advance what message size the peer will refuse to process.  When this was recommended to be a stream error, the risk was that a stream error can always be promoted to a connection error at implementation discretion, and a connection error would tear everything else down with it.

Now that we recommend an HTTP status code instead, this language might be unnecessary.

-- 
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/2774#discussion_r293503517

---

From: Lucas Pardue <[email-masked]>
Date: Tue, 18 Jun 2019 10:23:30 -0700

LPardue approved this pull request.

This looks better. I'll add an observation point that RFC 6585 says a 431 can be generated "when a single header field is at fault.". I don't think we need any specific text there to handle 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/2774#pullrequestreview-251246157

---

From: Mike Bishop <[email-masked]>
Date: Tue, 18 Jun 2019 14:26:17 -0700

9d13fb39c64bf567b1e3a21c9112676f529b3c5f  Connection (probably) won't blow up

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2774/files/4f5fbccc0f297a0fc43014e4ccfbd85c2ee1a76e..9d13fb39c64bf567b1e3a21c9112676f529b3c5f

---

From: Mike Bishop <[email-masked]>
Date: Tue, 18 Jun 2019 14:26:56 -0700

Merged #2774 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/2774#event-2422297817