From f6d81c1a87d79e9698ba39d90a604cdbed6c18a9 Mon Sep 17 00:00:00 2001 From: Francesco Cozzuto Date: Tue, 2 Dec 2025 12:02:20 +0100 Subject: [PATCH] Add -Wall -Wextra flags to the build script and fix the resulting compiler warnings --- Makefile | 2 +- chttp.c | 58 ++++++++++++++++++++++++++------------------ chttp.h | 12 ++++----- src/basic.h | 2 +- src/byte_queue.c | 6 ++--- src/byte_queue.h | 8 +++--- src/secure_context.c | 2 ++ src/server.c | 30 +++++++++++++---------- src/socket.c | 20 +++++++++------ src/socket.h | 2 +- 10 files changed, 83 insertions(+), 59 deletions(-) diff --git a/Makefile b/Makefile index 700a63c..73a8c28 100644 --- a/Makefile +++ b/Makefile @@ -1,5 +1,5 @@ -CFLAGS = -ggdb +CFLAGS = -ggdb -Wall -Wextra LFLAGS = ifeq ($(shell uname -s),Linux) diff --git a/chttp.c b/chttp.c index 6b62d3b..9d76058 100644 --- a/chttp.c +++ b/chttp.c @@ -1765,6 +1765,8 @@ void client_secure_context_free(ClientSecureContext *ctx) { #ifdef HTTPS_ENABLED SSL_CTX_free(ctx->p); +#else + (void) ctx; #endif } @@ -2227,6 +2229,7 @@ static bool is_secure(Socket *s) return s->server_secure_context != NULL || s->client_secure_context != NULL; #else + (void) s; return false; #endif } @@ -2620,7 +2623,6 @@ int socket_manager_wakeup(SocketManager *sm) // NOTE: It's assumed send/write operate atomically // on The descriptor. char byte = 1; - int ret = 0; #ifdef _WIN32 if (send(sm->signal_sock, &byte, 1, 0) < 0) return HTTP_ERROR_UNSPECIFIED; @@ -2733,10 +2735,6 @@ int socket_manager_translate_events( if (sm->num_used == sm->max_used) continue; - // Determine whether the event came from - // the encrypted listener or not. - bool secure = (reg.polled[i].fd == sm->secure_sock); - Socket *s = sm->sockets; while (s->state != SOCKET_STATE_FREE) { s++; @@ -2757,6 +2755,10 @@ int socket_manager_translate_events( s->events = 0; s->user = NULL; #ifdef HTTPS_ENABLED + // Determine whether the event came from + // the encrypted listener or not. + bool secure = (reg.polled[i].fd == sm->secure_sock); + s->ssl = NULL; s->server_secure_context = NULL; s->client_secure_context = NULL; @@ -2996,6 +2998,8 @@ int socket_connect(SocketManager *sm, int num_targets, s->client_secure_context = &sm->client_secure_context; s->dont_verify_cert = dont_verify_cert; } +#else + (void) dont_verify_cert; #endif sm->num_used++; @@ -3069,6 +3073,9 @@ int socket_recv(SocketManager *sm, SocketHandle handle, ret = 0; } return ret; +#else + // Unreachable + return 0; #endif } } @@ -3117,13 +3124,15 @@ int socket_send(SocketManager *sm, SocketHandle handle, ret = 0; } return ret; +#else + // Unreachable + return 0; #endif } } void socket_close(SocketManager *sm, SocketHandle handle) { - int ret; Socket *s = handle_to_socket(sm, handle); if (s == NULL) return; @@ -3154,7 +3163,6 @@ void socket_set_user(SocketManager *sm, SocketHandle handle, void *user) bool socket_ready(SocketManager *sm, SocketHandle handle) { - bool ready = false; Socket *s = handle_to_socket(sm, handle); if (s == NULL) return false; @@ -3349,7 +3357,7 @@ int byte_queue_write_setmincap(ByteQueue *queue, uint32_t mincap) if (size > queue->limit) size = queue->limit; - uint8_t *data = malloc(size); + char *data = malloc(size); if (!data) { queue->flags |= BYTE_QUEUE_ERROR; return 0; @@ -3406,7 +3414,7 @@ void byte_queue_write_fmt2(ByteQueue *queue, return; } - if (len > dst.len) { + if ((size_t) len > dst.len) { byte_queue_write_ack(queue, 0); byte_queue_write_setmincap(queue, len+1); dst = byte_queue_write_buf(queue); @@ -3447,7 +3455,7 @@ void byte_queue_patch(ByteQueue *queue, ByteQueueOffset off, assert(len <= queue->used - (off - queue->curs)); // Perform the patch - uint8_t *dst = queue->data + queue->head + (off - queue->curs); + char *dst = queue->data + queue->head + (off - queue->curs); memcpy(dst, src, len); } @@ -4855,6 +4863,20 @@ void http_response_builder_status(HTTP_ResponseBuilder builder, int status) conn->state = HTTP_SERVER_CONN_WAIT_HEADER; } +static bool is_header_valid(HTTP_String str) +{ + bool has_colon = false; + for (int i = 0; i < str.len; i++) { + char c = str.ptr[i]; + if (c == ':') + has_colon = true; + // Reject control characters (especially \r and \n) + if (c < 0x20 && c != '\t') + return false; + } + return has_colon; +} + void http_response_builder_header(HTTP_ResponseBuilder builder, HTTP_String str) { HTTP_ServerConn *conn = builder_to_conn(builder); @@ -4864,19 +4886,9 @@ void http_response_builder_header(HTTP_ResponseBuilder builder, HTTP_String str) if (conn->state != HTTP_SERVER_CONN_WAIT_HEADER) return; - // Validate header: must contain a colon and no control characters - // (to prevent HTTP response splitting attacks) - bool has_colon = false; - for (int i = 0; i < str.len; i++) { - char c = str.ptr[i]; - if (c == ':') - has_colon = true; - // Reject control characters (especially \r and \n) - if (c < 0x20 && c != '\t') - return; - } - if (!has_colon) - return; + // Header must contain a colon and no control characters + // to prevent HTTP response splitting attacks + if (!is_header_valid(str)) return; // Silently drop it byte_queue_write(&conn->output, str.ptr, str.len); byte_queue_write(&conn->output, "\r\n", 2); diff --git a/chttp.h b/chttp.h index e754153..3c0b911 100644 --- a/chttp.h +++ b/chttp.h @@ -114,7 +114,7 @@ char *http_strerror(int code); #define HTTP_STR(X) ((HTTP_String) {(X), sizeof(X)-1}) // Returns the number of items of a static array. -#define HTTP_COUNT(X) (sizeof(X) / sizeof((X)[0])) +#define HTTP_COUNT(X) (int) (sizeof(X) / sizeof((X)[0])) // Macro to unpack an HTTP_String into its length and pointer components. // Useful for passing HTTP_String to printf-style functions with "%.*s" format. @@ -382,7 +382,7 @@ int server_secure_context_add_certificate(ServerSecureContext *ctx, #ifdef _WIN32 #define NATIVE_SOCKET SOCKET -#define NATIVE_SOCKET_INVALID SOCKET_ERROR +#define NATIVE_SOCKET_INVALID INVALID_SOCKET #define CLOSE_NATIVE_SOCKET closesocket #else #define NATIVE_SOCKET int @@ -688,19 +688,19 @@ enum { }; typedef struct { - uint8_t *ptr; - size_t len; + char *ptr; + size_t len; } ByteView; // Fields are for internal use only typedef struct { uint64_t curs; - uint8_t* data; + char* data; uint32_t head; uint32_t size; uint32_t used; uint32_t limit; - uint8_t* read_target; + char* read_target; uint32_t read_target_size; int flags; } ByteQueue; diff --git a/src/basic.h b/src/basic.h index 6edec5a..066bbda 100644 --- a/src/basic.h +++ b/src/basic.h @@ -71,7 +71,7 @@ char *http_strerror(int code); #define HTTP_STR(X) ((HTTP_String) {(X), sizeof(X)-1}) // Returns the number of items of a static array. -#define HTTP_COUNT(X) (sizeof(X) / sizeof((X)[0])) +#define HTTP_COUNT(X) (int) (sizeof(X) / sizeof((X)[0])) // Macro to unpack an HTTP_String into its length and pointer components. // Useful for passing HTTP_String to printf-style functions with "%.*s" format. diff --git a/src/byte_queue.c b/src/byte_queue.c index 23035a7..78891ee 100644 --- a/src/byte_queue.c +++ b/src/byte_queue.c @@ -179,7 +179,7 @@ int byte_queue_write_setmincap(ByteQueue *queue, uint32_t mincap) if (size > queue->limit) size = queue->limit; - uint8_t *data = malloc(size); + char *data = malloc(size); if (!data) { queue->flags |= BYTE_QUEUE_ERROR; return 0; @@ -236,7 +236,7 @@ void byte_queue_write_fmt2(ByteQueue *queue, return; } - if (len > dst.len) { + if ((size_t) len > dst.len) { byte_queue_write_ack(queue, 0); byte_queue_write_setmincap(queue, len+1); dst = byte_queue_write_buf(queue); @@ -277,7 +277,7 @@ void byte_queue_patch(ByteQueue *queue, ByteQueueOffset off, assert(len <= queue->used - (off - queue->curs)); // Perform the patch - uint8_t *dst = queue->data + queue->head + (off - queue->curs); + char *dst = queue->data + queue->head + (off - queue->curs); memcpy(dst, src, len); } diff --git a/src/byte_queue.h b/src/byte_queue.h index 2df19cd..7b66f20 100644 --- a/src/byte_queue.h +++ b/src/byte_queue.h @@ -15,19 +15,19 @@ enum { }; typedef struct { - uint8_t *ptr; - size_t len; + char *ptr; + size_t len; } ByteView; // Fields are for internal use only typedef struct { uint64_t curs; - uint8_t* data; + char* data; uint32_t head; uint32_t size; uint32_t used; uint32_t limit; - uint8_t* read_target; + char* read_target; uint32_t read_target_size; int flags; } ByteQueue; diff --git a/src/secure_context.c b/src/secure_context.c index b1b1dd9..2780142 100644 --- a/src/secure_context.c +++ b/src/secure_context.c @@ -45,6 +45,8 @@ void client_secure_context_free(ClientSecureContext *ctx) { #ifdef HTTPS_ENABLED SSL_CTX_free(ctx->p); +#else + (void) ctx; #endif } diff --git a/src/server.c b/src/server.c index 56707c9..10be1d0 100644 --- a/src/server.c +++ b/src/server.c @@ -415,6 +415,20 @@ void http_response_builder_status(HTTP_ResponseBuilder builder, int status) conn->state = HTTP_SERVER_CONN_WAIT_HEADER; } +static bool is_header_valid(HTTP_String str) +{ + bool has_colon = false; + for (int i = 0; i < str.len; i++) { + char c = str.ptr[i]; + if (c == ':') + has_colon = true; + // Reject control characters (especially \r and \n) + if (c < 0x20 && c != '\t') + return false; + } + return has_colon; +} + void http_response_builder_header(HTTP_ResponseBuilder builder, HTTP_String str) { HTTP_ServerConn *conn = builder_to_conn(builder); @@ -424,19 +438,9 @@ void http_response_builder_header(HTTP_ResponseBuilder builder, HTTP_String str) if (conn->state != HTTP_SERVER_CONN_WAIT_HEADER) return; - // Validate header: must contain a colon and no control characters - // (to prevent HTTP response splitting attacks) - bool has_colon = false; - for (int i = 0; i < str.len; i++) { - char c = str.ptr[i]; - if (c == ':') - has_colon = true; - // Reject control characters (especially \r and \n) - if (c < 0x20 && c != '\t') - return; - } - if (!has_colon) - return; + // Header must contain a colon and no control characters + // to prevent HTTP response splitting attacks + if (!is_header_valid(str)) return; // Silently drop it byte_queue_write(&conn->output, str.ptr, str.len); byte_queue_write(&conn->output, "\r\n", 2); diff --git a/src/socket.c b/src/socket.c index ed196cb..319c078 100644 --- a/src/socket.c +++ b/src/socket.c @@ -297,6 +297,7 @@ static bool is_secure(Socket *s) return s->server_secure_context != NULL || s->client_secure_context != NULL; #else + (void) s; return false; #endif } @@ -690,7 +691,6 @@ int socket_manager_wakeup(SocketManager *sm) // NOTE: It's assumed send/write operate atomically // on The descriptor. char byte = 1; - int ret = 0; #ifdef _WIN32 if (send(sm->signal_sock, &byte, 1, 0) < 0) return HTTP_ERROR_UNSPECIFIED; @@ -803,10 +803,6 @@ int socket_manager_translate_events( if (sm->num_used == sm->max_used) continue; - // Determine whether the event came from - // the encrypted listener or not. - bool secure = (reg.polled[i].fd == sm->secure_sock); - Socket *s = sm->sockets; while (s->state != SOCKET_STATE_FREE) { s++; @@ -827,6 +823,10 @@ int socket_manager_translate_events( s->events = 0; s->user = NULL; #ifdef HTTPS_ENABLED + // Determine whether the event came from + // the encrypted listener or not. + bool secure = (reg.polled[i].fd == sm->secure_sock); + s->ssl = NULL; s->server_secure_context = NULL; s->client_secure_context = NULL; @@ -1066,6 +1066,8 @@ int socket_connect(SocketManager *sm, int num_targets, s->client_secure_context = &sm->client_secure_context; s->dont_verify_cert = dont_verify_cert; } +#else + (void) dont_verify_cert; #endif sm->num_used++; @@ -1139,6 +1141,9 @@ int socket_recv(SocketManager *sm, SocketHandle handle, ret = 0; } return ret; +#else + // Unreachable + return 0; #endif } } @@ -1187,13 +1192,15 @@ int socket_send(SocketManager *sm, SocketHandle handle, ret = 0; } return ret; +#else + // Unreachable + return 0; #endif } } void socket_close(SocketManager *sm, SocketHandle handle) { - int ret; Socket *s = handle_to_socket(sm, handle); if (s == NULL) return; @@ -1224,7 +1231,6 @@ void socket_set_user(SocketManager *sm, SocketHandle handle, void *user) bool socket_ready(SocketManager *sm, SocketHandle handle) { - bool ready = false; Socket *s = handle_to_socket(sm, handle); if (s == NULL) return false; diff --git a/src/socket.h b/src/socket.h index c14834c..c1b21ac 100644 --- a/src/socket.h +++ b/src/socket.h @@ -50,7 +50,7 @@ #ifdef _WIN32 #define NATIVE_SOCKET SOCKET -#define NATIVE_SOCKET_INVALID SOCKET_ERROR +#define NATIVE_SOCKET_INVALID INVALID_SOCKET #define CLOSE_NATIVE_SOCKET closesocket #else #define NATIVE_SOCKET int