Subject: [quicwg/base-drafts] Invalid Largest Reference is a connection error. (#2249)

From: Bence Béky <[email-masked]>
Date: Sun, 23 Dec 2018 13:11:06 -0800

If Largest Reference does not figure, then the decoded Base Index will probably be bogus, resulting in corrupt entries.  I think it's better to close the connection with an error.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Invalid Largest Reference is a connection error.

-- File Changes --

    M draft-ietf-quic-qpack.md (18)

-- Patch Links --

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

---

From: Bence Béky <[email-masked]>
Date: Sun, 23 Dec 2018 13:14:48 -0800

bencebeky commented on this pull request.

>           # Decoder wrapped around 1 extra time
          CurrentWrapped += 2 * MaxEntries
 
-      LargestReference += TotalNumberOfInserts - CurrentWrapped
+      LargestReference += TotalNumberOfInserts
+
+      if LargestReference <= CurrentWrapped:

I put the error checking here because I am using unsigned integers.  If the majority of implementors are expected to use signed integers (after all, 2**62 required by the spec can comfortably be represented using 64 bit signed integers), then the original `LargestReference += TotalNumberOfInserts - CurrentWrapped` line can be kept and the error checking woul look like:

```
   if LargestReference <= 0:
      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/2249#pullrequestreview-187632746

---

From: Martin Thomson <[email-masked]>
Date: Sun, 23 Dec 2018 22:30:27 -0800

This will conflict with #2248.

-- 
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/2249#issuecomment-449692304

---

From: Bence Béky <[email-masked]>
Date: Mon, 24 Dec 2018 05:50:51 -0800

> This will conflict with #2248.

No worries, I do not expect all of my PRs to be approved (even though I think they are all improvements).  I'm just drafting them to start a discussion, because for small things like this it is much easier to talk about an actual PR then to try describing the changes.

If there is consensus to merge either this or #2248, I'm happy to rebase the other 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/2249#issuecomment-449736701