Unexpected panic when using header::OccupiedEntry::remove_entry_mult

l3ku commented 3 years ago

I am trying to remove all possible values for a SET_COOKIE header from a HeaderMap, and extract them into a separate variable called cookies. My current code is the following:

let response: Response<Body> = Response::builder()
    .header("set-cookie", "cookie1=a")
    .header("set-cookie", "cookie2=b")
    .body(Body::from(()))
    .expect("failed to create response");

let (parts, body) = response.into_parts();
let mut headers = parts.headers;

...

let cookies: Vec<HeaderValue> = match headers.entry(SET_COOKIE) {
    Entry::Occupied(e) => e.remove_entry_mult().1.collect(),
    Entry::Vacant(_) => vec![],
};

When I run this, I get the following panic:

thread 'response::tests::serialize_cookies' panicked at 'index out of bounds: the len is 0 but the index is 0', /Users/leotoikka/.cargo/registry/src/github.com-1ecc6299db9ec823/http-0.2.0/src/header/map.rs:1606:13
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Relevant parts of the backtrace when ran with RUST_BACKTRACE=1:

   9:        0x10b656034 - <http::header::map::RawLinks<T> as core::ops::index::IndexMut<usize>>::index_mut::haae19e4b2ee84e7b
                               at /Users/leotoikka/.cargo/registry/src/github.com-1ecc6299db9ec823/http-0.2.0/src/header/map.rs:3102
  10:        0x10b654e06 - http::header::map::remove_extra_value::hf7cd983444b37b83
                               at /Users/leotoikka/.cargo/registry/src/github.com-1ecc6299db9ec823/http-0.2.0/src/header/map.rs:1606
  11:        0x10b5c3414 - <http::header::map::ValueDrain<T> as core::iter::traits::iterator::Iterator>::next::h734d41d3af084598
                               at /Users/leotoikka/.cargo/registry/src/github.com-1ecc6299db9ec823/http-0.2.0/src/header/map.rs:3042
  12:        0x10b54858d - <http::header::map::ValueDrain<T> as core::ops::drop::Drop>::drop::h081f86fb65b64b3d
                               at /Users/leotoikka/.cargo/registry/src/github.com-1ecc6299db9ec823/http-0.2.0/src/header/map.rs:3072
  13:        0x10b5478c5 - core::ptr::drop_in_place::he2a9208b609548b1

Am I doing something wrong or is this a bug?

Thanks!

seanmonstar commented 3 years ago

It does look like a bug in http. However, I don't think the panic is inside remote_entry_mult(), but rather happening during (after?) the call to collect(), since the backtrace shows it's happening as part of the Iterator::next. I'd have to look more to try to find why exactly, unless you wanted to try to follow the code and find the bug.

dekellum commented 3 years ago

I had a related test case in flight, so I minimally reproduced this in 19e82f4. Note that it only panics with 2 or more header values for the same name; it doesn't panic on 0 or 1 header value.

l3ku commented 3 years ago

I can try to look into it, but I'm not very familiar with the implementation of HeaderMap so it might take me some time to find the root cause for the panic.

dekellum commented 3 years ago

I believe #449 is now a reasonable fix. Note that tests added there show the bug extends to OccupiedEntry::remove_entry() as well.

hyperium / http

Unexpected panic when using header::OccupiedEntry::remove_entry_mult #446