From eee5f940e38f65fcdacff00c60d9de887cef2e8b Mon Sep 17 00:00:00 2001 From: "Gu://em_" Date: Fri, 28 Nov 2025 15:44:33 +0100 Subject: [PATCH] fix: Goodbye memory leaks, welcome to awesome error codes that work just well --- httpd/src/daemon/daemon.c | 2 +- httpd/src/http/headers.c | 3 +++ httpd/src/http/http.c | 44 ++++++++++++++++++++------------- httpd/src/http/http.h | 2 +- httpd/src/logger/errors.h | 4 +-- httpd/src/logger/logs.c | 12 ++++++--- httpd/src/logger/logs.h | 10 +++++--- httpd/src/main.c | 3 ++- httpd/src/server/server.c | 20 ++++++++++----- httpd/src/server/server.h | 4 +-- httpd/src/utils/files/files.c | 19 +++++++++++--- httpd/src/utils/files/files.h | 12 ++++++++- httpd/src/utils/string/string.c | 1 + 13 files changed, 96 insertions(+), 40 deletions(-) diff --git a/httpd/src/daemon/daemon.c b/httpd/src/daemon/daemon.c index 3ceb251..0e1008c 100644 --- a/httpd/src/daemon/daemon.c +++ b/httpd/src/daemon/daemon.c @@ -48,7 +48,7 @@ int start_daemon(void) pid_t pid = fork(); if (!pid) // Daemon { - start_server(config->servers->ip, config->servers->port); + start_server(config); } else // Parent { diff --git a/httpd/src/http/headers.c b/httpd/src/http/headers.c index 5463d92..bf6ba65 100644 --- a/httpd/src/http/headers.c +++ b/httpd/src/http/headers.c @@ -5,12 +5,15 @@ #include #include "../utils/parsing/words.h" +#include "../utils/string/string.h" void destroy_headers(struct http_header *headers) { while (headers != NULL) { struct http_header *next = headers->next; + string_destroy(headers->field); + string_destroy(headers->value); free(headers); headers = next; } diff --git a/httpd/src/http/http.c b/httpd/src/http/http.c index 896866b..6df9389 100644 --- a/httpd/src/http/http.c +++ b/httpd/src/http/http.c @@ -91,21 +91,19 @@ static void split_target(struct http_request *req) } } -// Finds a valid path based on the client input -static bool find_target(struct http_request *req) +// Finds a valid path based on the client input and returns the corresponding +// FILES return value (see files.h) +static int find_target(struct http_request *req) { // Check filename if (!check_filename(req->target)) - return false; + return ERR_FILES_FORBIDDEN; char *target = string_to_charptr(req->target); int err = is_directory(target); free(target); - err = 0; // TODO fix that bug - if (err == -1) // Doesn't exists - return false; - else if (err == 1) + if (err == FILES_DIR) // Is a directory { // Append default file if directory if (req->target->data[req->target->size - 1] != '/') @@ -117,11 +115,10 @@ static bool find_target(struct http_request *req) target = string_to_charptr(req->target); err = is_directory(target); free(target); - return err == 0; + return err; } - // Exists - return true; + return err; } // WARNING allocates result on the heap @@ -218,7 +215,7 @@ void handle_request(int client_fd, char *client_ip) return; char *method = get_http_method(req->method); char *target = string_to_charptr(req->target); - log_request(method, target, client_ip); + print_log_request(method, target, client_ip); free(method); free(target); @@ -253,12 +250,15 @@ void handle_request(int client_fd, char *client_ip) int fd = open(target, O_RDONLY); if (fd > 0) sendfile(client_fd, fd, 0, atoi(cl_str)); + + free(target); + free(cl_str); } // Log response method = get_http_method(req->method); target = string_to_charptr(req->target); - log_response(resp->status_code, method, target, client_ip); + print_log_response(resp->status_code, method, target, client_ip); free(method); free(target); @@ -314,12 +314,23 @@ struct http_response *generate_response(struct http_request *req) // Status code if (req->status_code == 0) { - if (!find_target(req)) - res->status_code = 404; - else + switch (find_target(req)) + { + case FILES_REG: res->status_code = 200; + break; + case ERR_FILES_NOT_FOUND: + res->status_code = 404; + break; + default: + res->status_code = 403; + break; + } } + else + res->status_code = req->status_code; + // Check protocol and method check_req(req, res); // Headers @@ -339,8 +350,7 @@ struct http_response *generate_response(struct http_request *req) } else { - res->status_code = - 404; // TODO replace by 403 once find_target is fixed + res->status_code = 403; } } append_header(&res->headers, create_header("Connection", "close")); diff --git a/httpd/src/http/http.h b/httpd/src/http/http.h index c3df90c..8c3565c 100644 --- a/httpd/src/http/http.h +++ b/httpd/src/http/http.h @@ -73,7 +73,7 @@ void http_init(struct config *cfg); * * @param client_fd */ -void handle_request(int client_fd, char* client_ip); +void handle_request(int client_fd, char *client_ip); /* @brief Parses the HTTP request and splits it into a request structure * diff --git a/httpd/src/logger/errors.h b/httpd/src/logger/errors.h index a6f9be7..931a883 100644 --- a/httpd/src/logger/errors.h +++ b/httpd/src/logger/errors.h @@ -16,11 +16,11 @@ void errlog_init(bool enabled, int logfile_fd, struct server_config *serv_cfg); */ void print_err(); -/* @brief Prints error logs, just like print_log(), and also to stderr +/* @brief Prints error logs, just like print_log() but for errors */ void print_log_err(char *format, ...); -/* @brief Prints error logs, just like print_log() +/* @brief Returns the string corresponding to the last error that happened */ char *get_err(); diff --git a/httpd/src/logger/logs.c b/httpd/src/logger/logs.c index 118dd9b..f82f706 100644 --- a/httpd/src/logger/logs.c +++ b/httpd/src/logger/logs.c @@ -60,14 +60,20 @@ void print_log(char *format, ...) dprintf(config.logfile_fd, "\n"); } -void log_request(char *request_type, char *target, char *client_ip) +void print_log_request(char *request_type, char *target, char *client_ip) { print_log("received %s, on '%s' from %s", request_type, target, client_ip); } -void log_response(int status_code, char *request_type, char *target, - char *client_ip) +void print_log_response(int status_code, char *request_type, char *target, + char *client_ip) { print_log("responding with %d to %s for %s on '%s'", status_code, client_ip, request_type, target); } + +void log_terminate(void) +{ + if (config.logfile_fd != 0 && config.logfile_fd != STDOUT_FILENO) + close(config.logfile_fd); +} diff --git a/httpd/src/logger/logs.h b/httpd/src/logger/logs.h index 1e81ea3..f18a6d4 100644 --- a/httpd/src/logger/logs.h +++ b/httpd/src/logger/logs.h @@ -35,7 +35,7 @@ void print_log(char *format, ...); * @param target * @param client_ip */ -void log_request(char *request_type, char *target, char *client_ip); +void print_log_request(char *request_type, char *target, char *client_ip); /* @brief Prints response logs with the adequate format in the logfile * @@ -44,7 +44,11 @@ void log_request(char *request_type, char *target, char *client_ip); * @param target * @param client_ip */ -void log_response(int status_code, char *request_type, char *target, - char *client_ip); +void print_log_response(int status_code, char *request_type, char *target, + char *client_ip); + +/* @brief Gracefully exits the logs module + */ +void log_terminate(void); #endif // ! LOGS_H diff --git a/httpd/src/main.c b/httpd/src/main.c index f03aaeb..6f2c5b2 100644 --- a/httpd/src/main.c +++ b/httpd/src/main.c @@ -24,7 +24,7 @@ int main(int argc, char **argv) switch (config->daemon) { case NO_OPTION: - start_server(config->servers->ip, config->servers->port); + start_server(config); break; case START: @@ -43,5 +43,6 @@ int main(int argc, char **argv) return 2; } + config_destroy(config); return 0; } diff --git a/httpd/src/server/server.c b/httpd/src/server/server.c index b9a4c24..56c0c53 100644 --- a/httpd/src/server/server.c +++ b/httpd/src/server/server.c @@ -23,7 +23,8 @@ // === Static variables -int server_socket = 0; +static int server_socket = 0; +static struct config *config; // === Static functions @@ -111,12 +112,13 @@ static void get_ip(struct sockaddr *client_addr, char *res) static void signal_handler(int signal) { - print_log("signal received: %d", signal); + print_log("EVENT Signal received: %d", signal); switch (signal) { case SIGINT: { - print_log("Stopping server..."); + print_log("STOPPING Stopping server..."); stop_server(); + config_destroy(config); exit(0); } default: @@ -126,8 +128,12 @@ static void signal_handler(int signal) // === Functions -void start_server(const char *host, const char *port) +void start_server(struct config *cfg) { + config = cfg; + const char *host = config->servers->ip; + const char *port = config->servers->port; + server_socket = get_socket(host, port); if (server_socket == -1) // TODO log that @@ -145,14 +151,16 @@ void start_server(const char *host, const char *port) if (sigemptyset(&siga.sa_mask) < 0) // TODO log that return; - if (sigaction(SIGINT, &siga, NULL) == -1) + if (sigaction(SIGINT, &siga, NULL) == -1 + || sigaction(SIGPIPE, &siga, NULL) == -1) return; // Main loop while (1) { struct sockaddr client_addr; - int client_fd = accept(server_socket, &client_addr, NULL); + socklen_t client_addr_len = sizeof(struct sockaddr); + int client_fd = accept(server_socket, &client_addr, &client_addr_len); if (client_fd == -1) continue; diff --git a/httpd/src/server/server.h b/httpd/src/server/server.h index 10f6423..0c441ac 100644 --- a/httpd/src/server/server.h +++ b/httpd/src/server/server.h @@ -1,7 +1,7 @@ #ifndef SERVER_H #define SERVER_H -// #define _POSIX_C_SOURCE 200112L +#include "../config/config.h" /* @brief Starts the HTTP server * @@ -10,7 +10,7 @@ * @param hostname * @param port */ -void start_server(const char *host, const char *port); +void start_server(struct config *cfg); /* @brief Stops the currently running HTTP server */ diff --git a/httpd/src/utils/files/files.c b/httpd/src/utils/files/files.c index 0daea41..0f08c25 100644 --- a/httpd/src/utils/files/files.c +++ b/httpd/src/utils/files/files.c @@ -2,6 +2,7 @@ #include "files.h" +#include #include #include @@ -13,10 +14,22 @@ int is_directory(const char *path) { struct stat path_stat; - if (lstat(path, &path_stat) != 0) - return S_ISDIR(path_stat.st_mode); + if (lstat(path, &path_stat) == 0) + { + if (S_ISDIR(path_stat.st_mode)) // Directory + return FILES_DIR; + else if (S_ISREG(path_stat.st_mode)) // Regular file + return FILES_REG; + else + return FILES_OTHER; + } else - return -1; + { + if (errno == ENOENT) // File not found + return ERR_FILES_NOT_FOUND; + else // Other errors + return ERR_FILES_FORBIDDEN; + } } // TODO handle logging diff --git a/httpd/src/utils/files/files.h b/httpd/src/utils/files/files.h index ed0f133..0c2cdcc 100644 --- a/httpd/src/utils/files/files.h +++ b/httpd/src/utils/files/files.h @@ -5,6 +5,15 @@ #define BUFFER_SIZE 1024 +// Return codes +#define FILES_REG 0 +#define FILES_DIR 1 +#define FILES_OTHER 8 + +// Errors +#define ERR_FILES_NOT_FOUND -1 +#define ERR_FILES_FORBIDDEN -2 + // === Includes #include @@ -29,7 +38,8 @@ * * @param path * - * @return 1 if path is a directory, 0 if it is not and -1 if path is not valid + * @return Returns the corresponding return value / error code (see header + * definitions) */ int is_directory(const char *path); diff --git a/httpd/src/utils/string/string.c b/httpd/src/utils/string/string.c index d02d9de..62300b0 100644 --- a/httpd/src/utils/string/string.c +++ b/httpd/src/utils/string/string.c @@ -114,6 +114,7 @@ void str_concat_string(const char *str, size_t size, struct string *to_concat) memcpy(to_concat->data, str, size); memcpy(to_concat->data + size, tmp, tmp_size); + free(tmp); } }