Potential Infinite loop in evict_to_internal function

[KershawChang]

See bug 1939690 comment 4.

I've asked the reporter to provide STR. It's still not clear how easy this can be triggered.

[mxinden]

The reporter on Bugzilla is hinting at this code:

neqo/neqo-qpack/src/table.rs

Lines 263 to 285 in 922d266

	pub fn evict_to_internal(&mut self, reduce: u64, only_check: bool) -> bool {
	qtrace!(
	[self],
	"reduce table to {}, currently used:{} only_check:{}",
	reduce,
	self.used,
	only_check
	);
	let mut used = self.used;
	while (!self.dynamic.is_empty()) && used > reduce {
	if let Some(e) = self.dynamic.back() {
	if !e.can_reduce(self.acked_inserts_cnt) {
	return false;
	}
	used -= u64::try_from(e.size()).unwrap();
	if !only_check {
	self.used -= u64::try_from(e.size()).unwrap();
	self.dynamic.pop_back();
	}
	}
	}
	true
	}

Off the top of my head I see the following scenario:

• only_check is true
• self.dyamic.is_empty() is false
• e.can_reduce() is true

This would lead to infinite many unnecessary loop iterations.

The following should fix the issue:

diff --git a/neqo-qpack/src/table.rs b/neqo-qpack/src/table.rs
index 8c8b8d5e..abea8be2 100644
--- a/neqo-qpack/src/table.rs
+++ b/neqo-qpack/src/table.rs
@@ -260,6 +260,7 @@ impl HeaderTable {
         self.evict_to_internal(reduce, true)
     }
 
     pub fn evict_to_internal(&mut self, reduce: u64, only_check: bool) -> bool {
         qtrace!(
             [self],
@@ -278,6 +279,8 @@ impl HeaderTable {
                 if !only_check {
                     self.used -= u64::try_from(e.size()).unwrap();
                     self.dynamic.pop_back();
+                } else {
+                    return true;
                 }
             }
         }

I will give this more thought and try to reproduce it in a unit test.