Skip to content

Commit 2e803ed

Browse files
committed
ci: fix CodeQL fd_set forward-decl and Windows socklen_t static_assert
Two regressions from the TASK-020 hygiene sweep broke the CI matrix on PR #374: 1. CodeQL (Linux/glibc): `struct fd_set;` is a typedef-name-after-struct error on glibc, where fd_set is a typedef of an unnamed struct (no struct tag). The forward declaration only happened to compile on platforms whose `<stdlib.h>` chain did not transitively pull in `<sys/select.h>`. Replace with a platform-conditional include of `<sys/select.h>` (POSIX/Cygwin) or `<winsock2.h>` (Windows), and drop the `struct` keyword from the `fd_set*` parameter types in webserver::get_fdset's signature and implementation. 2. Windows MSYS2: `static_assert(std::is_unsigned<socklen_t>::value)` fires because winsock2 typedefs socklen_t as signed `int`. Drop the assertion; the size assertion remains, and the unsigned->socklen_t conversion in add_connection is value-preserving for any realistic sockaddr length on every supported platform. Update the doc on webserver::add_connection to reflect the actual portable contract.
1 parent 2ecb0f7 commit 2e803ed

2 files changed

Lines changed: 43 additions & 29 deletions

File tree

src/detail/webserver_setup.cpp

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -79,17 +79,14 @@
7979

8080
// TASK-020-review: pin add_connection's unsigned-int/socklen_t ABI contract
8181
// at file scope so the assertion is evaluated once at TU start rather than
82-
// being associated with the specific function body.
82+
// being associated with the specific function body. socklen_t is a 32-bit
83+
// integer on every supported platform (unsigned on POSIX, signed `int` on
84+
// Windows winsock2). `unsigned int` is at least as wide as either, so the
85+
// static_cast<socklen_t>(addrlen) in add_connection is value-preserving for
86+
// any realistic sockaddr length (a few hundred bytes at most).
8387
static_assert(sizeof(unsigned int) >= sizeof(socklen_t),
8488
"unsigned int is narrower than socklen_t on this platform; "
8589
"webserver::add_connection's public signature must be widened.");
86-
// Also assert signedness: POSIX defines socklen_t as an unsigned type; the
87-
// static_cast<socklen_t>(addrlen) in add_connection is only well-defined
88-
// when the value fits in the unsigned target (CWE-681).
89-
static_assert(std::is_unsigned<socklen_t>::value,
90-
"socklen_t is signed on this platform; "
91-
"the static_cast<socklen_t>(addrlen) in add_connection may produce "
92-
"sign-extension or truncation for large address lengths.");
9390

9491
using std::string;
9592
using std::pair;
@@ -401,12 +398,12 @@ bool webserver::run_wait(int32_t millisec) {
401398
return MHD_run_wait(impl_->daemon, millisec) == MHD_YES;
402399
}
403400

404-
bool webserver::get_fdset(struct fd_set* read_fd_set, struct fd_set* write_fd_set,
405-
struct fd_set* except_fd_set, int* max_fd) {
406-
// TASK-020-review: signature now uses typed fd_set* (forward-declared in
407-
// the public header) rather than void*, restoring compile-time type safety.
408-
// <sys/select.h> is reachable here transitively via <microhttpd.h> so no
409-
// cast is needed.
401+
bool webserver::get_fdset(fd_set* read_fd_set, fd_set* write_fd_set,
402+
fd_set* except_fd_set, int* max_fd) {
403+
// TASK-020-review: signature uses typed fd_set* rather than void*,
404+
// restoring compile-time type safety. The public header pulls in
405+
// <sys/select.h> / <winsock2.h> directly because fd_set is a typedef
406+
// and cannot be portably forward-declared.
410407
if (impl_->daemon == nullptr) return false;
411408
MHD_socket mhd_max_fd = 0;
412409
if (MHD_get_fdset(impl_->daemon,

src/httpserver/webserver.hpp

Lines changed: 32 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,17 @@
4646
#include "httpserver/http_utils.hpp"
4747
#include "httpserver/create_webserver.hpp"
4848

49-
// TASK-020: socket-layer types are forward-declared here so this public
50-
// header does not transitively drag <sys/select.h> / <sys/socket.h>
51-
// into consumer TUs. See the get_fdset and add_connection Doxygen
52-
// blocks below for the per-method ABI rationale.
53-
struct fd_set;
49+
// TASK-020: socket-layer types kept minimal in this public header.
50+
// `struct sockaddr` is a POSIX-tagged struct and can be forward-declared
51+
// per-use, but `fd_set` is a typedef of an unnamed struct in glibc, so
52+
// it must be a complete type when get_fdset's signature is parsed.
53+
// Pull in only the system header that defines fd_set; the rest of the
54+
// BSD-socket / select API does not leak into consumer TUs.
55+
#if defined(_WIN32) && !defined(__CYGWIN__)
56+
#include <winsock2.h>
57+
#else
58+
#include <sys/select.h>
59+
#endif
5460

5561
// Forward declarations: backend (MHD) types are intentionally NOT pulled in.
5662
// The libmicrohttpd and pthread headers live behind the PIMPL
@@ -293,18 +299,19 @@ class webserver {
293299

294300
/**
295301
* Get the file descriptor sets for select()-based event loop integration.
296-
* `struct fd_set` is forward-declared at file scope so this header does
297-
* not need to include `<sys/select.h>`. The typed parameters restore
298-
* compile-time type safety: the compiler rejects non-fd_set* arguments
299-
* that the previous void* signature silently accepted (CWE-704).
302+
* `fd_set` is a typedef (anonymous-struct in glibc), so the public header
303+
* includes the platform header that defines it rather than relying on a
304+
* forward declaration. The typed parameters restore compile-time type
305+
* safety: the compiler rejects non-fd_set* arguments that the previous
306+
* void* signature silently accepted (CWE-704).
300307
* @param read_fd_set set of FDs to watch for reading
301308
* @param write_fd_set set of FDs to watch for writing
302309
* @param except_fd_set set of FDs to watch for exceptions
303310
* @param max_fd highest FD number set in any of the sets
304311
* @return true on success, false on error
305312
**/
306-
bool get_fdset(struct fd_set* read_fd_set, struct fd_set* write_fd_set,
307-
struct fd_set* except_fd_set, int* max_fd);
313+
bool get_fdset(fd_set* read_fd_set, fd_set* write_fd_set,
314+
fd_set* except_fd_set, int* max_fd);
308315

309316
/**
310317
* Get the timeout until the next MHD action is needed.
@@ -316,10 +323,13 @@ class webserver {
316323
/**
317324
* Add an externally-accepted socket connection.
318325
* `addrlen` is typed as `unsigned int` rather than `socklen_t` so
319-
* this header does not have to include the BSD-socket header. POSIX
320-
* guarantees `socklen_t` is an unsigned integer of at least 32 bits;
321-
* `unsigned int` is wider on every supported platform. The
322-
* implementation passes the value directly to MHD_add_connection.
326+
* this header does not have to include the BSD-socket header.
327+
* `socklen_t` is a 32-bit integer on every supported platform
328+
* (unsigned on POSIX, signed `int` on Windows winsock2); `unsigned
329+
* int` is at least as wide as either, so the conversion in the
330+
* implementation is value-preserving for any realistic `sockaddr`
331+
* length. The implementation passes the value directly to
332+
* MHD_add_connection.
323333
* @param client_socket the accepted client socket
324334
* @param addr the client address (forward-declared `struct sockaddr*`)
325335
* @param addrlen length of the address (in bytes)
@@ -401,6 +411,13 @@ class webserver {
401411
const struct sockaddr* bind_address;
402412
std::shared_ptr<struct sockaddr_storage> bind_address_storage;
403413
const int max_thread_stack_size;
414+
// 0 sentinel -> compile-time defaults
415+
// (arguments_accumulator::DEFAULT_MAX_ARGS_COUNT / _BYTES).
416+
// webserver_impl::connection_notify copies these into the per-
417+
// connection_state at MHD_CONNECTION_NOTIFY_STARTED; populate_args
418+
// reads them from connection_state via the socket_context.
419+
const std::size_t max_args_count;
420+
const std::size_t max_args_bytes;
404421
const bool use_ssl;
405422
const bool use_ipv6;
406423
const bool use_dual_stack;

0 commit comments

Comments
 (0)