Skip to content

Fix rounding error in hpack resizing #559

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.

Sign up for GitHub

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

Open
DmitriyMusatkin wants to merge 3 commits into
base: main
Choose a base branch
from
+31 −4
Open

Fix rounding error in hpack resizing #559

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
56bc097
add test
DmitriyMusatkin May 18, 2026
8e89af3
fix rounding
DmitriyMusatkin May 18, 2026
3a75462
typo
DmitriyMusatkin May 18, 2026
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
6 changes: 2 additions & 4 deletions source/hpack.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,6 @@ const size_t s_hpack_dynamic_table_initial_elements = 512;
/* TODO: shouldn't be a hardcoded max_size, it should be driven by SETTINGS_HEADER_TABLE_SIZE */
const size_t s_hpack_dynamic_table_max_size = 16 * 1024 * 1024;

/* Used for growing the dynamic table buffer when it fills up */
const float s_hpack_dynamic_table_buffer_growth_rate = 1.5F;

struct aws_http_header s_static_header_table[] = {
#define HEADER(_index, _name) \
[_index] = { \
Expand Down Expand Up @@ -421,7 +418,8 @@ int aws_hpack_insert_header(struct aws_hpack_context *context, const struct aws_
/* If the buffer is currently of 0 size, reset it back to its initial size */
const size_t new_size =
context->dynamic_table.buffer_capacity
? (size_t)(context->dynamic_table.buffer_capacity * s_hpack_dynamic_table_buffer_growth_rate)
/* increase buffer by 1.5 rounded up. */
? (size_t)(context->dynamic_table.buffer_capacity + ((context->dynamic_table.buffer_capacity + 1) / 2))
Copy link
Copy Markdown
Contributor

@TingDaoK TingDaoK May 18, 2026

Choose a reason for hiding this comment

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

we cannot get a 0 buffer_capacity?

Will it be more clear if we do a min(context->dynamic_table.buffer_capacity / 2, 1)?

Copy link
Copy Markdown
Contributor Author

@DmitriyMusatkin DmitriyMusatkin May 18, 2026

Choose a reason for hiding this comment

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

from my reading of code, s_dynamic_table_resize_buffer supports new size of 0, which will result in buffer_capacity being 0.
wouldn't that min always result in 1?

: s_hpack_dynamic_table_initial_elements;

if (s_dynamic_table_resize_buffer(context, new_size)) {
Expand Down
1 change: 1 addition & 0 deletions tests/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,7 @@ add_test_case(hpack_dynamic_table_get)
add_test_case(hpack_decode_indexed_from_dynamic_table)
add_test_case(hpack_dynamic_table_empty_value)
add_test_case(hpack_dynamic_table_with_empty_header)
add_test_case(hpack_dynamic_table_growth_corner_case)
add_test_case(hpack_dynamic_table_size_update_from_setting)

if(ENABLE_LOCALHOST_INTEGRATION_TESTS)
Expand Down
28 changes: 28 additions & 0 deletions tests/test_hpack.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -815,6 +815,34 @@ static int test_hpack_dynamic_table_with_empty_header(struct aws_allocator *allo
return AWS_OP_SUCCESS;
}

AWS_TEST_CASE(hpack_dynamic_table_growth_corner_case, test_hpack_dynamic_table_growth_corner_case)
static int test_hpack_dynamic_table_growth_corner_case(struct aws_allocator *allocator, void *ctx) {
(void)ctx;

aws_http_library_init(allocator);
struct aws_hpack_context context;
aws_hpack_context_init(&context, allocator, AWS_LS_HTTP_GENERAL, NULL);

DEFINE_STATIC_HEADER(h1, "herp", "derp");
DEFINE_STATIC_HEADER(h2, "fizz", "buzz");

const size_t h1_size = aws_hpack_get_header_size(&h1);
const size_t h2_size = aws_hpack_get_header_size(&h2);

ASSERT_SUCCESS(aws_hpack_insert_header(&context, &h1));
ASSERT_UINT_EQUALS(1, aws_hpack_get_dynamic_table_num_elements(&context));

ASSERT_SUCCESS(aws_hpack_resize_dynamic_table(&context, h1_size + h2_size + 1));
ASSERT_UINT_EQUALS(1, aws_hpack_get_dynamic_table_num_elements(&context));

ASSERT_SUCCESS(aws_hpack_insert_header(&context, &h2));
ASSERT_UINT_EQUALS(2, aws_hpack_get_dynamic_table_num_elements(&context));

aws_hpack_context_clean_up(&context);
aws_http_library_clean_up();
return AWS_OP_SUCCESS;
}

AWS_TEST_CASE(hpack_dynamic_table_size_update_from_setting, test_hpack_dynamic_table_size_update_from_setting)
static int test_hpack_dynamic_table_size_update_from_setting(struct aws_allocator *allocator, void *ctx) {
(void)ctx;
Expand Down
Loading