fix soundness concern in h1 decoder

[robjtede]

PR Type

Unsoundness Fix

PR Checklist

• Tests for the changes have been added / updated.
• Documentation comments have been added / updated.
• A changelog entry has been made for the appropriate packages.

Overview

From all the sources I can see, its never okay to do:

let arr:[T; n] = unsafe { MaybeUninit::uninit().assume_init() };

unless T is MaybeUninit<T> (see the nomicon).

Parsing HTTP headers in the h1 decoder currently uses code like this which Miri flags as unsound. See this playground for a simplified example.

This PR removes all the unsound memory initializations in h1 decoder and opts to use a pre-filled const array to be handed to httparse.

I believe this to be the fastest sound solution without requiring changes to httparse. (Aside: I think in theory, httparse could provide a parse function that fills in a MaybeUninit header array and allows caller to declare the array intitialized with the number of headers it parsed. You can imagine it's data flow with the setup in this modifed playground. However, I opted for the safer approach for now)

Benchmark code is included. Using test cases from httparse we see the following regressions:

Short Response:

Original (Unsound) [short]        time:   [162.17 ns 166.87 ns 172.44 ns]
New (safe) [short]                time:   [207.06 ns 210.67 ns 214.79 ns]

Realistic Response:

Original (Unsound) [realistic]    time:   [516.38 ns 526.95 ns 538.79 ns]
New (safe) [realistic]            time:   [532.47 ns 543.05 ns 556.96 ns]

[codecov-commenter]

Codecov Report

Merging #1614 into master will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1614      +/-   ##
==========================================
+ Coverage   53.15%   53.18%   +0.03%     
==========================================
  Files         122      122              
  Lines       11774    11770       -4     
==========================================
+ Hits         6258     6260       +2     
+ Misses       5516     5510       -6     

Impacted Files	Coverage Δ
actix-http/src/h1/decoder.rs	61.64% <100.00%> (+1.50%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0ec335a...1c39a86. Read the comment docs.

[JohnTitor]

Considering benchmarks, I think it's reasonable trade-off. Thanks for detail explanation and implementation, great work :)

[Shnatsel]

This RFC will allow getting performance back safely: rust-lang/rfcs#2930