Skip to content

Commit

Permalink
Consume buf only if header name and value are both decoded
Browse files Browse the repository at this point in the history
Decoding error when processing continuation header which contains normal
header name at boundary
  • Loading branch information
caiyuanhao committed Jan 14, 2022
1 parent c876dda commit 605785c
Showing 1 changed file with 79 additions and 7 deletions.
86 changes: 79 additions & 7 deletions src/hpack/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,10 +279,13 @@ impl Decoder {

// First, read the header name
if table_idx == 0 {
let old_pos = buf.position();
let name_marker = self.try_decode_string(buf)?;
let value_marker = self.try_decode_string(buf)?;
buf.set_position(old_pos);
// Read the name as a literal
let name = self.decode_string(buf)?;
let value = self.decode_string(buf)?;

let name = take_string_marker(buf, name_marker);
let value = take_string_marker(buf, value_marker);
Header::new(name, value)
} else {
let e = self.table.get(table_idx)?;
Expand All @@ -292,7 +295,11 @@ impl Decoder {
}
}

fn decode_string(&mut self, buf: &mut Cursor<&mut BytesMut>) -> Result<Bytes, DecoderError> {
fn try_decode_string(
&mut self,
buf: &mut Cursor<&mut BytesMut>,
) -> Result<((usize, usize), Option<Bytes>), DecoderError> {
let old_pos = buf.position();
const HUFF_FLAG: u8 = 0b1000_0000;

// The first bit in the first byte contains the huffman encoded flag.
Expand All @@ -309,17 +316,27 @@ impl Decoder {
return Err(DecoderError::NeedMore(NeedMore::StringUnderflow));
}

let offset = (buf.position() - old_pos) as usize;
if huff {
let ret = {
let raw = &buf.chunk()[..len];
huffman::decode(raw, &mut self.buffer).map(BytesMut::freeze)
huffman::decode(raw, &mut self.buffer)
.map(|buf| ((offset, len), Some(BytesMut::freeze(buf))))
};

buf.advance(len);
return ret;
ret
} else {
buf.advance(len);
Ok(((offset, len), None))
}
}

Ok(take(buf, len))
fn decode_string(&mut self, buf: &mut Cursor<&mut BytesMut>) -> Result<Bytes, DecoderError> {
let old_pos = buf.position();
let marker = self.try_decode_string(buf)?;
buf.set_position(old_pos);
Ok(take_string_marker(buf, marker))
}
}

Expand Down Expand Up @@ -433,6 +450,21 @@ fn take(buf: &mut Cursor<&mut BytesMut>, n: usize) -> Bytes {
head.freeze()
}

fn take_string_marker(
buf: &mut Cursor<&mut BytesMut>,
marker: ((usize, usize), Option<Bytes>),
) -> Bytes {
let ((offset, len), string) = marker;
buf.advance(offset);
match string {
Some(string) => {
buf.advance(len);
string
}
None => take(buf, len),
}
}

fn consume(buf: &mut Cursor<&mut BytesMut>) {
// remove bytes from the internal BytesMut when they have been successfully
// decoded. This is a more permanent cursor position, which will be
Expand Down Expand Up @@ -850,4 +882,44 @@ mod test {
huffman::encode(src, &mut buf);
buf
}

#[test]
fn test_decode_continuation_header() {
let mut de = Decoder::new(0);
let value = huff_encode(b"bar");
let mut buf = BytesMut::new();
buf.extend(&[0b01000000, 0x00 | 3]);
buf.extend(b"foo");
buf.extend(&[0x80 | 3]);
buf.extend(&value[0..1]);

let mut res = vec![];
let e = de
.decode(&mut Cursor::new(&mut buf), |h| {
res.push(h);
})
.unwrap_err();
assert_eq!(e, DecoderError::NeedMore(NeedMore::StringUnderflow));
buf.extend(&value[1..]);

let _ = de
.decode(&mut Cursor::new(&mut buf), |h| {
res.push(h);
})
.unwrap();

assert_eq!(res.len(), 1);
assert_eq!(de.table.size(), 0);

match res[0] {
Header::Field {
ref name,
ref value,
} => {
assert_eq!(name, "foo");
assert_eq!(value, "bar");
}
_ => panic!(),
}
}
}

0 comments on commit 605785c

Please sign in to comment.