Skip to content

Commit

Permalink
change prefix_int to use u64 instead of usize (hyperium#223)
Browse files Browse the repository at this point in the history
RFC 9204 4.1.1:

    QPACK implementations MUST be able to decode integers up to and including 62 bits long.
  • Loading branch information
alexanderkjall authored Jan 9, 2024
1 parent a530808 commit e7c7ab9
Show file tree
Hide file tree
Showing 6 changed files with 159 additions and 53 deletions.
114 changes: 88 additions & 26 deletions h3/src/qpack/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,17 +177,30 @@ impl HeaderPrefix {
pub fn decode<R: Buf>(buf: &mut R) -> Result<Self, ParseError> {
let (_, encoded_insert_count) = prefix_int::decode(8, buf)?;
let (sign_negative, delta_base) = prefix_int::decode(7, buf)?;

if encoded_insert_count > (usize::MAX as u64) {
return Err(ParseError::Integer(
crate::qpack::prefix_int::Error::Overflow,
));
}

if delta_base > (usize::MAX as u64) {
return Err(ParseError::Integer(
crate::qpack::prefix_int::Error::Overflow,
));
}

Ok(Self {
encoded_insert_count,
delta_base,
encoded_insert_count: encoded_insert_count as usize,
delta_base: delta_base as usize,
sign_negative: sign_negative == 1,
})
}

pub fn encode<W: BufMut>(&self, buf: &mut W) {
let sign_bit = if self.sign_negative { 1 } else { 0 };
prefix_int::encode(8, 0, self.encoded_insert_count, buf);
prefix_int::encode(7, sign_bit, self.delta_base, buf);
prefix_int::encode(8, 0, self.encoded_insert_count as u64, buf);
prefix_int::encode(7, sign_bit, self.delta_base as u64, buf);
}
}

Expand All @@ -200,16 +213,32 @@ pub enum Indexed {
impl Indexed {
pub fn decode<R: Buf>(buf: &mut R) -> Result<Self, ParseError> {
match prefix_int::decode(6, buf)? {
(0b11, i) => Ok(Indexed::Static(i)),
(0b10, i) => Ok(Indexed::Dynamic(i)),
(0b11, i) => {
if i > (usize::MAX as u64) {
return Err(ParseError::Integer(
crate::qpack::prefix_int::Error::Overflow,
));
}

Ok(Indexed::Static(i as usize))
}
(0b10, i) => {
if i > (usize::MAX as u64) {
return Err(ParseError::Integer(
crate::qpack::prefix_int::Error::Overflow,
));
}

Ok(Indexed::Dynamic(i as usize))
}
(f, _) => Err(ParseError::InvalidPrefix(f)),
}
}

pub fn encode<W: BufMut>(&self, buf: &mut W) {
match self {
Indexed::Static(i) => prefix_int::encode(6, 0b11, *i, buf),
Indexed::Dynamic(i) => prefix_int::encode(6, 0b10, *i, buf),
Indexed::Static(i) => prefix_int::encode(6, 0b11, *i as u64, buf),
Indexed::Dynamic(i) => prefix_int::encode(6, 0b10, *i as u64, buf),
}
}
}
Expand All @@ -220,13 +249,21 @@ pub struct IndexedWithPostBase(pub usize);
impl IndexedWithPostBase {
pub fn decode<R: Buf>(buf: &mut R) -> Result<Self, ParseError> {
match prefix_int::decode(4, buf)? {
(0b0001, i) => Ok(IndexedWithPostBase(i)),
(0b0001, i) => {
if i > (usize::MAX as u64) {
return Err(ParseError::Integer(
crate::qpack::prefix_int::Error::Overflow,
));
}

Ok(IndexedWithPostBase(i as usize))
}
(f, _) => Err(ParseError::InvalidPrefix(f)),
}
}

pub fn encode<W: BufMut>(&self, buf: &mut W) {
prefix_int::encode(4, 0b0001, self.0, buf)
prefix_int::encode(4, 0b0001, self.0 as u64, buf)
}
}

Expand All @@ -253,26 +290,42 @@ impl LiteralWithNameRef {

pub fn decode<R: Buf>(buf: &mut R) -> Result<Self, ParseError> {
match prefix_int::decode(4, buf)? {
(f, i) if f & 0b0101 == 0b0101 => Ok(LiteralWithNameRef::new_static(
i,
prefix_string::decode(8, buf)?,
)),
(f, i) if f & 0b0101 == 0b0100 => Ok(LiteralWithNameRef::new_dynamic(
i,
prefix_string::decode(8, buf)?,
)),
(f, i) if f & 0b0101 == 0b0101 => {
if i > (usize::MAX as u64) {
return Err(ParseError::Integer(
crate::qpack::prefix_int::Error::Overflow,
));
}

Ok(LiteralWithNameRef::new_static(
i as usize,
prefix_string::decode(8, buf)?,
))
}
(f, i) if f & 0b0101 == 0b0100 => {
if i > (usize::MAX as u64) {
return Err(ParseError::Integer(
crate::qpack::prefix_int::Error::Overflow,
));
}

Ok(LiteralWithNameRef::new_dynamic(
i as usize,
prefix_string::decode(8, buf)?,
))
}
(f, _) => Err(ParseError::InvalidPrefix(f)),
}
}

pub fn encode<W: BufMut>(&self, buf: &mut W) -> Result<(), prefix_string::Error> {
match self {
LiteralWithNameRef::Static { index, value } => {
prefix_int::encode(4, 0b0101, *index, buf);
prefix_int::encode(4, 0b0101, *index as u64, buf);
prefix_string::encode(8, 0, value, buf)?;
}
LiteralWithNameRef::Dynamic { index, value } => {
prefix_int::encode(4, 0b0100, *index, buf);
prefix_int::encode(4, 0b0100, *index as u64, buf);
prefix_string::encode(8, 0, value, buf)?;
}
}
Expand All @@ -296,16 +349,24 @@ impl LiteralWithPostBaseNameRef {

pub fn decode<R: Buf>(buf: &mut R) -> Result<Self, ParseError> {
match prefix_int::decode(3, buf)? {
(f, i) if f & 0b1111_0000 == 0 => Ok(LiteralWithPostBaseNameRef::new(
i,
prefix_string::decode(8, buf)?,
)),
(f, i) if f & 0b1111_0000 == 0 => {
if i > (usize::MAX as u64) {
return Err(ParseError::Integer(
crate::qpack::prefix_int::Error::Overflow,
));
}

Ok(LiteralWithPostBaseNameRef::new(
i as usize,
prefix_string::decode(8, buf)?,
))
}
(f, _) => Err(ParseError::InvalidPrefix(f)),
}
}

pub fn encode<W: BufMut>(&self, buf: &mut W) -> Result<(), prefix_string::Error> {
prefix_int::encode(3, 0b0000, self.index, buf);
prefix_int::encode(3, 0b0000, self.index as u64, buf);
prefix_string::encode(8, 0, &self.value, buf)?;
Ok(())
}
Expand Down Expand Up @@ -347,6 +408,7 @@ impl Literal {
#[cfg(test)]
mod test {
use super::*;
use std::convert::TryInto;
use std::io::Cursor;

const TABLE_SIZE: usize = 4096;
Expand Down Expand Up @@ -424,7 +486,7 @@ mod test {
#[test]
fn base_index_too_small() {
let mut buf = vec![];
let encoded_largest_ref = (2 % (2 * TABLE_SIZE / 32)) + 1;
let encoded_largest_ref: u64 = ((2 % (2 * TABLE_SIZE / 32)) + 1).try_into().unwrap();
prefix_int::encode(8, 0, encoded_largest_ref, &mut buf);
prefix_int::encode(7, 1, 2, &mut buf); // base index negative = 0

Expand Down
13 changes: 11 additions & 2 deletions h3/src/qpack/decoder.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use bytes::{Buf, BufMut};
use std::{fmt, io::Cursor};
use std::{convert::TryInto, fmt, io::Cursor, num::TryFromIntError};

use tracing::trace;

Expand Down Expand Up @@ -36,6 +36,7 @@ pub enum Error {
BadBaseIndex(isize),
UnexpectedEnd,
HeaderTooLong(u64),
BufSize(TryFromIntError),
}

impl std::error::Error for Error {}
Expand All @@ -53,6 +54,7 @@ impl std::fmt::Display for Error {
Error::BadBaseIndex(i) => write!(f, "out of bounds base index: {}", i),
Error::UnexpectedEnd => write!(f, "unexpected end"),
Error::HeaderTooLong(_) => write!(f, "header too long"),
Error::BufSize(_) => write!(f, "number in buffer wrong size"),
}
}
}
Expand Down Expand Up @@ -126,7 +128,8 @@ impl Decoder {
}

if self.table.total_inserted() != inserted_on_start {
InsertCountIncrement(self.table.total_inserted() - inserted_on_start).encode(write);
InsertCountIncrement((self.table.total_inserted() - inserted_on_start).try_into()?)
.encode(write);
}

Ok(self.table.total_inserted())
Expand Down Expand Up @@ -326,6 +329,12 @@ impl From<ParseError> for Error {
}
}

impl From<TryFromIntError> for Error {
fn from(error: TryFromIntError) -> Self {
Error::BufSize(error)
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
5 changes: 2 additions & 3 deletions h3/src/qpack/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -239,9 +239,8 @@ impl Action {
let first = buf.chunk()[0];
let instruction = match DecoderInstruction::decode(first) {
DecoderInstruction::Unknown => return Err(Error::UnknownDecoderInstruction(first)),
DecoderInstruction::InsertCountIncrement => {
InsertCountIncrement::decode(&mut buf)?.map(|x| Action::ReceivedRefIncrement(x.0))
}
DecoderInstruction::InsertCountIncrement => InsertCountIncrement::decode(&mut buf)?
.map(|x| Action::ReceivedRefIncrement(x.0 as usize)),
DecoderInstruction::HeaderAck => {
HeaderAck::decode(&mut buf)?.map(|x| Action::Untrack(x.0))
}
Expand Down
20 changes: 10 additions & 10 deletions h3/src/qpack/prefix_int.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ impl std::fmt::Display for Error {
}
}

pub fn decode<B: Buf>(size: u8, buf: &mut B) -> Result<(u8, usize), Error> {
pub fn decode<B: Buf>(size: u8, buf: &mut B) -> Result<(u8, u64), Error> {
assert!(size <= 8);
let mut first = buf.get::<u8>()?;

Expand All @@ -31,13 +31,13 @@ pub fn decode<B: Buf>(size: u8, buf: &mut B) -> Result<(u8, usize), Error> {

// if first < 2usize.pow(size) - 1
if first < mask {
return Ok((flags, first as usize));
return Ok((flags, first as u64));
}

let mut value = mask as usize;
let mut value = mask as u64;
let mut power = 0usize;
loop {
let byte = buf.get::<u8>()? as usize;
let byte = buf.get::<u8>()? as u64;
value += (byte & 127) << power;
power += 7;

Expand All @@ -53,21 +53,21 @@ pub fn decode<B: Buf>(size: u8, buf: &mut B) -> Result<(u8, usize), Error> {
Ok((flags, value))
}

pub fn encode<B: BufMut>(size: u8, flags: u8, value: usize, buf: &mut B) {
pub fn encode<B: BufMut>(size: u8, flags: u8, value: u64, buf: &mut B) {
assert!(size <= 8);
// NOTE: following casts to u8 intend to trim the most significant bits, they are used as a
// workaround for shiftoverflow errors when size == 8.
let mask = !(0xFF << size) as u8;
let flags = ((flags as usize) << size) as u8;

// if value < 2usize.pow(size) - 1
if value < (mask as usize) {
if value < (mask as u64) {
buf.write(flags | value as u8);
return;
}

buf.write(mask | flags);
let mut remaining = value - mask as usize;
let mut remaining = value - mask as u64;

while remaining >= 128 {
let rest = (remaining % 128) as u8;
Expand All @@ -93,7 +93,7 @@ impl From<coding::UnexpectedEnd> for Error {
mod test {
use std::io::Cursor;

fn check_codec(size: u8, flags: u8, value: usize, data: &[u8]) {
fn check_codec(size: u8, flags: u8, value: u64, data: &[u8]) {
let mut buf = Vec::new();
super::encode(size, flags, value, &mut buf);
assert_eq!(buf, data);
Expand All @@ -110,7 +110,7 @@ mod test {
check_codec(
5,
0b010,
usize::max_value(),
u64::max_value(),
&[95, 224, 255, 255, 255, 255, 255, 255, 255, 255, 1],
);
}
Expand All @@ -122,7 +122,7 @@ mod test {
check_codec(
8,
0,
usize::max_value(),
u64::max_value(),
&[255, 128, 254, 255, 255, 255, 255, 255, 255, 255, 1],
);
}
Expand Down
13 changes: 12 additions & 1 deletion h3/src/qpack/prefix_string/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ mod bitwin;
mod decode;
mod encode;

use std::convert::TryInto;
use std::fmt;
use std::num::TryFromIntError;

use bytes::{Buf, BufMut};

Expand All @@ -22,6 +24,7 @@ pub enum Error {
Integer(IntegerError),
HuffmanDecoding(HuffmanDecodingError),
HuffmanEncoding(HuffmanEncodingError),
BufSize(TryFromIntError),
}

impl std::fmt::Display for Error {
Expand All @@ -31,12 +34,14 @@ impl std::fmt::Display for Error {
Error::Integer(e) => write!(f, "could not parse integer: {}", e),
Error::HuffmanDecoding(e) => write!(f, "Huffman decode failed: {:?}", e),
Error::HuffmanEncoding(e) => write!(f, "Huffman encode failed: {:?}", e),
Error::BufSize(_) => write!(f, "number in buffer wrong size"),
}
}
}

pub fn decode<B: Buf>(size: u8, buf: &mut B) -> Result<Vec<u8>, Error> {
let (flags, len) = prefix_int::decode(size - 1, buf)?;
let len: usize = len.try_into()?;
if buf.remaining() < len {
return Err(Error::UnexpectedEnd);
}
Expand All @@ -56,7 +61,7 @@ pub fn decode<B: Buf>(size: u8, buf: &mut B) -> Result<Vec<u8>, Error> {

pub fn encode<B: BufMut>(size: u8, flags: u8, value: &[u8], buf: &mut B) -> Result<(), Error> {
let encoded = Vec::from(value).hpack_encode()?;
prefix_int::encode(size - 1, flags << 1 | 1, encoded.len(), buf);
prefix_int::encode(size - 1, flags << 1 | 1, encoded.len().try_into()?, buf);
for byte in encoded {
buf.write(byte);
}
Expand Down Expand Up @@ -84,6 +89,12 @@ impl From<HuffmanDecodingError> for Error {
}
}

impl From<TryFromIntError> for Error {
fn from(error: TryFromIntError) -> Self {
Error::BufSize(error)
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
Loading

0 comments on commit e7c7ab9

Please sign in to comment.