More bug fixes

src/hpack/header.rs

@@ -204,7 +204,7 @@ impl<'a> Name<'a> {
                 Ok(Header::Authority(try!(ByteStr::from_utf8(value))))
             }
             Name::Method => {
-                Ok(Header::Scheme(try!(ByteStr::from_utf8(value))))
+                Ok(Header::Method(try!(Method::from_bytes(&*value))))
             }
             Name::Scheme => {
                 Ok(Header::Scheme(try!(ByteStr::from_utf8(value))))

src/hpack/table.rs

@@ -43,7 +43,7 @@ struct Slot {
     next: Option<usize>,
 }
 
-#[derive(Debug, Clone, Copy)]
+#[derive(Debug, Clone, Copy, Eq, PartialEq)]
 struct Pos {
     index: usize,
     hash: HashValue,
@@ -145,6 +145,8 @@ impl Table {
     }
 
     fn index_dynamic(&mut self, header: Header, statik: Option<(usize, bool)>) -> Index {
+        debug_assert!(self.assert_valid_state("one"));
+
         if header.len() + self.size < self.max_size || !header.is_sensitive() {
             // Only grow internal storage if needed
             self.reserve_one();
@@ -231,6 +233,8 @@ impl Table {
                 self.slots[new_real_idx].next = Some(idx);
             }
 
+            debug_assert!(self.assert_valid_state("bottom"));
+
             // Even if the previous header was evicted, we can still reference
             // it when inserting the new one...
             return Index::InsertedValue(real_idx + DYN_OFFSET, 0);
@@ -251,16 +255,20 @@ impl Table {
             return Index::new(statik, header);
         }
 
+        debug_assert!(self.assert_valid_state("top"));
+        debug_assert!(dist == 0 || self.indices[probe.wrapping_sub(1) & self.mask].is_some());
+
         // Passing in `usize::MAX` for prev_idx since there is no previous
         // header in this case.
         if self.update_size(header.len(), None) {
+            // TODO: This probably should have to walk back more than one time
             if dist != 0 {
                 let back = probe.wrapping_sub(1) & self.mask;
 
-                if let Some(pos) = self.indices[probe] {
+                if let Some(pos) = self.indices[back] {
                     let their_dist = probe_distance(self.mask, pos.hash, probe);
 
-                    if their_dist < dist {
+                    if their_dist < (dist - 1) {
                         probe = back;
                     }
                 } else {
@@ -269,6 +277,8 @@ impl Table {
             }
         }
 
+        debug_assert!(self.assert_valid_state("after update"));
+
         self.insert(header, hash);
 
         let pos_idx = 0usize.wrapping_sub(self.inserted);
@@ -292,6 +302,8 @@ impl Table {
             });
         }
 
+        debug_assert!(self.assert_valid_state("bottom"));
+
         if let Some((n, _)) = statik {
             Index::InsertedValue(n, 0)
         } else {
@@ -345,6 +357,9 @@ impl Table {
     fn evict(&mut self, prev_idx: Option<usize>) {
         let pos_idx = (self.slots.len() - 1).wrapping_sub(self.inserted);
 
+        debug_assert!(!self.slots.is_empty());
+        debug_assert!(self.assert_valid_state("one"));
+
         // Remove the header
         let slot = self.slots.pop_back().unwrap();
         let mut probe = desired_pos(self.mask, slot.hash);
@@ -352,8 +367,16 @@ impl Table {
         // Update the size
         self.size -= slot.header.len();
 
+        debug_assert_eq!(
+            self.indices.iter().filter_map(|p| *p)
+                .filter(|p| p.index == pos_idx)
+                .count(),
+            1);
+
         // Find the associated position
         probe_loop!(probe < self.indices.len(), {
+            debug_assert!(!self.indices[probe].is_none());
+
             let mut pos = self.indices[probe].unwrap();
 
             if pos.index == pos_idx {
@@ -371,6 +394,8 @@ impl Table {
                 break;
             }
         });
+
+        debug_assert!(self.assert_valid_state("two"));
     }
 
     // Shifts all indices that were displaced by the header that has just been
@@ -392,6 +417,8 @@ impl Table {
 
             last_probe = probe;
         });
+
+        debug_assert!(self.assert_valid_state("two"));
     }
 
     fn reserve_one(&mut self) {
@@ -454,6 +481,92 @@ impl Table {
             });
         }
     }
+
+    /// Checks that the internal map state is correct
+    fn assert_valid_state(&self, msg: &'static str) -> bool {
+        // Ensure all hash codes in indices match the associated slot
+        for pos in &self.indices {
+            if let Some(pos) = *pos {
+                let real_idx = pos.index.wrapping_add(self.inserted);
+
+                if real_idx.wrapping_add(1) != 0 {
+                    assert!(real_idx < self.slots.len(),
+                            "out of index; real={}; len={}, msg={}",
+                            real_idx, self.slots.len(), msg);
+
+                    assert_eq!(pos.hash, self.slots[real_idx].hash,
+                               "index hash does not match slot; msg={}", msg);
+                }
+            }
+        }
+
+        // Every index is only available once
+        for i in 0..self.indices.len() {
+            if self.indices[i].is_none() {
+                continue;
+            }
+
+            for j in i+1..self.indices.len() {
+                assert_ne!(self.indices[i], self.indices[j],
+                            "duplicate indices; msg={}", msg);
+            }
+        }
+
+        for (index, slot) in self.slots.iter().enumerate() {
+            let mut indexed = false;
+
+            // First, see if the slot is indexed
+            for pos in &self.indices {
+                if let Some(pos) = *pos {
+                    let real_idx = pos.index.wrapping_add(self.inserted);
+                    if real_idx == index {
+                        indexed = true;
+                        // Already know that there is no dup, so break
+                        break;
+                    }
+                }
+            }
+
+            if indexed {
+                // Ensure that it is accessible..
+                let desired = desired_pos(self.mask, slot.hash);
+                let mut probe = desired;
+                let mut dist = 0;
+
+                probe_loop!(probe < self.indices.len(), {
+                    assert!(self.indices[probe].is_some(),
+                            "unexpected empty slot; probe={}; hash={:?}; msg={}",
+                            probe, slot.hash, msg);
+
+                    let pos = self.indices[probe].unwrap();
+
+                    let their_dist = probe_distance(self.mask, pos.hash, probe);
+                    let real_idx = pos.index.wrapping_add(self.inserted);
+
+                    if real_idx == index {
+                        break;
+                    }
+
+                    assert!(dist <= their_dist,
+                            "could not find entry; desired={}; dist={}; their_dist={}; index={}; msg={}",
+                            desired, dist, their_dist, index.wrapping_sub(self.inserted), msg);
+
+                    dist += 1;
+                });
+            } else {
+                // There is exactly one next link
+                let cnt = self.slots.iter().map(|s| s.next)
+                    .filter(|n| *n == Some(index.wrapping_sub(self.inserted)))
+                    .count();
+
+                assert_eq!(1, cnt, "more than one node pointing here; msg={}", msg);
+            }
+        }
+
+        // TODO: Ensure linked lists are correct: no cycles, etc...
+
+        true
+    }
 }
 
 #[cfg(test)]

src/hpack/test.rs

@@ -26,13 +26,38 @@ use std::str;
 const MAX_CHUNK: usize = 2 * 1024;
 
 #[test]
-fn hpack_fuzz_failing_1() {
+fn hpack_failing_1() {
     FuzzHpack::new_reduced([
-        12433181738898983662,
-        14102727336666980714,
-        6105092270172216412,
-        16258270543720336235,
-    ], 1).run();
+        14571479824075392697, 1933795017656710260, 3334564825787790363, 18384038562943935004,
+    ], 100).run();
+}
+
+#[test]
+fn hpack_failing_2() {
+    FuzzHpack::new_reduced([
+        93927840931624528, 7252171548136134810, 13289640692556535960, 11484086244506193733,
+    ], 100).run();
+}
+
+#[test]
+fn hpack_failing_3() {
+    FuzzHpack::new_reduced([
+        4320463360720445614, 7244328615656028238, 10856862580207993426, 5400459931473084625,
+    ], 61).run();
+}
+
+#[test]
+fn hpack_failing_4() {
+    FuzzHpack::new_reduced([
+        7199712575090753518, 8301132414711594706, 8069319383349578021, 5376546610900316263,
+    ], 107).run();
+}
+
+#[test]
+fn hpack_failing_5() {
+    FuzzHpack::new_reduced([
+        17764083779960581082, 12579311332935512090, 16627815831742045696, 13140603923739395199,
+    ], 101).run();
 }
 
 #[test]
@@ -136,6 +161,8 @@ impl FuzzHpack {
             }
         }
 
+        encoded.push(buf);
+
         // Decode
         for buf in encoded {
             decoder.decode(&buf.into(), |e| {