diff --git a/src/transport/xqc_conn.h b/src/transport/xqc_conn.h index ed9206715..a9dd16e38 100644 --- a/src/transport/xqc_conn.h +++ b/src/transport/xqc_conn.h @@ -39,6 +39,16 @@ #define XQC_MAX_RECV_WINDOW (16 * 1024 * 1024) +/* RFC 9000: variable-length integer max value, used as flow control upper bound */ +#define XQC_MAX_FLOW_CONTROL_WINDOW (((uint64_t)1 << 62) - 1) + +/* clamp a flow control value to the RFC 9000 variable-length integer maximum */ +static inline uint64_t +xqc_clamp_to_max_flow_ctl(uint64_t value) +{ + return value > XQC_MAX_FLOW_CONTROL_WINDOW ? XQC_MAX_FLOW_CONTROL_WINDOW : value; +} + #define XQC_MP_SETTINGS_STR_LEN (30) static const uint32_t MAX_RSP_CONN_CLOSE_CNT = 3; diff --git a/src/transport/xqc_frame.c b/src/transport/xqc_frame.c index 33d5619a3..5f8833445 100644 --- a/src/transport/xqc_frame.c +++ b/src/transport/xqc_frame.c @@ -1268,7 +1268,7 @@ xqc_process_data_blocked_frame(xqc_connection_t *conn, xqc_packet_in_t *packet_i return XQC_OK; } - new_limit = conn->conn_flow_ctl.fc_data_read + conn->conn_flow_ctl.fc_recv_windows_size; + new_limit = xqc_clamp_to_max_flow_ctl(conn->conn_flow_ctl.fc_data_read + conn->conn_flow_ctl.fc_recv_windows_size); if (new_limit > conn->conn_flow_ctl.fc_max_data_can_recv) { conn->conn_flow_ctl.fc_max_data_can_recv = new_limit; @@ -1324,7 +1324,7 @@ xqc_process_stream_data_blocked_frame(xqc_connection_t *conn, xqc_packet_in_t *p return XQC_OK; } - new_limit = stream->stream_data_in.next_read_offset + stream->stream_flow_ctl.fc_stream_recv_window_size; + new_limit = xqc_clamp_to_max_flow_ctl(stream->stream_data_in.next_read_offset + stream->stream_flow_ctl.fc_stream_recv_window_size); if (new_limit > stream->stream_flow_ctl.fc_max_stream_data_can_recv) { stream->stream_flow_ctl.fc_max_stream_data_can_recv = new_limit; diff --git a/src/transport/xqc_packet_out.c b/src/transport/xqc_packet_out.c index b564cc947..08b09003f 100644 --- a/src/transport/xqc_packet_out.c +++ b/src/transport/xqc_packet_out.c @@ -1215,6 +1215,7 @@ xqc_write_stream_frame_to_packet(xqc_connection_t *conn, if (stream->stream_flow_ctl.fc_stream_recv_window_size > available_window) { stream->stream_flow_ctl.fc_max_stream_data_can_recv += (stream->stream_flow_ctl.fc_stream_recv_window_size - available_window); + stream->stream_flow_ctl.fc_max_stream_data_can_recv = xqc_clamp_to_max_flow_ctl(stream->stream_flow_ctl.fc_max_stream_data_can_recv); xqc_log(conn->log, XQC_LOG_DEBUG, "|initial_fc_credit_update|stream:%ui|new_max_data:%ui|stream_max_recv_offset:%ui|next_read_offset:%ui|window_size:%ui|pkt_type:%d|", stream->stream_id, diff --git a/src/transport/xqc_stream.c b/src/transport/xqc_stream.c index 479a6fcd4..feae94a44 100644 --- a/src/transport/xqc_stream.c +++ b/src/transport/xqc_stream.c @@ -416,6 +416,7 @@ xqc_stream_do_recv_flow_ctl(xqc_stream_t *stream) if (stream->stream_flow_ctl.fc_stream_recv_window_size > available_window) { stream->stream_flow_ctl.fc_max_stream_data_can_recv += (stream->stream_flow_ctl.fc_stream_recv_window_size - available_window); + stream->stream_flow_ctl.fc_max_stream_data_can_recv = xqc_clamp_to_max_flow_ctl(stream->stream_flow_ctl.fc_max_stream_data_can_recv); xqc_log(conn->log, XQC_LOG_DEBUG, "|xqc_write_max_stream_data_to_packet|new_max_data:%ui|stream_max_recv_offset:%ui|next_read_offset:%ui|window_size:%ui|", stream->stream_flow_ctl.fc_max_stream_data_can_recv, stream->stream_max_recv_offset, @@ -462,6 +463,7 @@ xqc_stream_do_recv_flow_ctl(xqc_stream_t *stream) if (conn->conn_flow_ctl.fc_recv_windows_size > available_window) { conn->conn_flow_ctl.fc_max_data_can_recv += (conn->conn_flow_ctl.fc_recv_windows_size - available_window); + conn->conn_flow_ctl.fc_max_data_can_recv = xqc_clamp_to_max_flow_ctl(conn->conn_flow_ctl.fc_max_data_can_recv); xqc_log(conn->log, XQC_LOG_DEBUG, "|xqc_write_max_data_to_packet|new_max_data:%ui|fc_data_recved:%ui|fc_data_read:%ui|window_size:%ui|", conn->conn_flow_ctl.fc_max_data_can_recv, conn->conn_flow_ctl.fc_data_recved, @@ -1052,7 +1054,7 @@ xqc_stream_update_settings(xqc_stream_t *stream, xqc_log(conn->log, XQC_LOG_DEBUG, "|fc_win_update|old_fc_win:%ui|fc_win:%ui|", old_fc_win, stream->stream_flow_ctl.fc_stream_recv_window_size); - new_offset = stream->stream_data_in.next_read_offset + stream->stream_flow_ctl.fc_stream_recv_window_size; + new_offset = xqc_clamp_to_max_flow_ctl(stream->stream_data_in.next_read_offset + stream->stream_flow_ctl.fc_stream_recv_window_size); if(new_offset > stream->stream_flow_ctl.fc_max_stream_data_can_recv) { stream->stream_flow_ctl.fc_max_stream_data_can_recv = new_offset; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index f72fb508a..f2a7fd448 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -116,6 +116,7 @@ if(HAVE_CUNIT) ${UNIT_TEST_DIR}/xqc_send_ctl_test.c ${UNIT_TEST_DIR}/xqc_vn_test.c ${UNIT_TEST_DIR}/xqc_frame_type_bit_test.c + ${UNIT_TEST_DIR}/xqc_flow_ctl_test.c ) if(XQC_ENABLE_FEC) diff --git a/tests/unittest/main.c b/tests/unittest/main.c index b6b4a4e86..0b99ac79e 100644 --- a/tests/unittest/main.c +++ b/tests/unittest/main.c @@ -44,6 +44,7 @@ #include "xqc_send_ctl_test.h" #include "xqc_vn_test.h" #include "xqc_frame_type_bit_test.h" +#include "xqc_flow_ctl_test.h" static int xqc_init_suite(void) { return 0; } static int xqc_clean_suite(void) { return 0; } @@ -85,6 +86,10 @@ main() || !CU_add_test(pSuite, "xqc_test_pq", xqc_test_pq) || !CU_add_test(pSuite, "xqc_test_common", xqc_test_common) || !CU_add_test(pSuite, "xqc_test_vint", xqc_test_vint) + /* issue #675: flow control window clamp to 2^62-1 (RFC 9000) */ + || !CU_add_test(pSuite, "xqc_test_stream_flow_ctl_clamp", xqc_test_stream_flow_ctl_clamp) + || !CU_add_test(pSuite, "xqc_test_conn_flow_ctl_clamp", xqc_test_conn_flow_ctl_clamp) + || !CU_add_test(pSuite, "xqc_test_flow_ctl_normal_no_clamp", xqc_test_flow_ctl_normal_no_clamp) || !CU_add_test(pSuite, "xqc_test_recv_record", xqc_test_recv_record) || !CU_add_test(pSuite, "xqc_test_reno", xqc_test_reno) || !CU_add_test(pSuite, "xqc_test_reno_init_cwnd", xqc_test_reno_init_cwnd) diff --git a/tests/unittest/xqc_flow_ctl_test.c b/tests/unittest/xqc_flow_ctl_test.c new file mode 100644 index 000000000..ba27039c9 --- /dev/null +++ b/tests/unittest/xqc_flow_ctl_test.c @@ -0,0 +1,91 @@ +/** + * @copyright Copyright (c) 2022, Alibaba Group Holding Limited + */ + +#include "xqc_flow_ctl_test.h" +#include "src/transport/xqc_conn.h" +#include "src/transport/xqc_stream.h" +#include +#include + +/** + * Test: stream-level fc_max_stream_data_can_recv is clamped to 2^62-1. + * Simulates the accumulation logic in xqc_stream_do_recv_flow_ctl and + * verifies the clamp prevents overflow beyond XQC_MAX_FLOW_CONTROL_WINDOW. + */ +void +xqc_test_stream_flow_ctl_clamp(void) +{ + /* XQC_MAX_FLOW_CONTROL_WINDOW must equal 2^62 - 1 */ + CU_ASSERT_EQUAL(XQC_MAX_FLOW_CONTROL_WINDOW, ((uint64_t)1 << 62) - 1); + + xqc_stream_flow_ctl_t flow_ctl; + memset(&flow_ctl, 0, sizeof(flow_ctl)); + + /* set fc_max_stream_data_can_recv near the limit */ + flow_ctl.fc_max_stream_data_can_recv = XQC_MAX_FLOW_CONTROL_WINDOW - 100; + flow_ctl.fc_stream_recv_window_size = XQC_MAX_RECV_WINDOW; + + /* simulate the accumulation that happens in xqc_stream_do_recv_flow_ctl */ + uint64_t available_window = 0; /* worst case: all consumed */ + uint64_t increment = flow_ctl.fc_stream_recv_window_size - available_window; + flow_ctl.fc_max_stream_data_can_recv += increment; + + /* apply clamp (mirrors the fix in xqc_stream.c) */ + if (flow_ctl.fc_max_stream_data_can_recv > XQC_MAX_FLOW_CONTROL_WINDOW) { + flow_ctl.fc_max_stream_data_can_recv = XQC_MAX_FLOW_CONTROL_WINDOW; + } + + CU_ASSERT_EQUAL(flow_ctl.fc_max_stream_data_can_recv, XQC_MAX_FLOW_CONTROL_WINDOW); +} + +/** + * Test: connection-level fc_max_data_can_recv is clamped to 2^62-1. + */ +void +xqc_test_conn_flow_ctl_clamp(void) +{ + xqc_conn_flow_ctl_t flow_ctl; + memset(&flow_ctl, 0, sizeof(flow_ctl)); + + /* set near the limit */ + flow_ctl.fc_max_data_can_recv = XQC_MAX_FLOW_CONTROL_WINDOW - 50; + flow_ctl.fc_recv_windows_size = XQC_MAX_RECV_WINDOW; + + /* simulate accumulation */ + uint64_t available_window = 0; + uint64_t increment = flow_ctl.fc_recv_windows_size - available_window; + flow_ctl.fc_max_data_can_recv += increment; + + /* apply clamp */ + if (flow_ctl.fc_max_data_can_recv > XQC_MAX_FLOW_CONTROL_WINDOW) { + flow_ctl.fc_max_data_can_recv = XQC_MAX_FLOW_CONTROL_WINDOW; + } + + CU_ASSERT_EQUAL(flow_ctl.fc_max_data_can_recv, XQC_MAX_FLOW_CONTROL_WINDOW); +} + +/** + * Test: normal values well below 2^62-1 are not affected by the clamp. + */ +void +xqc_test_flow_ctl_normal_no_clamp(void) +{ + xqc_stream_flow_ctl_t stream_fc; + memset(&stream_fc, 0, sizeof(stream_fc)); + + stream_fc.fc_max_stream_data_can_recv = 1024 * 1024; /* 1MB */ + stream_fc.fc_stream_recv_window_size = XQC_MAX_RECV_WINDOW; + + uint64_t expected = stream_fc.fc_max_stream_data_can_recv + stream_fc.fc_stream_recv_window_size; + + /* simulate accumulation + clamp */ + stream_fc.fc_max_stream_data_can_recv += stream_fc.fc_stream_recv_window_size; + if (stream_fc.fc_max_stream_data_can_recv > XQC_MAX_FLOW_CONTROL_WINDOW) { + stream_fc.fc_max_stream_data_can_recv = XQC_MAX_FLOW_CONTROL_WINDOW; + } + + /* normal value should pass through unchanged */ + CU_ASSERT_EQUAL(stream_fc.fc_max_stream_data_can_recv, expected); + CU_ASSERT(stream_fc.fc_max_stream_data_can_recv < XQC_MAX_FLOW_CONTROL_WINDOW); +} diff --git a/tests/unittest/xqc_flow_ctl_test.h b/tests/unittest/xqc_flow_ctl_test.h new file mode 100644 index 000000000..e307935ce --- /dev/null +++ b/tests/unittest/xqc_flow_ctl_test.h @@ -0,0 +1,12 @@ +/** + * @copyright Copyright (c) 2022, Alibaba Group Holding Limited + */ + +#ifndef _XQC_TEST_FLOW_CTL_H_INCLUDED_ +#define _XQC_TEST_FLOW_CTL_H_INCLUDED_ + +void xqc_test_stream_flow_ctl_clamp(void); +void xqc_test_conn_flow_ctl_clamp(void); +void xqc_test_flow_ctl_normal_no_clamp(void); + +#endif /* _XQC_TEST_FLOW_CTL_H_INCLUDED_ */