From a83a482fc650da06fbc87f7504e1c9d614a0084a Mon Sep 17 00:00:00 2001 From: Kazuho Oku Date: Wed, 22 Jan 2025 09:36:12 +0900 Subject: [PATCH 1/3] in the tests, use error codes above 65535 to test 64-bit transfer --- t/simple.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/t/simple.c b/t/simple.c index ba507f4b..14408b32 100644 --- a/t/simple.c +++ b/t/simple.c @@ -167,8 +167,8 @@ static void test_reset_then_close(void) ok(ret == 0); stream_id = client_stream->stream_id; client_streambuf = client_stream->data; - quicly_reset_stream(client_stream, QUICLY_ERROR_FROM_APPLICATION_ERROR_CODE(12345)); - quicly_request_stop(client_stream, QUICLY_ERROR_FROM_APPLICATION_ERROR_CODE(54321)); + quicly_reset_stream(client_stream, QUICLY_ERROR_FROM_APPLICATION_ERROR_CODE(1234567)); + quicly_request_stop(client_stream, QUICLY_ERROR_FROM_APPLICATION_ERROR_CODE(7654321)); transmit(client, server); @@ -179,8 +179,8 @@ static void test_reset_then_close(void) server_streambuf = server_stream->data; ok(quicly_sendstate_transfer_complete(&server_stream->sendstate)); ok(quicly_recvstate_transfer_complete(&server_stream->recvstate)); - ok(server_streambuf->error_received.reset_stream == QUICLY_ERROR_FROM_APPLICATION_ERROR_CODE(12345)); - ok(server_streambuf->error_received.stop_sending == QUICLY_ERROR_FROM_APPLICATION_ERROR_CODE(54321)); + ok(server_streambuf->error_received.reset_stream == QUICLY_ERROR_FROM_APPLICATION_ERROR_CODE(1234567)); + ok(server_streambuf->error_received.stop_sending == QUICLY_ERROR_FROM_APPLICATION_ERROR_CODE(7654321)); quic_now += QUICLY_DELAYED_ACK_TIMEOUT; transmit(server, client); @@ -188,7 +188,7 @@ static void test_reset_then_close(void) /* client closes the stream */ ok(client_streambuf->is_detached); ok(client_streambuf->error_received.stop_sending == -1); - ok(client_streambuf->error_received.reset_stream == QUICLY_ERROR_FROM_APPLICATION_ERROR_CODE(54321)); + ok(client_streambuf->error_received.reset_stream == QUICLY_ERROR_FROM_APPLICATION_ERROR_CODE(7654321)); ok(quicly_num_streams(client) == 0); quic_now += QUICLY_DELAYED_ACK_TIMEOUT; @@ -338,7 +338,7 @@ static void tiny_stream_window(void) ok(buffer_is(&server_streambuf->super.ingress, "orld")); ok(quicly_recvstate_transfer_complete(&server_stream->recvstate)); - quicly_request_stop(client_stream, QUICLY_ERROR_FROM_APPLICATION_ERROR_CODE(12345)); + quicly_request_stop(client_stream, QUICLY_ERROR_FROM_APPLICATION_ERROR_CODE(1234567)); transmit(client, server); @@ -348,14 +348,14 @@ static void tiny_stream_window(void) /* client should have sent ACK(FIN),STOP_RESPONDING and waiting for response */ ok(quicly_num_streams(client) == 1); ok(!server_streambuf->is_detached); - ok(server_streambuf->error_received.stop_sending == QUICLY_ERROR_FROM_APPLICATION_ERROR_CODE(12345)); + ok(server_streambuf->error_received.stop_sending == QUICLY_ERROR_FROM_APPLICATION_ERROR_CODE(1234567)); ok(quicly_sendstate_transfer_complete(&server_stream->sendstate)); transmit(server, client); /* client can close the stream when it receives an RESET_STREAM in response */ ok(client_streambuf->is_detached); - ok(client_streambuf->error_received.reset_stream == QUICLY_ERROR_FROM_APPLICATION_ERROR_CODE(12345)); + ok(client_streambuf->error_received.reset_stream == QUICLY_ERROR_FROM_APPLICATION_ERROR_CODE(1234567)); ok(client_streambuf->error_received.stop_sending == -1); ok(quicly_num_streams(client) == 0); ok(quicly_num_streams(server) == 1); @@ -413,13 +413,13 @@ static void test_reset_during_loss(void) } /* transmit RESET_STREAM */ - quicly_reset_stream(client_stream, QUICLY_ERROR_FROM_APPLICATION_ERROR_CODE(12345)); + quicly_reset_stream(client_stream, QUICLY_ERROR_FROM_APPLICATION_ERROR_CODE(1234567)); ok(quicly_sendstate_transfer_complete(&client_stream->sendstate)); transmit(client, server); ok(quicly_recvstate_transfer_complete(&server_stream->recvstate)); - ok(server_streambuf->error_received.reset_stream == QUICLY_ERROR_FROM_APPLICATION_ERROR_CODE(12345)); - quicly_reset_stream(server_stream, QUICLY_ERROR_FROM_APPLICATION_ERROR_CODE(54321)); + ok(server_streambuf->error_received.reset_stream == QUICLY_ERROR_FROM_APPLICATION_ERROR_CODE(1234567)); + quicly_reset_stream(server_stream, QUICLY_ERROR_FROM_APPLICATION_ERROR_CODE(7654321)); ok(!server_streambuf->is_detached); ok(quicly_sendstate_transfer_complete(&server_stream->sendstate)); @@ -443,7 +443,7 @@ static void test_reset_during_loss(void) /* RESET_STREAM for downstream is sent */ transmit(server, client); - ok(client_streambuf->error_received.reset_stream == QUICLY_ERROR_FROM_APPLICATION_ERROR_CODE(54321)); + ok(client_streambuf->error_received.reset_stream == QUICLY_ERROR_FROM_APPLICATION_ERROR_CODE(7654321)); ok(client_streambuf->is_detached); ok(quicly_num_streams(client) == 0); ok(quicly_num_streams(server) == 1); @@ -459,7 +459,7 @@ static void test_reset_during_loss(void) quic_ctx.transport_params.max_stream_data = max_stream_data_orig; } -static uint16_t test_close_error_code; +static uint64_t test_close_error_code; static void test_closed_by_remote(quicly_closed_by_remote_t *self, quicly_conn_t *conn, quicly_error_t err, uint64_t frame_type, const char *reason, size_t reason_len) @@ -484,7 +484,7 @@ static void test_close(void) quic_ctx.closed_by_remote = &closed_by_remote; /* client sends close */ - ret = quicly_close(client, QUICLY_ERROR_FROM_APPLICATION_ERROR_CODE(12345), "good bye"); + ret = quicly_close(client, QUICLY_ERROR_FROM_APPLICATION_ERROR_CODE(1234567), "good bye"); ok(ret == 0); ok(quicly_get_state(client) == QUICLY_STATE_CLOSING); ok(quicly_get_first_timeout(client) <= quic_now); @@ -499,7 +499,7 @@ static void test_close(void) decode_packets(&decoded, &datagram, 1); ret = quicly_receive(server, NULL, &fake_address.sa, &decoded); ok(ret == 0); - ok(test_close_error_code == 12345); + ok(test_close_error_code == 1234567); ok(quicly_get_state(server) == QUICLY_STATE_DRAINING); server_timeout = quicly_get_first_timeout(server); ok(quic_now < server_timeout && server_timeout < quic_now + 1000); /* 3 pto or something */ From a35cc6672bbb219aaa4581c969822091b11cfd26 Mon Sep 17 00:00:00 2001 From: Kazuho Oku Date: Wed, 22 Jan 2025 09:43:52 +0900 Subject: [PATCH 2/3] fix uint16_t fields that cause truncation --- include/quicly.h | 4 ++-- lib/quicly.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/include/quicly.h b/include/quicly.h index d021e1ab..dea5e1d9 100644 --- a/include/quicly.h +++ b/include/quicly.h @@ -836,7 +836,7 @@ struct st_quicly_stream_t { */ struct { quicly_sender_state_t sender_state; - uint16_t error_code; + uint64_t error_code; } stop_sending; /** * reset_stream @@ -846,7 +846,7 @@ struct st_quicly_stream_t { * STATE_NONE until RST is generated */ quicly_sender_state_t sender_state; - uint16_t error_code; + uint64_t error_code; } reset_stream; /** * sends receive window updates to remote peer diff --git a/lib/quicly.c b/lib/quicly.c index cb09ee79..28a4761f 100644 --- a/lib/quicly.c +++ b/lib/quicly.c @@ -310,7 +310,7 @@ struct st_quicly_conn_t { * valid if state is CLOSING */ struct { - uint16_t error_code; + uint64_t error_code; uint64_t frame_type; /* UINT64_MAX if application close */ const char *reason_phrase; unsigned long num_packets_received; @@ -5783,7 +5783,7 @@ static quicly_error_t enter_close(quicly_conn_t *conn, int local_is_initiating, quicly_error_t initiate_close(quicly_conn_t *conn, quicly_error_t err, uint64_t frame_type, const char *reason_phrase) { - uint16_t quic_error_code; + uint64_t quic_error_code; if (conn->super.state >= QUICLY_STATE_CLOSING) return 0; From 4199947b3e87f022f49e69f83da565911b59d23c Mon Sep 17 00:00:00 2001 From: Kazuho Oku Date: Wed, 22 Jan 2025 10:22:34 +0900 Subject: [PATCH 3/3] increase in field size reveals a bug that was previously hidden by 16-bit truncation --- lib/quicly.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/quicly.c b/lib/quicly.c index 28a4761f..7b5796a0 100644 --- a/lib/quicly.c +++ b/lib/quicly.c @@ -5801,7 +5801,7 @@ quicly_error_t initiate_close(quicly_conn_t *conn, quicly_error_t err, uint64_t quic_error_code = QUICLY_ERROR_GET_ERROR_CODE(err); frame_type = UINT64_MAX; } else if (PTLS_ERROR_GET_CLASS(err) == PTLS_ERROR_CLASS_SELF_ALERT) { - quic_error_code = QUICLY_TRANSPORT_ERROR_CRYPTO(PTLS_ERROR_TO_ALERT(err)); + quic_error_code = QUICLY_ERROR_GET_ERROR_CODE(QUICLY_TRANSPORT_ERROR_CRYPTO(PTLS_ERROR_TO_ALERT(err))); } else { quic_error_code = QUICLY_ERROR_GET_ERROR_CODE(QUICLY_TRANSPORT_ERROR_INTERNAL); }