-
Notifications
You must be signed in to change notification settings - Fork 4.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[fuzz] add fuzz tests for hpack encoding and decoding #13315
Merged
Merged
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
b33f075
add fuzz test
asraa 55c4590
fixup
asraa 844f98f
don't ignore statuses and compare with vector
asraa c909e82
fixup
asraa a9b2344
fix spelling
asraa f99ca68
fix another spelling
asraa c89915d
fix compilation error
asraa fa4614c
Merge remote-tracking branch 'upstream/master' into hpack-tests
asraa c1a512a
fix compilation
asraa 9ec6761
Merge remote-tracking branch 'upstream/master' into hpack-tests
asraa e68ad9c
address clang tidy
asraa f30ec09
address comments
asraa fd7af21
make flags const
asraa File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
3 changes: 3 additions & 0 deletions
3
test/common/http/http2/hpack_corpus/crash-52ef0a2d4d861941325ba57fde63d2aa700f43cf
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
syntax = "proto3"; | ||
|
||
package test.common.http.http2; | ||
|
||
import "test/fuzz/common.proto"; | ||
|
||
import "validate/validate.proto"; | ||
|
||
// Structured input for hpack_fuzz_test. | ||
|
||
message HpackTestCase { | ||
test.fuzz.Headers headers = 1 [(validate.rules).message.required = true]; | ||
bool end_headers = 2; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,155 @@ | ||
// Fuzzer for HPACK encoding and decoding. | ||
// TODO(asraa): Speed up by using raw byte input and separators rather than protobuf input. | ||
|
||
#include <algorithm> | ||
|
||
#include "test/common/http/http2/hpack_fuzz.pb.validate.h" | ||
#include "test/fuzz/fuzz_runner.h" | ||
#include "test/test_common/utility.h" | ||
|
||
#include "absl/container/fixed_array.h" | ||
#include "nghttp2/nghttp2.h" | ||
|
||
namespace Envoy { | ||
namespace Http { | ||
namespace Http2 { | ||
namespace { | ||
|
||
// Dynamic Header Table Size | ||
constexpr int kHeaderTableSize = 4096; | ||
|
||
std::vector<nghttp2_nv> createNameValueArray(const test::fuzz::Headers& input) { | ||
const size_t nvlen = input.headers().size(); | ||
std::vector<nghttp2_nv> nva(nvlen); | ||
int i = 0; | ||
for (const auto& header : input.headers()) { | ||
// TODO(asraa): Consider adding flags in fuzzed input. | ||
const uint8_t flags = 0; | ||
nva[i++] = {const_cast<uint8_t*>(reinterpret_cast<const uint8_t*>(header.key().data())), | ||
const_cast<uint8_t*>(reinterpret_cast<const uint8_t*>(header.value().data())), | ||
header.key().size(), header.value().size(), flags}; | ||
} | ||
|
||
return nva; | ||
} | ||
|
||
Buffer::OwnedImpl encodeHeaders(nghttp2_hd_deflater* deflater, | ||
const std::vector<nghttp2_nv>& input_nv) { | ||
// Estimate the upper bound | ||
const size_t buflen = nghttp2_hd_deflate_bound(deflater, input_nv.data(), input_nv.size()); | ||
|
||
Buffer::RawSlice iovec; | ||
Buffer::OwnedImpl payload; | ||
payload.reserve(buflen, &iovec, 1); | ||
ASSERT(iovec.len_ >= buflen); | ||
|
||
// Encode using nghttp2 | ||
uint8_t* buf = reinterpret_cast<uint8_t*>(iovec.mem_); | ||
ASSERT(input_nv.data() != nullptr); | ||
const ssize_t result = | ||
nghttp2_hd_deflate_hd(deflater, buf, buflen, input_nv.data(), input_nv.size()); | ||
ASSERT(result >= 0, absl::StrCat("Failed to decode with result ", result)); | ||
|
||
iovec.len_ = result; | ||
payload.commit(&iovec, 1); | ||
|
||
return payload; | ||
} | ||
|
||
std::vector<nghttp2_nv> decodeHeaders(nghttp2_hd_inflater* inflater, | ||
const Buffer::OwnedImpl& payload, bool end_headers) { | ||
// Decode using nghttp2 | ||
Buffer::RawSliceVector slices = payload.getRawSlices(); | ||
const int num_slices = slices.size(); | ||
ASSERT(num_slices == 1, absl::StrCat("number of slices ", num_slices)); | ||
|
||
std::vector<nghttp2_nv> decoded_headers; | ||
int inflate_flags = 0; | ||
nghttp2_nv decoded_nv; | ||
while (slices[0].len_ > 0) { | ||
ssize_t result = nghttp2_hd_inflate_hd2(inflater, &decoded_nv, &inflate_flags, | ||
reinterpret_cast<uint8_t*>(slices[0].mem_), | ||
slices[0].len_, end_headers); | ||
// Decoding should not fail and data should not be left in slice. | ||
ASSERT(result >= 0); | ||
|
||
slices[0].mem_ = reinterpret_cast<uint8_t*>(slices[0].mem_) + result; | ||
slices[0].len_ -= result; | ||
|
||
if (inflate_flags & NGHTTP2_HD_INFLATE_EMIT) { | ||
// One header key value pair has been successfully decoded. | ||
decoded_headers.push_back(decoded_nv); | ||
} | ||
} | ||
|
||
if (end_headers) { | ||
nghttp2_hd_inflate_end_headers(inflater); | ||
} | ||
|
||
return decoded_headers; | ||
} | ||
|
||
struct NvComparator { | ||
inline bool operator()(const nghttp2_nv& a, const nghttp2_nv& b) { | ||
absl::string_view a_str(reinterpret_cast<char*>(a.name), a.namelen); | ||
absl::string_view b_str(reinterpret_cast<char*>(b.name), b.namelen); | ||
return a_str.compare(b_str); | ||
} | ||
}; | ||
|
||
DEFINE_PROTO_FUZZER(const test::common::http::http2::HpackTestCase& input) { | ||
// Validate headers. | ||
try { | ||
TestUtility::validate(input); | ||
} catch (const EnvoyException& e) { | ||
ENVOY_LOG_MISC(trace, "EnvoyException: {}", e.what()); | ||
return; | ||
} | ||
|
||
// Create name value pairs from headers. | ||
std::vector<nghttp2_nv> input_nv = createNameValueArray(input.headers()); | ||
// Skip encoding empty headers. nghttp2 will throw a nullptr error on runtime if it receives a | ||
// nullptr input. | ||
if (!input_nv.data()) { | ||
return; | ||
} | ||
|
||
// Create Deflater and Inflater | ||
nghttp2_hd_deflater* deflater = nullptr; | ||
int rc = nghttp2_hd_deflate_new(&deflater, kHeaderTableSize); | ||
ASSERT(rc == 0); | ||
nghttp2_hd_inflater* inflater = nullptr; | ||
rc = nghttp2_hd_inflate_new(&inflater); | ||
ASSERT(rc == 0); | ||
|
||
// Encode headers with nghttp2. | ||
const Buffer::OwnedImpl payload = encodeHeaders(deflater, input_nv); | ||
ASSERT(!payload.getRawSlices().empty()); | ||
|
||
// Decode headers with nghttp2 | ||
std::vector<nghttp2_nv> output_nv = decodeHeaders(inflater, payload, input.end_headers()); | ||
|
||
// Verify that decoded == encoded. | ||
ASSERT(input_nv.size() == output_nv.size()); | ||
std::sort(input_nv.begin(), input_nv.end(), NvComparator()); | ||
std::sort(output_nv.begin(), output_nv.end(), NvComparator()); | ||
for (size_t i = 0; i < input_nv.size(); i++) { | ||
absl::string_view in_name = {reinterpret_cast<char*>(input_nv[i].name), input_nv[i].namelen}; | ||
absl::string_view out_name = {reinterpret_cast<char*>(output_nv[i].name), output_nv[i].namelen}; | ||
absl::string_view in_val = {reinterpret_cast<char*>(input_nv[i].value), input_nv[i].valuelen}; | ||
absl::string_view out_val = {reinterpret_cast<char*>(output_nv[i].value), | ||
output_nv[i].valuelen}; | ||
ASSERT(in_name == out_name); | ||
ASSERT(in_val == out_val); | ||
} | ||
|
||
// Delete inflater | ||
nghttp2_hd_inflate_del(inflater); | ||
// Delete deflater. | ||
nghttp2_hd_deflate_del(deflater); | ||
} | ||
|
||
} // namespace | ||
} // namespace Http2 | ||
} // namespace Http | ||
} // namespace Envoy |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe leave a TODO to make this even faster by skipping LPM and working with direct byte array representing lists of headers (let's say separated by any characters that aren't valid HTTP header vals).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done. using vectors rather than HeaderMap sped it up to about 800, adding verification via qsort and compare brought it back down to 700.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was actually suggesting to skip using proto entirely, rather than skipping the HeaderMap, but up to you, I think it's fine to merge as is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, acked. The TODO about that is at the top of the file "// TODO(asraa): Speed up by using raw byte input and seperators rather than protobuf input."