Subject: [quicwg/base-drafts] Push ID limit checking in DUPLICATE_PUSH (#2695)

From: Mike Bishop <[email-masked]>
Date: Mon, 13 May 2019 11:08:16 -0700

Fixes #2681.

Skirts the editorial/design line.  Previously, DUPLICATE_PUSH referred to a Push ID in a previously-sent PUSH_PROMISE frame; that frame would have caused a connection error on arrival.  This says that the error should be sent regardless of which frame arrives first.

May need reconciling with #2662.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Push ID limit checking in DUPLICATE_PUSH
  * Not malformed

-- File Changes --

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

-- Patch Links --

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

---

From: ianswett <[email-masked]>
Date: Mon, 13 May 2019 11:39:29 -0700

ianswett 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/2695#pullrequestreview-236849712

---

From: Martin Thomson <[email-masked]>
Date: Mon, 13 May 2019 18:44:58 -0700

martinthomson commented on this pull request.

I think that we have a frame name problem, otherwise, this is fine.

> @@ -890,7 +890,11 @@ of a DUPLICATE_PUSH frame as a connection error of type HTTP_UNEXPECTED_FRAME.
 
 The DUPLICATE_PUSH frame carries a single variable-length integer that
 identifies the Push ID of a resource that the server has previously promised
-(see {{frame-push-promise}}).
+(see {{frame-push-promise}}), though that promise might not be received before
+this frame.  A server MUST NOT use a Push ID that is larger than the client has
+provided in a MAX_PUSH_ID frame ({{frame-max-push-id}}).  A client MUST treat
+receipt of a PUSH_PROMISE that contains a larger Push ID than the client has

or DUPLICATE_PUSH?

-- 
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/2695#pullrequestreview-236992784

---

From: Lucas Pardue <[email-masked]>
Date: Tue, 14 May 2019 06:24:24 -0700

LPardue requested changes on this pull request.

I think some wires got crossed in the suggested changes. Your comment points out that this might need reconciliation with #2662 but it seems to borrow the text directly from that PR.

The outcome is that this PR is inconsistent with itself because at line [796](https://github.com/quicwg/base-drafts/pull/2695/files#diff-3d2f9fd0738589657498f2c06ee02f2fL796) the same error with PUSH_PROMISE causes a MALFORMED_FRAME. 

I think the best path is to make this change be a MALFORMED_FRAME error and reconcile on #2662 if the group want that change.

-- 
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/2695#pullrequestreview-237240393

---

From: Mike Bishop <[email-masked]>
Date: Tue, 14 May 2019 09:20:01 -0700

MikeBishop commented on this pull request.

Derp, yes.  Copied the text and didn't get this changed.

-- 
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/2695#discussion_r283886395

---

From: Mike Bishop <[email-masked]>
Date: Tue, 14 May 2019 09:20:54 -0700

@MikeBishop pushed 1 commit.

09dcba386d2f6b12ad8de33dcef1c4ca3cc2b8d3  Reconcile and revert

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2695/files/c40ead760ac4444d1d7e051e6feee011994ee0f7..09dcba386d2f6b12ad8de33dcef1c4ca3cc2b8d3

---

From: Mike Bishop <[email-masked]>
Date: Tue, 14 May 2019 09:22:41 -0700

Merged #2695 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/2695#event-2340710801