More HPACK encoder changes

src/hpack/encoder.rs

@@ -6,29 +6,74 @@ use bytes::{BytesMut, BufMut};
 
 pub struct Encoder {
     table: Table,
-
+    size_update: Option<SizeUpdate>,
-    // The remote sent a max size update, we must shrink the table on next call
-    // to encode. This is in bytes
-    max_size_update: Option<usize>,
 }
 
 pub enum EncoderError {
 }
 
+#[derive(Debug, Copy, Clone, Eq, PartialEq)]
+enum SizeUpdate {
+    One(usize),
+    Two(usize, usize), // min, max
+}
+
 impl Encoder {
     pub fn new(max_size: usize, capacity: usize) -> Encoder {
         Encoder {
             table: Table::new(max_size, capacity),
-            max_size_update: None,
+            size_update: None,
+        }
+    }
+
+    /// Queues a max size update.
+    ///
+    /// The next call to `encode` will include a dynamic size update frame.
+    pub fn update_max_size(&mut self, val: usize) {
+        match self.size_update {
+            Some(SizeUpdate::One(old)) => {
+                if val > old {
+                    if old > self.table.max_size() {
+                        self.size_update = Some(SizeUpdate::One(val));
+                    } else {
+                        self.size_update = Some(SizeUpdate::Two(old, val));
+                    }
+                } else {
+                    self.size_update = Some(SizeUpdate::One(val));
+                }
+            }
+            Some(SizeUpdate::Two(min, max)) => {
+                if val < min {
+                    self.size_update = Some(SizeUpdate::One(val));
+                } else {
+                    self.size_update = Some(SizeUpdate::Two(min, val));
+                }
+            }
+            None => {
+                if val != self.table.max_size() {
+                    // Don't bother writing a frame if the value already matches
+                    // the table's max size.
+                    self.size_update = Some(SizeUpdate::One(val));
+                }
+            }
         }
     }
 
     pub fn encode<'a, I>(&mut self, headers: I, dst: &mut BytesMut) -> Result<(), EncoderError>
         where I: IntoIterator<Item=Header>,
     {
-        if let Some(max_size_update) = self.max_size_update.take() {
+        match self.size_update.take() {
-            // Write size update frame
+            Some(SizeUpdate::One(val)) => {
-            unimplemented!();
+                self.table.resize(val);
+                encode_size_update(val, dst);
+            }
+            Some(SizeUpdate::Two(min, max)) => {
+                self.table.resize(min);
+                self.table.resize(max);
+                encode_size_update(min, dst);
+                encode_size_update(max, dst);
+            }
+            None => {}
         }
 
         for h in headers {
@@ -135,6 +180,10 @@ fn encode_str(val: &[u8], dst: &mut BytesMut) {
     }
 }
 
+fn encode_size_update<B: BufMut>(val: usize, dst: &mut B) {
+    encode_int(val, 5, 0b00100000, dst)
+}
+
 /// Encode an integer into the given destination buffer
 fn encode_int<B: BufMut>(
     mut value: usize,   // The integer to encode
@@ -355,19 +404,128 @@ mod test {
     }
 
     #[test]
-    fn test_at_most_two_values_per_name_indexed() {
+    fn test_encoding_headers_with_same_name() {
+        let mut encoder = Encoder::default();
+        let name = "hello";
+
+        // Encode first one
+        let _ = encode(&mut encoder, vec![header(name, "one")]);
+
+        // Encode second one
+        let res = encode(&mut encoder, vec![header(name, "two")]);
+        assert_eq!(&[0x40 | 62, 0x80 | 3], &res[0..2]);
+        assert_eq!("two", huff_decode(&res[2..]));
+        assert_eq!(5, res.len());
+
+        // Encode the first one again
+        let res = encode(&mut encoder, vec![header(name, "one")]);
+        assert_eq!(&[0x80 | 62], &res[..]);
+
+        // Now the second one
+        let res = encode(&mut encoder, vec![header(name, "two")]);
+        assert_eq!(&[0x80 | 63], &res[..]);
     }
 
     #[test]
-    fn test_index_header_with_duplicate_name_does_not_evict() {
+    fn test_evicting_headers_when_multiple_of_same_name_are_in_table() {
+        // The encoder only has space for 2 headers
+        let mut encoder = Encoder::new(76, 0);
+
+        let _ = encode(&mut encoder, vec![header("foo", "bar")]);
+        assert_eq!(1, encoder.table.len());
+
+        let _ = encode(&mut encoder, vec![header("bar", "foo")]);
+        assert_eq!(2, encoder.table.len());
+
+        // This will evict the first header, while still referencing the header
+        // name
+        let res = encode(&mut encoder, vec![header("foo", "baz")]);
+        assert_eq!(&[0x40 | 62, 0x80 | 3], &res[..2]);
+        assert_eq!(2, encoder.table.len());
+
+        // Try adding the same header again
+        let res = encode(&mut encoder, vec![header("foo", "baz")]);
+        assert_eq!(&[0x80 | 63], &res[..]);
+        assert_eq!(2, encoder.table.len());
     }
 
     #[test]
     fn test_max_size_zero() {
+        // Static table only
+        let mut encoder = Encoder::new(0, 0);
+        let res = encode(&mut encoder, vec![method("GET")]);
+        assert_eq!(*res, [0x80 | 2]);
+        assert_eq!(encoder.table.len(), 0);
+
+        let res = encode(&mut encoder, vec![header("foo", "bar")]);
+        assert_eq!(&[0, 0x80 | 2], &res[..2]);
+        assert_eq!("foo", huff_decode(&res[2..4]));
+        assert_eq!(0x80 | 3, res[4]);
+        assert_eq!("bar", huff_decode(&res[5..8]));
+        assert_eq!(0, encoder.table.len());
+
+        // Encode a custom value
+        let res = encode(&mut encoder, vec![header("transfer-encoding", "chunked")]);
+        assert_eq!(&[15, 42, 0x80 | 6], &res[..3]);
+        assert_eq!("chunked", huff_decode(&res[3..]));
     }
 
     #[test]
-    fn test_increasing_table_size() {
+    fn test_update_max_size_combos() {
+        let mut encoder = Encoder::default();
+        assert!(encoder.size_update.is_none());
+        assert_eq!(4096, encoder.table.max_size());
+
+        encoder.update_max_size(4096); // Default size
+        assert!(encoder.size_update.is_none());
+
+        encoder.update_max_size(0);
+        assert_eq!(Some(SizeUpdate::One(0)), encoder.size_update);
+
+        encoder.update_max_size(100);
+        assert_eq!(Some(SizeUpdate::Two(0, 100)), encoder.size_update);
+
+        let mut encoder = Encoder::default();
+        encoder.update_max_size(8000);
+        assert_eq!(Some(SizeUpdate::One(8000)), encoder.size_update);
+
+        encoder.update_max_size(100);
+        assert_eq!(Some(SizeUpdate::One(100)), encoder.size_update);
+
+        encoder.update_max_size(8000);
+        assert_eq!(Some(SizeUpdate::Two(100, 8000)), encoder.size_update);
+
+        encoder.update_max_size(4000);
+        assert_eq!(Some(SizeUpdate::Two(100, 4000)), encoder.size_update);
+
+        encoder.update_max_size(50);
+        assert_eq!(Some(SizeUpdate::One(50)), encoder.size_update);
+    }
+
+    #[test]
+    fn test_resizing_table() {
+        let mut encoder = Encoder::default();
+
+        // Add a header
+        let _ = encode(&mut encoder, vec![header("foo", "bar")]);
+
+        encoder.update_max_size(1);
+        assert_eq!(1, encoder.table.len());
+
+        let res = encode(&mut encoder, vec![method("GET")]);
+        assert_eq!(&[32 | 1, 0x80 | 2], &res[..]);
+        assert_eq!(0, encoder.table.len());
+
+        let res = encode(&mut encoder, vec![header("foo", "bar")]);
+        assert_eq!(0, res[0]);
+
+        encoder.update_max_size(100);
+        let res = encode(&mut encoder, vec![header("foo", "bar")]);
+        assert_eq!(&[32 | 31, 69, 64], &res[..3]);
+
+        encoder.update_max_size(0);
+        let res = encode(&mut encoder, vec![header("foo", "bar")]);
+        assert_eq!(&[32, 0], &res[..2]);
     }
 
     #[test]

src/hpack/table.rs

@@ -4,7 +4,7 @@ use fnv::FnvHasher;
 use http::method;
 use http::header::{self, HeaderName, HeaderValue};
 
-use std::{cmp, mem};
+use std::{cmp, mem, usize};
 use std::collections::VecDeque;
 use std::hash::{Hash, Hasher};
 
@@ -54,10 +54,6 @@ struct Pos {
 struct HashValue(usize);
 
 const MAX_SIZE: usize = (1 << 16);
-
-// Index at most this number of values for any given header name.
-const MAX_VALUES_PER_NAME: usize = 3;
-
 const DYN_OFFSET: usize = 62;
 
 macro_rules! probe_loop {
@@ -106,6 +102,10 @@ impl Table {
         usable_capacity(self.indices.len())
     }
 
+    pub fn max_size(&self) -> usize {
+        self.max_size
+    }
+
     /// Index the header in the HPACK table.
     pub fn index(&mut self, header: Header) -> Index {
         // Check the static table
@@ -162,7 +162,7 @@ impl Table {
                     return self.index_vacant(header, hash, dist, probe, statik);
                 } else if pos.hash == hash && self.slots[slot_idx].header.name() == header.name() {
                     // Matching name, check values
-                    return self.index_occupied(header, pos.index, statik);
+                    return self.index_occupied(header, hash, pos.index, statik);
                 }
             } else {
                 return self.index_vacant(header, hash, dist, probe, statik);
@@ -172,15 +172,17 @@ impl Table {
         });
     }
 
-    fn index_occupied(&mut self, header: Header, mut index: usize, statik: Option<(usize, bool)>)
+    fn index_occupied(&mut self,
+                      header: Header,
+                      hash: HashValue,
+                      mut index: usize,
+                      statik: Option<(usize, bool)>)
         -> Index
     {
         // There already is a match for the given header name. Check if a value
         // matches. The header will also only be inserted if the table is not at
         // capacity.
-        let mut n = 0;
+        loop {
-
-        while n < MAX_VALUES_PER_NAME {
             // Compute the real index into the VecDeque
             let real_idx = index.wrapping_sub(self.evicted);
 
@@ -190,7 +192,7 @@ impl Table {
             }
 
             if let Some(next) = self.slots[real_idx].next {
-                n = next;
+                index = next;
                 continue;
             }
 
@@ -198,8 +200,34 @@ impl Table {
                 return Index::Name(real_idx + DYN_OFFSET, header);
             }
 
-            // Maybe index
+            self.update_size(header.len(), index);
-            unimplemented!();
+
+            let new_idx = self.slots.len();
+
+            // If `evicted` is greater than `index`, then the previous node in
+            // the linked list got evicted. The header we are about to insert is
+            // the new "head" of the list and `indices` has already been
+            // updated. So, all that is left to do is insert the header in the
+            // VecDeque.
+            //
+            // TODO: This logic isn't correct in the face of wrapping
+            if self.evicted <= index {
+                // Recompute `real_idx` since this could have been modified by
+                // entries being evicted
+                let real_idx = index.wrapping_sub(self.evicted);
+
+                self.slots[real_idx].next = Some(new_idx.wrapping_add(self.evicted));
+            }
+
+            self.slots.push_back(Slot {
+                hash: hash,
+                header: header,
+                next: None,
+            });
+
+            // Even if the previous header was evicted, we can still reference
+            // it when inserting the new one...
+            return Index::InsertedValue(real_idx + DYN_OFFSET, &self.slots[new_idx].header);
         }
 
         Index::NotIndexed(header)
@@ -217,7 +245,9 @@ impl Table {
             return Index::new(statik, header);
         }
 
-        if self.update_size(header.len()) {
+        // Passing in `usize::MAX` for prev_idx since there is no previous
+        // header in this case.
+        if self.update_size(header.len(), usize::MAX) {
             if dist != 0 {
                 let back = probe.wrapping_sub(1) & self.mask;
 
@@ -269,19 +299,40 @@ impl Table {
         }
     }
 
-    fn update_size(&mut self, len: usize) -> bool {
+    pub fn resize(&mut self, size: usize) {
+        self.max_size = size;
+
+        if size == 0 {
+            self.size = 0;
+
+            for i in &mut self.indices {
+                *i = None;
+            }
+
+            self.slots.clear();
+            self.evicted = 0;
+        } else {
+            self.converge(usize::MAX);
+        }
+    }
+
+    fn update_size(&mut self, len: usize, prev_idx: usize) -> bool {
         self.size += len;
+        self.converge(prev_idx)
+    }
+
+    fn converge(&mut self, prev_idx: usize) -> bool {
         let mut ret = false;
 
         while self.size > self.max_size {
             ret = true;
-            self.evict();
+            self.evict(prev_idx);
         }
 
         ret
     }
 
-    fn evict(&mut self) {
+    fn evict(&mut self, prev_idx: usize) {
         debug_assert!(!self.slots.is_empty());
 
         // Remove the header
@@ -302,7 +353,11 @@ impl Table {
                 if let Some(idx) = slot.next {
                     pos.index = idx;
                     self.indices[probe] = Some(pos);
+                } else if pos.index == prev_idx {
+                    pos.index = (self.slots.len() + 1).wrapping_add(self.evicted);
+                    self.indices[probe] = Some(pos);
                 } else {
+                    self.indices[probe] = None;
                     self.remove_phase_two(probe);
                 }