hyperium / http

Rust HTTP types
Apache License 2.0
1.12k stars 283 forks source link

Undocumented capacity limit for HeaderMap #603

Open jongiddy opened 1 year ago

jongiddy commented 1 year ago

HeaderMap has a capacity limit of 24576 headers. Adding any more headers triggers a panic when reserving more capacity.

I can't see any way to prevent the panic when handling incoming headers, except to hard-wire a check for this value into calling code.

It would be useful to expose this value as a public constant.

At the very least it should be documented. Currently HeaderMap::reserve says "Panics if the new allocation size overflows usize." This should be "Panics if the new allocation size is greater than 24576."

seanmonstar commented 1 year ago

Good catch, thanks for reporting this! I'd propose a few improvements:

DDtKey commented 1 year ago

I believe the correct way to fix that is to introduce fallible try_append method. It's too fragile to rely on reservation methods & documentation only and easy to miss the handling.

It's clearly vulnerability right now, because all usages like this one will cause a panic

cc @seanmonstar

seanmonstar commented 1 year ago

Yea, I think the three bullet points I listed above would be a great addition. Want to submit a PR?