Skip to content

Commit

Permalink
Code review changes: Removed some asserts. Formatted if statements
Browse files Browse the repository at this point in the history
  • Loading branch information
@ganeshmurthy
ganeshmurthy committed Oct 14, 2024
1 parent e0079ab commit ea1de32
Showing 1 changed file with 28 additions and 19 deletions.
47 changes: 28 additions & 19 deletions src/decoders/http2/http2_decoder.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ qd_http2_frame_type get_frame_payload_length_and_type(qd_http2_decoder_t *decode

static int allocate_move_to_scratch_buffer(qd_decoder_buffer_t *scratch_buffer, const uint8_t *data, size_t capacity)
{
if (capacity > HTTP2_SCRATCH_BUFFER_MAX_SIZE) {
if (capacity > HTTP2_SCRATCH_BUFFER_MAX_SIZE || capacity <= 0) {
return -1;
}
scratch_buffer->bytes = qd_malloc(capacity);
Expand All @@ -164,7 +164,9 @@ static int allocate_move_to_scratch_buffer(qd_decoder_buffer_t *scratch_buffer,

static int reallocate_move_to_scratch_buffer(qd_decoder_buffer_t *scratch_buffer, const uint8_t *data, size_t data_length)
{
assert(scratch_buffer->size != 0);
if (scratch_buffer->size == 0 || data_length <= 0) {
return -1;
}
size_t old_size = scratch_buffer->size;
if (old_size + data_length > HTTP2_SCRATCH_BUFFER_MAX_SIZE) {
return -1;
Expand All @@ -187,8 +189,6 @@ void reset_scratch_buffer(qd_decoder_buffer_t *scratch_buffer)

int move_to_scratch_buffer(qd_decoder_buffer_t *scratch_buffer, const uint8_t *data, size_t data_length)
{
assert(data_length > 0);

if (scratch_buffer->size > 0) {
return reallocate_move_to_scratch_buffer(scratch_buffer, data, data_length);
} else {
Expand All @@ -198,14 +198,13 @@ int move_to_scratch_buffer(qd_decoder_buffer_t *scratch_buffer, const uint8_t *

static bool is_scratch_buffer_empty(qd_decoder_buffer_t *scratch_buffer)
{
if(!scratch_buffer->bytes)
if (!scratch_buffer->bytes)
return true;
return false;
}

uintptr_t qd_http2_decoder_connection_get_context(const qd_http2_decoder_connection_t *conn_state)
{
assert(conn_state);
return conn_state->user_context;
}

Expand Down Expand Up @@ -252,7 +251,10 @@ void qd_http2_decoder_connection_free(qd_http2_decoder_connection_t *conn_state)
*/
static int decompress_headers(qd_http2_decoder_t *decoder, uint32_t stream_id, const uint8_t *data, size_t length)
{
assert(length > 0);
if (length <= 0) {
qd_log(LOG_HTTP2_DECODER, QD_LOG_ERROR, "[C%"PRIu64"] decompress_headers - passed in length < 0", decoder->conn_state->conn_id);
return -1;
}
qd_http2_decoder_connection_t *conn_state = decoder->conn_state;
if (conn_state->callbacks && conn_state->callbacks->on_begin_header) {
conn_state->callbacks->on_begin_header(conn_state,
Expand Down Expand Up @@ -303,7 +305,6 @@ static int decompress_headers(qd_http2_decoder_t *decoder, uint32_t stream_id, c
*/
static bool is_nth_bit_set(const uint8_t data, int bit)
{
assert(bit < 8);
return ((uint8_t)data) & (uint8_t)(1 << bit);
}

Expand All @@ -318,7 +319,15 @@ static void get_header_flags(const uint8_t data, bool *end_stream, bool *end_hea
bool get_request_headers(qd_http2_decoder_t *decoder)
{
qd_decoder_buffer_t *scratch_buffer = &decoder->scratch_buffer;
assert(scratch_buffer->size > 0);
if (scratch_buffer->size == 0) {
qd_log(LOG_HTTP2_DECODER, QD_LOG_DEBUG, "[C%"PRIu64"] get_request_headers - failure due to zero scratch buffer size, moving decoder state to HTTP2_DECODE_ERROR", decoder->conn_state->conn_id);
static char error[106];
snprintf(error, sizeof(error), "get_request_headers - failure due to zero scratch buffer size, moving decoder state to HTTP2_DECODE_ERROR");
reset_decoder_frame_info(decoder);
reset_scratch_buffer(&decoder->scratch_buffer);
parser_error(decoder, error);
return false;
}
bool end_stream, end_headers, is_padded, has_priority;
get_header_flags(scratch_buffer->bytes[0], &end_stream, &end_headers, &is_padded, &has_priority);
decoder->frame_length_processed += 1;
Expand All @@ -328,14 +337,14 @@ bool get_request_headers(qd_http2_decoder_t *decoder)
scratch_buffer->offset += HTTP2_FRAME_STREAM_ID_LENGTH;
decoder->frame_length_processed += HTTP2_FRAME_STREAM_ID_LENGTH;
uint8_t pad_length = 0;
if(is_padded) {
if (is_padded) {
pad_length = get_pad_length(scratch_buffer->bytes + scratch_buffer->offset);
// Move one byte to account for pad length
scratch_buffer->offset += 1;
decoder->frame_length_processed += 1;
}

if(has_priority) {
if (has_priority) {
// Skip the Stream Dependency field if the priority flag is set
// Stream Dependency field is 4 octets.
scratch_buffer->offset += 4;
Expand All @@ -355,7 +364,7 @@ bool get_request_headers(qd_http2_decoder_t *decoder)
bool valid_pad_length = contains_pad_length > 0;
bool valid_buffer_data = buffer_data_size > 0;
bool valid_pad_length_offset = pad_length_offset > 0;
if(decoder->frame_payload_length == 0 || !valid_buffer_data || !valid_pad_length || !valid_pad_length_offset) {
if (decoder->frame_payload_length == 0 || !valid_buffer_data || !valid_pad_length || !valid_pad_length_offset) {
qd_log(LOG_HTTP2_DECODER, QD_LOG_DEBUG, "[C%"PRIu64"] get_request_headers - failure, moving decoder state to HTTP2_DECODE_ERROR", decoder->conn_state->conn_id);
static char error[130];
snprintf(error, sizeof(error), "get_request_headers - either request or response header was received with zero payload or contains bogus data, stopping decoder");
Expand All @@ -370,7 +379,7 @@ bool get_request_headers(qd_http2_decoder_t *decoder)

// We are now finally at a place which matters to us - The Header block fragment. We will look thru and decompress it so we can get the request/response headers.
int rv = decompress_headers(decoder, stream_id, scratch_buffer->bytes + scratch_buffer->offset, scratch_buffer->size - scratch_buffer->offset);
if(rv < 0) {
if (rv < 0) {
qd_log(LOG_HTTP2_DECODER, QD_LOG_DEBUG, "[C%"PRIu64"] get_request_headers - failure, moving decoder state to HTTP2_DECODE_ERROR", decoder->conn_state->conn_id);
static char error[105];
snprintf(error, sizeof(error), "get_request_headers - failure, moving decoder state to HTTP2_DECODE_ERROR nghttp2 error code=%i", rv);
Expand All @@ -394,7 +403,7 @@ static bool parse_request_header(qd_http2_decoder_t *decoder, const uint8_t **da
if (*length == 0)
return false;
size_t bytes_to_copy = decoder->frame_length - decoder->frame_length_processed;
if(*length < bytes_to_copy) {
if (*length < bytes_to_copy) {
int allocated = move_to_scratch_buffer(&decoder->scratch_buffer, *data, *length);
if (allocated == -1) {
parser_error(decoder, "scratch buffer size exceeded 65535 bytes, stopping decoder");
Expand All @@ -412,9 +421,9 @@ static bool parse_request_header(qd_http2_decoder_t *decoder, const uint8_t **da
}
*data += bytes_to_copy;
*length -= bytes_to_copy;
if((decoder->frame_length_processed + bytes_to_copy) == decoder->frame_length) {
if ((decoder->frame_length_processed + bytes_to_copy) == decoder->frame_length) {
bool header_success = get_request_headers(decoder);
if(!header_success)
if (!header_success)
return false;
}
if (*length > 0) {
Expand Down Expand Up @@ -460,7 +469,7 @@ static bool parse_frame_header(qd_http2_decoder_t *decoder, const uint8_t **data
qd_log(LOG_HTTP2_DECODER, QD_LOG_DEBUG, "[C%"PRIu64"] parse_frame_header - bytes_remaining=%zu", decoder->conn_state->conn_id, bytes_remaining);

if (*length >= bytes_remaining) {
if(bytes_remaining > 0) {
if (bytes_remaining > 0) {
int allocated = move_to_scratch_buffer(&decoder->scratch_buffer, *data, bytes_remaining);
if (allocated == -1) {
parser_error(decoder, "scratch buffer size exceeded 65535 bytes, stopping decoder");
Expand Down Expand Up @@ -524,7 +533,7 @@ static bool parse_frame_header(qd_http2_decoder_t *decoder, const uint8_t **data
static bool parse_client_magic(qd_http2_decoder_t *decoder, bool from_client, const uint8_t **data, size_t *length)
{
if (from_client) {
if(is_scratch_buffer_empty(&decoder->scratch_buffer)) {
if (is_scratch_buffer_empty(&decoder->scratch_buffer)) {
if (*length < HTTP2_CLIENT_PREFIX_LEN) {
// There is not enough length to check client magic.
// Copy the data into the scratch buffer and read it from there next time.
Expand Down Expand Up @@ -552,7 +561,7 @@ static bool parse_client_magic(qd_http2_decoder_t *decoder, bool from_client, co
} else {

// Append this data to the data in the scratch buffer
if(decoder->scratch_buffer.size + *length >= HTTP2_CLIENT_PREFIX_LEN) {
if (decoder->scratch_buffer.size + *length >= HTTP2_CLIENT_PREFIX_LEN) {
// There is already some data in the scratch buffer, there are enough bytes to compare the magic.
// Just move the required number of bytes into the scratch buffer
size_t num_bytes_to_consume = HTTP2_CLIENT_PREFIX_LEN - decoder->scratch_buffer.size;
Expand Down

0 comments on commit ea1de32

Please sign in to comment.