Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions include/proxy/hdrs/HdrHeap.h
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ struct StrHeapDesc {
class HdrHeap
{
public:
// NOTE: also sizes an on-stack HEADERS encode buffer (2*this) in HTTP/2; a static_assert bounds the growth.
Comment thread
JosiahWI marked this conversation as resolved.
static constexpr int DEFAULT_SIZE = 2048;

void init();
Expand Down
2 changes: 2 additions & 0 deletions include/proxy/http2/Http2Stream.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ class Http2Stream : public ProxyTransaction
void set_expect_receive_trailer() override;

Http2ErrorCode decode_header_blocks(HpackHandle &hpack_handle, uint32_t maximum_table_size);
Http2ErrorCode decode_header_blocks(HpackHandle &hpack_handle, uint32_t maximum_table_size, const uint8_t *block,
uint32_t block_len);
void send_headers(Http2ConnectionState &cstate);
void initiating_close();
bool is_outbound_connection() const;
Expand Down
33 changes: 25 additions & 8 deletions src/proxy/http2/Http2ConnectionState.cc
Original file line number Diff line number Diff line change
Expand Up @@ -452,11 +452,22 @@ Http2ConnectionState::rcv_headers_frame(const Http2Frame &frame)
if (stream->trailing_header_is_possible()) {
// Don't leak the header_blocks from the initial, non-trailing headers.
ats_free(stream->header_blocks);
stream->header_blocks = nullptr;
Comment thread
JosiahWI marked this conversation as resolved.
}
stream->header_blocks = static_cast<uint8_t *>(ats_malloc(header_block_fragment_length));
frame.reader()->memcpy(stream->header_blocks, header_block_fragment_length, header_block_fragment_offset);

if (frame.header().flags & HTTP2_FLAGS_HEADERS_END_HEADERS) {
// Avoids the per-request malloc+memcpy+free of the header block.
bool const end_headers = frame.header().flags & HTTP2_FLAGS_HEADERS_END_HEADERS;
uint8_t const *inplace_block = nullptr;

if (end_headers && frame.reader()->block_read_avail() >=
static_cast<int64_t>(header_block_fragment_offset) + static_cast<int64_t>(header_block_fragment_length)) {
inplace_block = reinterpret_cast<uint8_t const *>(frame.reader()->start()) + header_block_fragment_offset;
} else {
stream->header_blocks = static_cast<uint8_t *>(ats_malloc(header_block_fragment_length));
frame.reader()->memcpy(stream->header_blocks, header_block_fragment_length, header_block_fragment_offset);
}

if (end_headers) {
// NOTE: If there are END_HEADERS flag, decode stored Header Blocks.
if (!stream->change_state(HTTP2_FRAME_TYPE_HEADERS, frame.header().flags)) {
return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_CONNECTION, Http2ErrorCode::HTTP2_ERROR_PROTOCOL_ERROR,
Expand All @@ -475,8 +486,12 @@ Http2ConnectionState::rcv_headers_frame(const Http2Frame &frame)
} else {
stream->mark_milestone(Http2StreamMilestone::START_DECODE_HEADERS);
}
Http2ErrorCode result = stream->decode_header_blocks(*this->local_hpack_handle,
this->acknowledged_local_settings.get(HTTP2_SETTINGS_HEADER_TABLE_SIZE));
Http2ErrorCode result = inplace_block ?
stream->decode_header_blocks(*this->local_hpack_handle,
this->acknowledged_local_settings.get(HTTP2_SETTINGS_HEADER_TABLE_SIZE),
inplace_block, header_block_fragment_length) :
stream->decode_header_blocks(*this->local_hpack_handle,
this->acknowledged_local_settings.get(HTTP2_SETTINGS_HEADER_TABLE_SIZE));

// If this was an outbound connection and the state was already closed, just clear the
// headers after processing. We just processed the header blocks to keep the dynamic table in
Expand Down Expand Up @@ -2472,9 +2487,11 @@ Http2ConnectionState::send_headers_frame(Http2Stream *stream)
http2_convert_header_from_1_1_to_2(send_hdr);
}

uint32_t buf_len = send_hdr->length_get() * 2; // Make it double just in case
ts::LocalBuffer local_buffer(buf_len);
uint8_t *buf = local_buffer.data();
uint32_t buf_len = send_hdr->length_get() * 2; // Make it double just in case
// Keep typical header sets off the heap; oversized ones spill to it.
static_assert(HdrHeap::DEFAULT_SIZE * 2 <= 8192, "keep HEADERS encode stack buffer within the event-thread stack budget");
ts::LocalBuffer<uint8_t, HdrHeap::DEFAULT_SIZE * 2> local_buffer(buf_len);
uint8_t *buf = local_buffer.data();
Comment on lines +2490 to +2494

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do I understand correctly that you are increasing the buffer capacity here to increase the chance that typical response headers will fit in the buffer?


stream->mark_milestone(Http2StreamMilestone::START_ENCODE_HEADERS);
Http2ErrorCode result = http2_encode_header_blocks(send_hdr, buf, buf_len, &header_blocks_size, *(this->peer_hpack_handle),
Expand Down
10 changes: 8 additions & 2 deletions src/proxy/http2/Http2Stream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -283,10 +283,16 @@ Http2Stream::main_event_handler(int event, void *edata)

Http2ErrorCode
Http2Stream::decode_header_blocks(HpackHandle &hpack_handle, uint32_t maximum_table_size)
{
return this->decode_header_blocks(hpack_handle, maximum_table_size, header_blocks, header_blocks_length);
}

Http2ErrorCode
Http2Stream::decode_header_blocks(HpackHandle &hpack_handle, uint32_t maximum_table_size, const uint8_t *block, uint32_t block_len)
{
Http2ErrorCode error =
http2_decode_header_blocks(&_receive_header, (const uint8_t *)header_blocks, header_blocks_length, nullptr, hpack_handle,
_trailing_header_is_possible, maximum_table_size, this->is_outbound_connection());
http2_decode_header_blocks(&_receive_header, block, block_len, nullptr, hpack_handle, _trailing_header_is_possible,
maximum_table_size, this->is_outbound_connection());
Comment on lines +286 to +295

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reason for adding the 4-argument overload?

if (error != Http2ErrorCode::HTTP2_ERROR_NO_ERROR) {
Http2StreamDebug("Error decoding header blocks: %u", static_cast<uint32_t>(error));
}
Expand Down