Merge pull request #1791 from private-octopus/code-coverage-202411

Add test of h3zero client data processing

CMakeLists.txt

@@ -8,7 +8,7 @@ else()
 endif()
 
 project(picoquic
-        VERSION 1.1.28.5
+        VERSION 1.1.28.6
         DESCRIPTION "picoquic library"
         LANGUAGES C CXX)
 

UnitTest1/unittest1.cpp

@@ -2772,6 +2772,24 @@ namespace UnitTest1
             Assert::AreEqual(ret, 0);
         }
 
+        TEST_METHOD(h3zero_unidir_error) {
+            int ret = h3zero_unidir_error_test();
+
+            Assert::AreEqual(ret, 0);
+        }
+
+        TEST_METHOD(h3zero_setting_error) {
+            int ret = h3zero_setting_error_test();
+
+            Assert::AreEqual(ret, 0);
+        }
+
+        TEST_METHOD(h3zero_client_data) {
+            int ret = h3zero_client_data_test();
+
+            Assert::AreEqual(ret, 0);
+        }
+
         TEST_METHOD(qpack_huffman) {
             int ret = qpack_huffman_test();
 

picohttp/h3zero_common.c

@@ -63,6 +63,9 @@ static void picohttp_clear_stream_ctx(h3zero_stream_ctx_t* stream_ctx)
 	if (stream_ctx->F != NULL) {
 		stream_ctx->F = picoquic_file_close(stream_ctx->F);
 	}
+	if (stream_ctx->F != NULL) {
+		stream_ctx->F = picoquic_file_close(stream_ctx->F);
+	}
 
 	if (stream_ctx->path_callback != NULL) {
 		(void)stream_ctx->path_callback(stream_ctx->cnx, NULL, 0, picohttp_callback_free, stream_ctx, stream_ctx->path_callback_ctx);
@@ -254,6 +257,8 @@ void h3zero_delete_all_stream_prefixes(picoquic_cnx_t * cnx, h3zero_callback_ctx
 	}
 }
 
+#if 0
+/* Unused code */
 uint64_t h3zero_parse_stream_prefix(uint8_t* buffer_8, size_t* nb_in_buffer, uint8_t* data, size_t data_length, size_t * nb_read)
 {
 	uint64_t prefix = UINT64_MAX;
@@ -274,6 +279,7 @@ uint64_t h3zero_parse_stream_prefix(uint8_t* buffer_8, size_t* nb_in_buffer, uin
 
 	return prefix;
 }
+#endif
 
 int h3zero_protocol_init(picoquic_cnx_t* cnx)
 {
@@ -475,7 +481,7 @@ static uint8_t* h3zero_parse_control_stream(uint8_t* bytes, uint8_t* bytes_max,
 	return bytes;
 }
 
-uint8_t* h3zero_parse_control_stream_id(
+uint8_t* h3zero_wt_parse_control_stream_id(
 	uint8_t* bytes, uint8_t* bytes_max,
 	h3zero_data_stream_state_t* stream_state,
 	h3zero_stream_ctx_t* stream_ctx,
@@ -525,7 +531,7 @@ uint8_t* h3zero_parse_remote_bidir_stream(
 		}
 	}
 	if (stream_state->stream_type == h3zero_frame_webtransport_stream) {
-		bytes = h3zero_parse_control_stream_id(bytes, bytes_max, stream_state, stream_ctx, ctx);
+		bytes = h3zero_wt_parse_control_stream_id(bytes, bytes_max, stream_state, stream_ctx, ctx);
 	}
 	else {
 		/* Not and expected stream */
@@ -567,7 +573,7 @@ uint8_t* h3zero_parse_remote_unidir_stream(
 		bytes = bytes_max;
 		break;
 	case h3zero_stream_type_webtransport: /* unidir stream is used as specified in web transport */
-		bytes = h3zero_parse_control_stream_id(bytes, bytes_max, stream_state, stream_ctx, ctx);
+		bytes = h3zero_wt_parse_control_stream_id(bytes, bytes_max, stream_state, stream_ctx, ctx);
 		break;
 	default:
 		/* Per section 6.2 of RFC 9114, unknown stream types are just ignored */
@@ -1152,17 +1158,16 @@ int h3zero_client_open_stream_file(picoquic_cnx_t* cnx, h3zero_callback_ctx_t* c
 {
 	int ret = 0;
 
-	if (!stream_ctx->is_file_open && ctx->no_disk == 0) {
+	if (stream_ctx->F == NULL && ctx->no_disk == 0) {
 		int last_err = 0;
-		stream_ctx->F = picoquic_file_open_ex(stream_ctx->f_name, "wb", &last_err);
-		if (stream_ctx->F == NULL) {
+		stream_ctx->F = picoquic_file_open_ex(stream_ctx->file_path, "wb", &last_err);
+		if (stream_ctx->F== NULL) {
 			picoquic_log_app_message(cnx,
-				"Could not open file <%s> for stream %" PRIu64 ", error %d (0x%x)\n", stream_ctx->f_name, stream_ctx->stream_id, last_err, last_err);
-			DBG_PRINTF("Could not open file <%s> for stream %" PRIu64 ", error %d (0x%x)", stream_ctx->f_name, stream_ctx->stream_id, last_err, last_err);
+				"Could not open file <%s> for stream %" PRIu64 ", error %d (0x%x)\n", stream_ctx->file_path, stream_ctx->stream_id, last_err, last_err);
+			DBG_PRINTF("Could not open file <%s> for stream %" PRIu64 ", error %d (0x%x)", stream_ctx->file_path, stream_ctx->stream_id, last_err, last_err);
 			ret = -1;
 		}
 		else {
-			stream_ctx->is_file_open = 1;
 			ctx->nb_open_files++;
 		}
 	}
@@ -1175,16 +1180,16 @@ int h3zero_client_close_stream(picoquic_cnx_t * cnx,
 	h3zero_callback_ctx_t* ctx, h3zero_stream_ctx_t* stream_ctx)
 {
 	int ret = 0;
-	if (stream_ctx != NULL && stream_ctx->is_open) {
+	if (stream_ctx != NULL) {
 		picoquic_unlink_app_stream_ctx(cnx, stream_ctx->stream_id);
-		if (stream_ctx->f_name != NULL) {
-			free(stream_ctx->f_name);
-			stream_ctx->f_name = NULL;
+
+		if (stream_ctx->file_path != NULL) {
+			free(stream_ctx->file_path);
+			stream_ctx->file_path = NULL;
 		}
-		stream_ctx->F = picoquic_file_close(stream_ctx->F);
-		if (stream_ctx->is_file_open) {
+		if (stream_ctx->F != NULL) {
+			stream_ctx->F = picoquic_file_close(stream_ctx->F);
 			ctx->nb_open_files--;
-			stream_ctx->is_file_open = 0;
 		}
 		stream_ctx->is_open = 0;
 		ctx->nb_open_streams--; 
@@ -1295,7 +1300,7 @@ int h3zero_process_h3_client_data(picoquic_cnx_t* cnx,
 	h3zero_stream_ctx_t* stream_ctx, uint64_t* fin_stream_id)
 {
 	int ret = 0;
-	if (!stream_ctx->is_file_open && ctx->no_disk == 0 && stream_ctx->file_path != NULL) {
+	if (stream_ctx->F == NULL && ctx->no_disk == 0 && stream_ctx->file_path != NULL) {
 		ret = h3zero_client_open_stream_file(cnx, ctx, stream_ctx);
 	}
 	if (ret == 0 && length > 0) {
@@ -1355,7 +1360,12 @@ int h3zero_process_h3_client_data(picoquic_cnx_t* cnx,
 			stream_ctx->path_callback(cnx, NULL, 0, picohttp_callback_post_fin, stream_ctx, stream_ctx->path_callback_ctx);
 		}
 		else {
-			if (h3zero_client_close_stream(cnx, ctx, stream_ctx)) {
+			if (stream_ctx->ps.stream_state.current_frame_read < stream_ctx->ps.stream_state.current_frame_length) {
+				ret = picoquic_close(cnx, H3ZERO_FRAME_ERROR);
+				picoquic_log_app_message(cnx,
+					"Stream %" PRIu64 " closed when a frame is not complete, error 0x%x", stream_id, H3ZERO_FRAME_ERROR);
+			}
+			else if (h3zero_client_close_stream(cnx, ctx, stream_ctx)) {
 				*fin_stream_id = stream_id;
 				if (stream_id <= 64 && !ctx->no_print) {
 					fprintf(stdout, "Stream %" PRIu64 " ended after %" PRIu64 " bytes\n",

picohttp/h3zero_common.h

@@ -101,20 +101,20 @@ extern "C" {
         uint64_t echo_length;
         uint64_t echo_sent;
         uint64_t post_received;
-        /* Client state file management */
-        unsigned int is_open : 1;
-        unsigned int is_file_open : 1;
-        unsigned int flow_opened : 1;
+        picohttp_post_data_cb_fn path_callback;
+        void* path_callback_ctx;
+        uint8_t frame[PICOHTTP_SERVER_FRAME_MAX];
+        /* Client state management */
+        unsigned int is_open : 1; /* The client has initiated this stream */
+        unsigned int flow_opened : 1; /* Flow control parameters updated to allow receiving expected data */
         uint64_t received_length;
         uint64_t post_size;
         uint64_t post_sent;
-        char* f_name;
-        /* Global state variables */
-        uint8_t frame[PICOHTTP_SERVER_FRAME_MAX];
+        //char* f_name;
+        //FILE* FC;
+        /* File state variables, used by both cclient and server */
         char* file_path;
         FILE* F;
-        picohttp_post_data_cb_fn path_callback;
-        void* path_callback_ctx;
     } h3zero_stream_ctx_t;
 
     /* Parsing of a data stream. This is implemented as a filter, with a set of states:

picohttp_t/picohttp_t.c

@@ -48,6 +48,9 @@ static const picoquic_test_def_t test_table[] = {
     { "h3zero_integer", h3zero_integer_test },
     { "h3zero_varint_stream", h3zero_varint_stream_test },
     { "h3zero_incoming_unidir", h3zero_incoming_unidir_test },
+    { "h3zero_unidir_error", h3zero_unidir_error_test },
+    { "h3zero_setting_error", h3zero_setting_error_test },
+    { "h3zero_client_data", h3zero_client_data_test },
     { "qpack_huffman", qpack_huffman_test },
     { "qpack_huffman_base", qpack_huffman_base_test},
     { "h3zero_parse_qpack", h3zero_parse_qpack_test },

picoquic/picoquic.h

@@ -40,7 +40,7 @@
 extern "C" {
 #endif
 
-#define PICOQUIC_VERSION "1.1.28.5"
+#define PICOQUIC_VERSION "1.1.28.6"
 #define PICOQUIC_ERROR_CLASS 0x400
 #define PICOQUIC_ERROR_DUPLICATE (PICOQUIC_ERROR_CLASS + 1)
 #define PICOQUIC_ERROR_AEAD_CHECK (PICOQUIC_ERROR_CLASS + 3)