CI: Run the mod_http3 tests

.github/workflows/linux.yml

@@ -179,6 +179,20 @@ jobs:
               TEST_PYTEST=1
               PYTEST_ARGS=modules/http2
           # -------------------------------------------------------------------------
+          # mod_http3 needs OpenSSL >= 3.5 and libnghttp3, which Ubuntu
+          # 26.04 has. Its curl has no HTTP/3, so one is built from source.
+          # Strict mode turns a skip for a missing dependency into a failure.
+          - name: mod_http3
+            os: ubuntu-26.04
+            config: --enable-mods-shared=reallyall --enable-maintainer-mode --with-mpm=event --enable-http3
+            pkgs: libnghttp3-dev
+            env: |
+              TEST_CURL_H3=8.18.0
+              NO_TEST_FRAMEWORK=1
+              TEST_PYTEST=1
+              PYTEST_ARGS=modules/http3
+              PYHTTPD_STRICT_OPTIONAL=1
+          # -------------------------------------------------------------------------
           - name: Default
             # Check the defaults expected for a default build on Linux.
             config-output: >-

modules/http3/config2.m4

@@ -139,7 +139,8 @@ AC_DEFUN([APACHE_CHECK_NGHTTP3],[
       dnl Run library and function checks
       liberrors=""
       AC_CHECK_HEADERS([nghttp3/nghttp3.h])
-      AC_CHECK_FUNCS([nghttp3_conn_server_new], [], [liberrors="yes"])
+      dnl nghttp3_conn_server_new() is a macro around the versioned function
+      AC_CHECK_FUNCS([nghttp3_conn_server_new_versioned], [], [liberrors="yes"])
       if test "x$liberrors" != "x"; then
         AC_MSG_WARN([nghttp3 library is unusable])
       fi

modules/http3/h3_io.c

@@ -45,6 +45,13 @@
 #include "quic/h3q_conn.h"
 #include "quic/h3q_stream.h"
 
+#if NGHTTP3_VERSION_NUM >= 0x011000
+#define H3_NGHTTP3_CONN_IS_DRAINED(conn) nghttp3_conn_is_drained2(conn)
+#else
+/* nghttp3 < 1.16.0 has the same function, without the const argument */
+#define H3_NGHTTP3_CONN_IS_DRAINED(conn) nghttp3_conn_is_drained(conn)
+#endif
+
 h3_io_t* child_h3_io = NULL;
 
 int h3_io_at_connection_limit(h3_io_t* io)
@@ -155,15 +162,17 @@ apr_status_t h3_io_listen_start(apr_pool_t* pchild, server_rec* s, h3_server_con
 
     io->thread_running = 1;
     child_h3_io = io;
+    /* The event thread allocates from pchild too, so allocate everything
+     * from it here before the thread starts. */
     apr_threadattr_t* attr = NULL;
+    apr_pool_pre_cleanup_register(pchild, io, listen_stop_cleanup);
     if (apr_threadattr_create(&attr, pchild) != APR_SUCCESS || apr_thread_create(&io->event_thread, attr, h3_event_thread, io, pchild) != APR_SUCCESS)
     {
         io->thread_running = 0;
         teardown(io);
         child_h3_io = NULL;
         return APR_EGENERAL;
     }
-    apr_pool_pre_cleanup_register(pchild, io, listen_stop_cleanup);
     ap_log_error(APLOG_MARK, APLOG_INFO, 0, s, "mod_http3 loaded with version: %d (%s) on pid=%d port=%d; built against httpd %d.%d.%d MMN %d:%d (%s)", MOD_HTTP3_VERSION, MOD_HTTP3_VERSION_STRING, h3_getpid(), (int)conf->h3_port, AP_SERVER_MAJORVERSION_NUMBER, AP_SERVER_MINORVERSION_NUMBER,
                  AP_SERVER_PATCHLEVEL_NUMBER, MODULE_MAGIC_NUMBER_MAJOR, MODULE_MAGIC_NUMBER_MINOR, H3_DEVEL ? "devel" : "stable");
     return APR_SUCCESS;
@@ -422,13 +431,19 @@ int service_session_pass(h3_io_t* io, h3_session* session)
 
     if (io->draining || !io->thread_running)
     {
+        int drained;
         apr_thread_mutex_lock(session->lock);
         if (!session->goaway_sent)
         {
             nghttp3_conn_submit_shutdown_notice(session->ngh3);
         }
+        if (!io->thread_running && !session->final_goaway_sent)
+        {
+            nghttp3_conn_shutdown(session->ngh3);
+            session->final_goaway_sent = 1;
+        }
         flush_nghttp3(session);
-        int drained = nghttp3_conn_is_drained2(session->ngh3);
+        drained = H3_NGHTTP3_CONN_IS_DRAINED(session->ngh3);
         apr_thread_mutex_unlock(session->lock);
         if (!session->goaway_sent)
         {
@@ -443,7 +458,11 @@ int service_session_pass(h3_io_t* io, h3_session* session)
         if (drained || (session->goaway_deadline && apr_time_now() >= session->goaway_deadline))
         {
             apr_thread_mutex_lock(session->lock);
-            nghttp3_conn_shutdown(session->ngh3);
+            if (!session->final_goaway_sent)
+            {
+                nghttp3_conn_shutdown(session->ngh3);
+                session->final_goaway_sent = 1;
+            }
             flush_nghttp3(session);
             apr_thread_mutex_unlock(session->lock);
             session->aborted = 1;

modules/http3/h3_request.c

@@ -342,10 +342,15 @@ static void* APR_THREAD_FUNC stream_worker(apr_thread_t* thd, void* data)
 
     ap_add_output_filter_handle(h3_proto_out_filter_handle, h3ctx, r, r->connection);
 
-    ap_update_vhost_from_headers(r);
+    /* Validate the request and select the vhost like HTTP/1.1 and HTTP/2 do
+     * (StrictHostCheck, a missing Host, Expect); on failure r->status holds
+     * the error.
+     */
+    ap_check_request_header(r);
     r->per_dir_config = r->server->lookup_defaults;
 
     ap_log_error(APLOG_MARK, APLOG_INFO, 0, s, "before ap_process_request");
+    int access_status;
     if (r->status != HTTP_OK)
     {
         int err = r->status;
@@ -356,6 +361,14 @@ static void* APR_THREAD_FUNC stream_worker(apr_thread_t* thd, void* data)
     {
         ap_die(HTTP_REQUEST_ENTITY_TOO_LARGE, r);
     }
+    /* Run the post_read_request hooks (mod_setenvif, mod_remoteip,
+     * mod_headers "early", mod_proxy forward proxy detection, ...) and the
+     * core checks on the complete request, as HTTP/1.1 and HTTP/2 do.
+     */
+    else if ((access_status = ap_post_read_request(r)) != OK)
+    {
+        ap_die(access_status, r);
+    }
     else
     {
         ap_process_request(r);

modules/http3/h3_server.c

@@ -30,6 +30,7 @@
 #include "h3_os.h"
 #include "h3_server.h"
 #include "h3_socket.h"
+#include "h3_threads.h"
 #include "mod_http3.h"
 
 static volatile int child_stopping = 0;
@@ -38,20 +39,29 @@ static apr_thread_t* port_acquire_thread = NULL;
 
 struct port_acquire_args
 {
-    apr_pool_t* pchild;
+    apr_pool_t* pool;
     server_rec* vhost;
     h3_server_conf* conf;
 };
 
 static void* APR_THREAD_FUNC port_acquire_thread_fn(apr_thread_t* thread H3_UNUSED, void* data)
 {
     struct port_acquire_args* args = data;
+    /* Each attempt allocates; clear them, or retrying leaks for the life
+     * of the child. A successful attempt's socket stays in ptry. */
+    apr_pool_t* ptry = NULL;
+    if (apr_pool_create(&ptry, args->pool) != APR_SUCCESS)
+    {
+        ap_log_error(APLOG_MARK, APLOG_ERR, 0, args->vhost, "port acquirer: could not create a pool");
+        return NULL;
+    }
     while (!child_stopping)
     {
         int udp_fd = -1;
-        apr_status_t rv = h3_socket_open(args->conf->h3_port, args->conf->h3_socket_buffer_size, args->pchild, &udp_fd);
+        apr_status_t rv = h3_socket_open(args->conf->h3_port, args->conf->h3_socket_buffer_size, ptry, &udp_fd);
         if (rv == APR_EAGAIN)
         {
+            apr_pool_clear(ptry);
             apr_sleep(apr_time_from_msec(H3_PORT_ACQUIRE_RETRY_MS));
             continue;
         }
@@ -65,7 +75,7 @@ static void* APR_THREAD_FUNC port_acquire_thread_fn(apr_thread_t* thread H3_UNUS
             h3_socket_close(udp_fd);
             break;
         }
-        if (h3_io_listen_start(args->pchild, args->vhost, args->conf, udp_fd) != APR_SUCCESS)
+        if (h3_io_listen_start(args->pool, args->vhost, args->conf, udp_fd) != APR_SUCCESS)
         {
             ap_log_error(APLOG_MARK, APLOG_ERR, 0, args->vhost, "port acquirer: h3_io_listen_start failed");
             h3_socket_close(udp_fd);
@@ -96,27 +106,45 @@ static h3_server_conf* find_h3_server(server_rec* s, server_rec** out_server)
     return conf;
 }
 
+/* The threads must be gone before the child pool they run from is
+ * destroyed, and before exit() runs OpenSSL's cleanup under them. Most
+ * MPMs stop them through the child_stopping hook, but prefork skips that
+ * hook when a child exits from a signal handler. */
+static apr_status_t child_pool_cleanup(void* data H3_UNUSED)
+{
+    h3_c1_child_stopping(NULL, 0);
+    return APR_SUCCESS;
+}
+
 void h3_child_init(apr_pool_t* pchild, server_rec* s)
 {
     server_rec* vhost = NULL;
     h3_server_conf* conf = find_h3_server(s, &vhost);
+    apr_pool_t* pool = NULL;
+    int udp_fd = -1;
+    apr_status_t rv;
     if (!conf)
     {
         ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, "h3_child_init: no H3 cert/key configured, skipping");
         return;
     }
+    apr_pool_pre_cleanup_register(pchild, NULL, child_pool_cleanup);
+    if (h3_thread_safe_pool_create(pchild, "h3_child", &pool) != APR_SUCCESS)
+    {
+        ap_log_error(APLOG_MARK, APLOG_ERR, 0, vhost, "h3_child_init: could not create the HTTP/3 pool");
+        return;
+    }
 
-    int udp_fd = -1;
-    apr_status_t rv = h3_socket_open(conf->h3_port, conf->h3_socket_buffer_size, pchild, &udp_fd);
+    rv = h3_socket_open(conf->h3_port, conf->h3_socket_buffer_size, pool, &udp_fd);
     if (rv == APR_EAGAIN)
     {
+        struct port_acquire_args* args = apr_palloc(pool, sizeof(*args));
+        apr_threadattr_t* attr = NULL;
         ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, vhost, "h3_child_init: pid=%d port already owned, will keep retrying in background", h3_getpid());
-        struct port_acquire_args* args = apr_palloc(pchild, sizeof(*args));
-        args->pchild = pchild;
+        args->pool = pool;
         args->vhost = vhost;
         args->conf = conf;
-        apr_threadattr_t* attr = NULL;
-        if (apr_threadattr_create(&attr, pchild) != APR_SUCCESS || apr_thread_create(&port_acquire_thread, attr, port_acquire_thread_fn, args, pchild) != APR_SUCCESS)
+        if (apr_threadattr_create(&attr, pool) != APR_SUCCESS || apr_thread_create(&port_acquire_thread, attr, port_acquire_thread_fn, args, pool) != APR_SUCCESS)
         {
             ap_log_error(APLOG_MARK, APLOG_ERR, 0, vhost, "h3_child_init: failed to start port acquirer thread");
             port_acquire_thread = NULL;
@@ -128,7 +156,7 @@ void h3_child_init(apr_pool_t* pchild, server_rec* s)
         ap_log_error(APLOG_MARK, APLOG_ERR, 0, vhost, "h3_child_init: h3_socket_open failed");
         return;
     }
-    if (h3_io_listen_start(pchild, vhost, conf, udp_fd) != APR_SUCCESS)
+    if (h3_io_listen_start(pool, vhost, conf, udp_fd) != APR_SUCCESS)
     {
         ap_log_error(APLOG_MARK, APLOG_ERR, 0, vhost, "h3_child_init: h3_io_listen_start failed");
         h3_socket_close(udp_fd);

modules/http3/h3_threads.c

@@ -21,6 +21,8 @@
 
 #include <apr_atomic.h>
 #include <apr_hash.h>
+#include <apr_pools.h>
+#include <apr_thread_mutex.h>
 
 #include "h3.h"
 #include "h3_io.h"
@@ -32,6 +34,34 @@
 
 #define H3_PUMP_BUDGET 32
 
+apr_status_t h3_thread_safe_pool_create(apr_pool_t* parent, const char* tag, apr_pool_t** out)
+{
+    apr_allocator_t* allocator = NULL;
+    apr_thread_mutex_t* mutex = NULL;
+    apr_status_t rv = apr_allocator_create(&allocator);
+    if (rv != APR_SUCCESS)
+    {
+        return rv;
+    }
+    rv = apr_pool_create_ex(out, parent, NULL, allocator);
+    if (rv != APR_SUCCESS)
+    {
+        apr_allocator_destroy(allocator);
+        return rv;
+    }
+    apr_allocator_owner_set(allocator, *out);
+    apr_pool_tag(*out, tag);
+    rv = apr_thread_mutex_create(&mutex, APR_THREAD_MUTEX_DEFAULT, *out);
+    if (rv != APR_SUCCESS)
+    {
+        apr_pool_destroy(*out);
+        *out = NULL;
+        return rv;
+    }
+    apr_allocator_mutex_set(allocator, mutex);
+    return APR_SUCCESS;
+}
+
 void* APR_THREAD_FUNC h3_event_thread(apr_thread_t* thread, void* data)
 {
     (void)thread;

modules/http3/include/h3_session.h

@@ -74,6 +74,7 @@ struct h3_session
 
     int control_streams_created;
     int goaway_sent;
+    int final_goaway_sent;
     apr_time_t goaway_deadline;
 
     apr_time_t last_activity;

modules/http3/include/h3_threads.h

@@ -18,6 +18,7 @@
 #define H3_THREADS_H
 
 #include "h3_io.h"
+#include <apr_pools.h>
 #include <apr_thread_proc.h>
 
 /**
@@ -51,4 +52,19 @@ void* APR_THREAD_FUNC worker_thread(apr_thread_t* thread, void* data);
  */
 void* APR_THREAD_FUNC h3_event_thread(apr_thread_t* thread, void* data);
 
+/**
+ * Create a pool with an allocator of its own, which has a mutex, so that
+ * threads can create and destroy subpools of it concurrently. Pools such
+ * as the child pool may have an allocator without one; prefork shares it,
+ * unlocked, with the pool of every request. The mutex does not make
+ * allocating from the pool itself thread-safe: only one thread at a time
+ * may do that.
+ *
+ * @param parent The parent pool.
+ * @param tag    The tag for the new pool.
+ * @param out    Receives the new pool.
+ * @return APR_SUCCESS, or the error from APR.
+ */
+apr_status_t h3_thread_safe_pool_create(apr_pool_t* parent, const char* tag, apr_pool_t** out);
+
 #endif /* H3_THREADS_H */

modules/http3/quic/h3q_conn.c

@@ -106,7 +106,11 @@ int h3q_conn_shutdown(h3q_conn* conn, int is_rapid, uint64_t app_error, const ch
     {
         return 1;
     }
-    uint64_t flags = is_rapid ? (uint64_t)SSL_SHUTDOWN_FLAG_RAPID : 0;
+    /* A rapid shutdown happens when the child stops, after the GOAWAY grace
+     * period: do not wait for unacknowledged stream data either, or a peer
+     * that has gone away keeps the connection, and the child, alive until
+     * the idle timeout. */
+    uint64_t flags = is_rapid ? (uint64_t)(SSL_SHUTDOWN_FLAG_RAPID | SSL_SHUTDOWN_FLAG_NO_STREAM_FLUSH) : 0;
     int ret = 0;
     if (reason)
     {

modules/http3/quic/h3q_stream.c

@@ -23,6 +23,14 @@
 
 #include "quic/h3q_stream.h"
 
+/*
+ * Marks a stream, through its SSL app data, whose sending half was
+ * concluded. OpenSSL before 3.5.7 frees the send buffer once the peer
+ * acknowledges the FIN, and SSL_get_stream_write_state() then dereferences
+ * it, so the write state of such a stream must not be asked for.
+ */
+static char concluded_marker;
+
 h3q_write_result h3q_stream_write(h3q_stream* st, const h3q_vec* vec, size_t nvec, int fin)
 {
     SSL* ssl = (SSL*)st;
@@ -57,7 +65,10 @@ h3q_write_result h3q_stream_write(h3q_stream* st, const h3q_vec* vec, size_t nve
     }
     if (fin && !res.blocked && !res.broken && res.accepted == expected)
     {
-        SSL_stream_conclude(ssl, 0);
+        if (SSL_stream_conclude(ssl, 0))
+        {
+            SSL_set_app_data(ssl, &concluded_marker);
+        }
     }
     return res;
 }
@@ -95,7 +106,8 @@ void h3q_stream_is_read_finished(h3q_stream* st, int* read_finished, int* write_
     int rstate = SSL_get_stream_read_state(ssl);
     *read_finished = (rstate == SSL_STREAM_STATE_FINISHED || rstate == SSL_STREAM_STATE_RESET_REMOTE || rstate == SSL_STREAM_STATE_CONN_CLOSED);
     int wstate = SSL_STREAM_STATE_FINISHED;
-    if (rstate != SSL_STREAM_STATE_CONN_CLOSED && rstate != SSL_STREAM_STATE_RESET_REMOTE)
+    if (rstate != SSL_STREAM_STATE_CONN_CLOSED && rstate != SSL_STREAM_STATE_RESET_REMOTE &&
+        SSL_get_app_data(ssl) != &concluded_marker)
     {
         wstate = SSL_get_stream_write_state(ssl);
     }

test/README.ci

@@ -80,6 +80,11 @@ The CI scripts use the following environment variables:
   HTTP/2 workflow to test against nghttp2 versions other than the one
   packaged for the runner image.
 
+* TEST_CURL_H3 - if set to a version string (e.g. "8.18.0"), that
+  release of curl is built with HTTP/3 (OpenSSL QUIC and libnghttp3)
+  and used by the pytest suites through $CURL.  Used by the mod_http3
+  job, since the runner's curl has no HTTP/3.
+
 * TEST_MD - set together with TEST_PYTEST; installs pebble first so
   that modules/md runs instead of skipping.  See the
   "Python pytest test suites" job in .github/workflows/linux.yml.

test/gha-resolve-deps.sh

@@ -65,6 +65,10 @@ if test -n "${TEST_NGHTTP2-}"; then
     root="${root} nghttp2-${TEST_NGHTTP2}"
 fi
 
+if test -n "${TEST_CURL_H3-}"; then
+    root="${root} curl-h3-${TEST_CURL_H3}"
+fi
+
 if test -n "$root"; then
     # The compiler and every flag which reaches the build are baked into
     # the libraries, NOTEST_* included: APR adds those to the build too,