From 489affa74c8a229b8a4dd541710d4a5debedb7b4 Mon Sep 17 00:00:00 2001 From: Carlos Garcia Campos Date: Mon, 16 Feb 2026 12:09:08 +0100 Subject: [PATCH] server: protect message io while reading and writing Ensure the nghttp2 session is not destroyed while being used. Closes #496 --- .../http2/soup-server-message-io-http2.c | 117 +++++++++++++----- tests/http2-test.c | 54 ++++++++ 2 files changed, 141 insertions(+), 30 deletions(-) Index: libsoup-3.4.4/libsoup/server/http2/soup-server-message-io-http2.c =================================================================== --- libsoup-3.4.4.orig/libsoup/server/http2/soup-server-message-io-http2.c +++ libsoup-3.4.4/libsoup/server/http2/soup-server-message-io-http2.c @@ -69,6 +69,8 @@ typedef struct { GHashTable *messages; guint in_callback; + guint protected; + gboolean destroyed; } SoupServerMessageIOHTTP2; static void soup_server_message_io_http2_send_response (SoupServerMessageIOHTTP2 *io, @@ -146,6 +148,8 @@ soup_server_message_io_http2_destroy (So { SoupServerMessageIOHTTP2 *io = (SoupServerMessageIOHTTP2 *)iface; + io->destroyed = TRUE; + if (io->read_source) { g_source_destroy (io->read_source); g_source_unref (io->read_source); @@ -160,10 +164,14 @@ soup_server_message_io_http2_destroy (So } g_clear_object (&io->iostream); - g_clear_pointer (&io->session, nghttp2_session_del); - g_clear_pointer (&io->messages, g_hash_table_unref); + io->istream = NULL; + io->ostream = NULL; - g_free (io); + if (io->protected == 0) { + g_clear_pointer (&io->session, nghttp2_session_del); + g_clear_pointer (&io->messages, g_hash_table_unref); + g_free (io); + } } static void @@ -321,7 +329,33 @@ static const SoupServerMessageIOFuncs io soup_server_message_io_http2_is_paused }; +static void +soup_server_message_io_http2_protect (SoupServerMessageIOHTTP2 *io) +{ + io->protected++; + g_object_ref (io->conn); +} + static gboolean +soup_server_message_io_http2_unprotect (SoupServerMessageIOHTTP2 *io) +{ + g_object_unref (io->conn); + + if (--io->protected > 0) + return FALSE; + + if (io->destroyed) { + g_clear_pointer (&io->session, nghttp2_session_del); + g_clear_pointer (&io->messages, g_hash_table_unref); + g_free (io); + + return TRUE; + } + + return FALSE; +} + +static void io_write (SoupServerMessageIOHTTP2 *io, GError **error) { @@ -336,51 +370,57 @@ io_write (SoupServerMessageIOHTTP2 *io, if (io->write_buffer_size == 0) { /* Done */ io->write_buffer = NULL; - return TRUE; + return; } } + if (!io->ostream) + return; + gssize ret = g_pollable_stream_write (io->ostream, io->write_buffer + io->written_bytes, io->write_buffer_size - io->written_bytes, FALSE, NULL, error); - if (ret < 0) - return FALSE; - - io->written_bytes += ret; - return TRUE; + if (ret > 0) + io->written_bytes += ret; } static gboolean io_write_ready (GObject *stream, SoupServerMessageIOHTTP2 *io) { - SoupServerConnection *conn = io->conn; GError *error = NULL; - g_object_ref (conn); + soup_server_message_io_http2_protect (io); + + while (!error) { + if (io->destroyed) + break; + + if (!nghttp2_session_want_write (io->session)) + break; - while (!error && soup_server_connection_get_io_data (conn) == (SoupServerMessageIO *)io && nghttp2_session_want_write (io->session)) io_write (io, &error); + } if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_WOULD_BLOCK)) { g_error_free (error); - g_object_unref (conn); + soup_server_message_io_http2_unprotect (io); return G_SOURCE_CONTINUE; } - if (soup_server_connection_get_io_data (conn) == (SoupServerMessageIO *)io) { + if (!io->destroyed) { if (error) h2_debug (io, NULL, "[SESSION] IO error: %s", error->message); g_clear_pointer (&io->write_source, g_source_unref); if (error || (!nghttp2_session_want_read (io->session) && !nghttp2_session_want_write (io->session))) - soup_server_connection_disconnect (conn); + soup_server_connection_disconnect (io->conn); } g_clear_error (&error); - g_object_unref (conn); + soup_server_message_io_http2_unprotect (io); return G_SOURCE_REMOVE; } @@ -390,13 +430,12 @@ static gboolean io_write_idle_cb (SoupSe static void io_try_write (SoupServerMessageIOHTTP2 *io) { - SoupServerConnection *conn = io->conn; GError *error = NULL; if (io->write_source) return; - if (io->in_callback && soup_server_connection_get_io_data (conn) == (SoupServerMessageIO *)io) { + if (io->in_callback && !io->destroyed) { if (!nghttp2_session_want_write (io->session)) return; @@ -420,12 +459,19 @@ io_try_write (SoupServerMessageIOHTTP2 * g_clear_pointer (&io->write_idle_source, g_source_unref); } - g_object_ref (conn); + soup_server_message_io_http2_protect (io); + + while (!error) { + if (io->destroyed) + break; + + if (!nghttp2_session_want_write (io->session)) + break; - while (!error && soup_server_connection_get_io_data (conn) == (SoupServerMessageIO *)io && !io->in_callback && nghttp2_session_want_write (io->session)) io_write (io, &error); + } - if (soup_server_connection_get_io_data (conn) == (SoupServerMessageIO *)io) { + if (!io->destroyed) { if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_WOULD_BLOCK)) { g_clear_error (&error); io->write_source = g_pollable_output_stream_create_source (G_POLLABLE_OUTPUT_STREAM (io->ostream), NULL); @@ -442,11 +488,11 @@ io_try_write (SoupServerMessageIOHTTP2 * h2_debug (io, NULL, "[SESSION] IO error: %s", error->message); if (error || (!nghttp2_session_want_read (io->session) && !nghttp2_session_want_write (io->session))) - soup_server_connection_disconnect (conn); + soup_server_connection_disconnect (io->conn); } g_clear_error (&error); - g_object_unref (conn); + soup_server_message_io_http2_unprotect (io); } static gboolean @@ -489,31 +535,37 @@ static gboolean io_read_ready (GObject *stream, SoupServerMessageIOHTTP2 *io) { - SoupServerConnection *conn = io->conn; gboolean progress = TRUE; GError *error = NULL; - g_object_ref (conn); + soup_server_message_io_http2_protect (io); + + while (progress) { + if (io->destroyed) + break; + + if (!nghttp2_session_want_read (io->session)) + break; - while (progress && soup_server_connection_get_io_data (conn) == (SoupServerMessageIO *)io && nghttp2_session_want_read (io->session)) progress = io_read (io, &error); + } if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_WOULD_BLOCK)) { g_error_free (error); - g_object_unref (conn); + soup_server_message_io_http2_unprotect (io); return G_SOURCE_CONTINUE; } - if (soup_server_connection_get_io_data (conn) == (SoupServerMessageIO *)io) { + if (!io->destroyed) { if (error) h2_debug (io, NULL, "[SESSION] IO error: %s", error->message); if (error || (!nghttp2_session_want_read (io->session) && !nghttp2_session_want_write (io->session))) - soup_server_connection_disconnect (conn); + soup_server_connection_disconnect (io->conn); } g_clear_error (&error); - g_object_unref (conn); + soup_server_message_io_http2_unprotect (io); return G_SOURCE_REMOVE; } @@ -938,5 +990,10 @@ soup_server_message_io_http2_new (SoupSe nghttp2_submit_settings (io->session, NGHTTP2_FLAG_NONE, settings, G_N_ELEMENTS (settings)); io_try_write (io); +#ifdef __clang_analyzer__ + // Suppress false positive about io being destroyed here, since at this point we have only + // send the initial settings and not callback is called. + [[clang::suppress]] +#endif return (SoupServerMessageIO *)io; } Index: libsoup-3.4.4/tests/http2-test.c =================================================================== --- libsoup-3.4.4.orig/tests/http2-test.c +++ libsoup-3.4.4/tests/http2-test.c @@ -1259,6 +1259,40 @@ do_broken_pseudo_header_test (Test *test g_uri_unref (uri); } +static void +disconnect_on_got_headers (SoupServerMessage *msg, gpointer user_data) +{ + GUri *uri; + SoupServerConnection *conn; + + uri = soup_server_message_get_uri (msg); + if (!g_str_equal (g_uri_get_path (uri), "/close-on-got-headers")) + return; + + conn = soup_server_message_get_connection (msg); + soup_server_connection_disconnect (conn); +} + +static void +do_server_disconnect_on_got_headers_test (Test *test, gconstpointer data) +{ + SoupMessage *msg; + GUri *uri; + GBytes *response; + GError *error = NULL; + + uri = g_uri_parse_relative (base_uri, "/close-on-got-headers", SOUP_HTTP_URI_FLAGS, NULL); + msg = soup_message_new_from_uri (SOUP_METHOD_GET, uri); + + response = soup_test_session_async_send (test->session, msg, NULL, &error); + g_assert_error (error, G_IO_ERROR, G_IO_ERROR_PARTIAL_INPUT); + + g_clear_error (&error); + g_bytes_unref (response); + g_object_unref (msg); + g_uri_unref (uri); +} + static gboolean unpause_message (SoupServerMessage *msg) { @@ -1378,9 +1412,23 @@ server_handler (SoupServer *serve soup_server_message_set_response (msg, "text/plain", SOUP_MEMORY_STATIC, "Success!", 8); + } else if (strcmp (path, "/close-on-got-headers") == 0) { + soup_server_message_set_response (msg, "text/plain", + SOUP_MEMORY_STATIC, + "Success!", 8); } } +static void +server_request_started (SoupServer *server, + SoupServerMessage *msg, + SoupServerConnection *conn, + gpointer user_data) +{ + g_signal_connect (msg, "got-headers", + G_CALLBACK (disconnect_on_got_headers), NULL); +} + static gboolean server_basic_auth_callback (SoupAuthDomain *auth_domain, SoupServerMessage *msg, @@ -1407,6 +1455,8 @@ main (int argc, char **argv) return 0; server = soup_test_server_new (SOUP_TEST_SERVER_IN_THREAD | SOUP_TEST_SERVER_HTTP2); + g_signal_connect (server, "request-started", + G_CALLBACK (server_request_started), NULL); auth = soup_auth_domain_basic_new ("realm", "http2-test", "auth-callback", server_basic_auth_callback, NULL); @@ -1563,6 +1613,10 @@ main (int argc, char **argv) setup_session, do_broken_pseudo_header_test, teardown_session); + g_test_add ("/http2/server-disconnect-on-got-headers", Test, NULL, + setup_session, + do_server_disconnect_on_got_headers_test, + teardown_session); ret = g_test_run ();