Subject: Re: [quicwg/base-drafts] Use the FRAME_ENCODING_ERROR for errors in frame encoding (#3042)

From: Marten Seemann <[email-masked]>
Date: Sun, 03 Nov 2019 19:21:35 -0800

@marten-seemann pushed 1 commit.

35c522b33f0d5cf53a3ba6cdeaf5ff9d0301760e  add an error for overflow of a CRYPTO stream

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3042/files/c56dd27788254e1cc6574a3e50a77f16b1fccabd..35c522b33f0d5cf53a3ba6cdeaf5ff9d0301760e

---

From: Marten Seemann <[email-masked]>
Date: Sun, 03 Nov 2019 19:29:43 -0800

2ff363cee4bd2602e7f21e8727c6f3d70f86cfac  add an error for overflow of a MAX_STREAMS frame

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3042/files/35c522b33f0d5cf53a3ba6cdeaf5ff9d0301760e..2ff363cee4bd2602e7f21e8727c6f3d70f86cfac

---

From: Marten Seemann <[email-masked]>
Date: Sun, 03 Nov 2019 19:32:42 -0800

@marten-seemann pushed 2 commits.

810e84bfa7c591f9a914c32a0ac54fbd4d5b3d45  add an error for overflow of a CRYPTO stream
d6a83ceb22c2cade9cdb18be50dcfa09b86b10ce  add an error for overflow of a MAX_STREAMS frame

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3042/files/2ff363cee4bd2602e7f21e8727c6f3d70f86cfac..d6a83ceb22c2cade9cdb18be50dcfa09b86b10ce

---

From: Marten Seemann <[email-masked]>
Date: Sun, 03 Nov 2019 19:34:38 -0800

ed73161363ed99d6d75b9af190da1c4c27e02b94  add an error for overflow of a STREAMS_BLOCKED frame

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

---

From: Marten Seemann <[email-masked]>
Date: Sun, 03 Nov 2019 19:35:51 -0800

@martinthomson, @kazuho: I added text for CRYPTO, MAX_STREAMS and STREAMS_BLOCKED frames, however:

- for CRYPTO frames, there's no flow control. It would be awkward to use a FLOW_CONTROL_ERROR here, so I opted for just using a FRAME_ENCODING_ERROR,
- for MAX_STREAMS frames FRAME_ENCODING_ERROR is the only applicable error. We do have a STREAM_LIMIT_ERROR, but this error is described as "Too many streams opened", and MAX_STREAMS doesn't open any streams by itself.

-- 
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/3042#issuecomment-549216134

---

From: Martin Thomson <[email-masked]>
Date: Sun, 03 Nov 2019 19:39:20 -0800

Merged #3042 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/3042#event-2766494455

---

From: Martin Thomson <[email-masked]>
Date: Sun, 03 Nov 2019 19:39:46 -0800

Those seem like reasonable conclusions to me.  I'm happy taking this.  Now I just need to ensure that the changelog is updated.

-- 
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/3042#issuecomment-549216628

---

From: Marten Seemann <[email-masked]>
Date: Sun, 03 Nov 2019 20:36:27 -0800

@kazuho There's no flow control in the sense that you don't advertise a window and you don't update any window.

-- 
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/3042#issuecomment-549222679

---

From: Kazuho Oku <[email-masked]>
Date: Sun, 03 Nov 2019 20:33:17 -0800

@marten-seemann 
> for CRYPTO frames, there's no flow control. It would be awkward to use a FLOW_CONTROL_ERROR here, so I opted for just using a FRAME_ENCODING_ERROR,

I'm not sure if I follow the logic here.

If there is no flow control, then I'd presume that exceeding 2<sup>62</sup> bytes is totally fine, and therefore that that the receiver should not be required (i.e. MUST) to send 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/3042#issuecomment-549222362

---

From: Kazuho Oku <[email-masked]>
Date: Sun, 03 Nov 2019 20:46:05 -0800

@marten-seemann Are you suggesting that the CRYPTO stream has implicit limit of 2<sup>62</sup> bytes that can be transferred? I think my question is how you would justify it. My claim is that it's an unnecessary complexity.

-- 
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/3042#issuecomment-549223654

---

From: Marten Seemann <[email-masked]>
Date: Sun, 03 Nov 2019 20:48:47 -0800

@kazuho That's what's written in the spec. I was not happy with our decision from Tokio to not communicate a limit for the data that can be sent on a CRYPTO stream, and have peers just drop any frames sent on that stream in the future instead. But that's what the working group decided to do.

-- 
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/3042#issuecomment-549223932

---

From: Kazuho Oku <[email-masked]>
Date: Sun, 03 Nov 2019 21:02:35 -0800

@marten-seemann I think you are conflating two different issues. #2524 is an issue about the amount of data buffered, while this issue is about the maximum offset of data an endpoint can ultimately represent.

Let me explain why "MUST respond with a FRAME_ENCODING_ERROR" for a CRYPTO stream is a problem.

The reason we have allowed the use of FLOW_CONTROL_ERROR for STREAM frames is because that is an error that can be caught by the stream-level receive buffer (and it's logic), rather than the frame decoder.

Stating that the error code MUST be FRAME_ENCODING_ERROR in this particular case goes against that agreement.

That logic that we used for STREAM frames can be equally applied to CRYPTO streams. The error code can either be CRYPTO_BUFFER_EXCEEDED or FLOW_CONTROL_ERROR. I don't mind either. But requiring FRAME_ENCODING_ERROR is different, because it could require the stream-level logic to respond either with a stream-level error code (CRYPTO_BUFFER_EXCEEDED or FLOW_CONTROL_ERROR) and frame-level error code (FRAME_DECODING_ERROR) based on the input. As can be seen, this requirement to distinguish between the two cases is the same as the one we argue for the STREAM frame 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/3042#issuecomment-549225351

---

From: Marten Seemann <[email-masked]>
Date: Sun, 03 Nov 2019 21:05:16 -0800

@kazuho You're right. I missed that we have a CRYPTO_BUFFER_EXCEEDED error. I'll prepare a follow-up PR later today. Unfortunately, it looks like it won't make it into -24 any more 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/3042#issuecomment-549225665