From ce67805e1bc8cb2f9bd6f4829ea347c74398781d Mon Sep 17 00:00:00 2001
From: Quinten <quinten@bumbal.eu>
Date: Wed, 22 Oct 2025 16:51:58 +0200
Subject: [PATCH] feat: auto close fds on exec

---
 webserv/handler/CgiProcess.cpp  | 21 ++++++++++------
 webserv/server/Server.cpp       |  5 ++--
 webserv/socket/ASocket.cpp      | 44 ++++++++++++++++++++++++++++++++-
 webserv/socket/ServerSocket.cpp |  2 +-
 4 files changed, 60 insertions(+), 12 deletions(-)

diff --git a/webserv/handler/CgiProcess.cpp b/webserv/handler/CgiProcess.cpp
index 5b024e2..d115848 100644
--- a/webserv/handler/CgiProcess.cpp
+++ b/webserv/handler/CgiProcess.cpp
@@ -2,6 +2,7 @@
 
 #include "webserv/handler/CgiEnvironment.hpp"
 #include "webserv/http/HttpRequest.hpp"
+#include "webserv/log/Log.hpp"
 #include "webserv/socket/CgiSocket.hpp"
 
 #include <webserv/handler/URI.hpp>
@@ -12,6 +13,7 @@
 #include <memory>
 #include <string>
 
+#include <fcntl.h>
 #include <sys/socket.h>
 #include <sys/wait.h>
 #include <unistd.h>
@@ -34,14 +36,16 @@ void CgiProcess::spawn()
     // pipes
     // TODO pipe can handle flags O_CLOEXEC | O_NONBLOCK as open we should use this everywhere, then we dont need to set
     // non blocking and the fd will be closed when exec is run
+    // NOLINTBEGIN
     int pipeStdin[2];
     int pipeStdout[2];
     int pipeStderr[2];
 
-    if (pipe(pipeStdin) == -1 || pipe(pipeStdout) == -1 || pipe(pipeStderr) == -1)
+    if (pipe2(pipeStdin, O_CLOEXEC | O_NONBLOCK) == -1 || pipe2(pipeStdout, O_CLOEXEC | O_NONBLOCK) == -1 || pipe2(pipeStderr, O_CLOEXEC | O_NONBLOCK) == -1)
     {
         throw std::runtime_error("Failed to create pipes");
     }
+    // NOLINTEND
     CgiEnvironment cgiEnv(uri, request_);
     _pid = fork();
     if (_pid < 0)
@@ -60,15 +64,16 @@ void CgiProcess::spawn()
         dup2(pipeStdout[1], STDOUT_FILENO);
         dup2(pipeStderr[1], STDERR_FILENO);
 
-        close(pipeStdin[0]);
-        close(pipeStdin[1]);
-        close(pipeStdout[0]);
-        close(pipeStdout[1]);
-        close(pipeStderr[0]);
-        close(pipeStderr[1]);
+        // close(pipeStdin[0]);
+        // close(pipeStdin[1]);
+        // close(pipeStdout[0]);
+        // close(pipeStdout[1]);
+        // close(pipeStderr[0]);
+        // close(pipeStderr[1]);
 
         // Log::debug("Executing CGI: " + cgiPath);
-        std::cerr << "Executing CGI: " << cgiPath << std::endl;
+        // std::cerr << "Executing CGI: " << cgiPath << std::endl;
+        Log::clearChannels();
 
         // Prepare arguments
         std::string fullPath = uri.getFullPath();
diff --git a/webserv/server/Server.cpp b/webserv/server/Server.cpp
index 34de21f..05ca6a7 100644
--- a/webserv/server/Server.cpp
+++ b/webserv/server/Server.cpp
@@ -20,6 +20,7 @@
 #include <utility>       // for move, pair
 #include <vector>        // for vector
 
+#include <fcntl.h>
 #include <sys/epoll.h> // for epoll_event, epoll_ctl, EPOLLIN, EPOLLOUT, epoll_create1, epoll_wait, EPOLLERR, EPOLLHUP, EPOLL_CTL_ADD, EPOLL_CTL_DEL, EPOLL_CTL_MOD
 #include <sys/socket.h> // for send, SOMAXCONN
 #include <sys/types.h>  // for ssize_t
@@ -27,7 +28,7 @@
 
 class Router;
 
-Server::Server(const ConfigManager &configManager) : epoll_fd_(epoll_create1(0)), configManager_(configManager)
+Server::Server(const ConfigManager &configManager) : epoll_fd_(epoll_create1(O_CLOEXEC)), configManager_(configManager)
 {
     Log::trace(LOCATION);
     const auto &serverConfigs = configManager.getServerConfigs();
@@ -307,6 +308,6 @@ void Server::run()
     {
         pollSockets();
         pollClients();
-        handleEpoll(events, MAX_EVENTS);
+        handleEpoll(events, MAX_EVENTS); // NOLINT (cppcoreguidelines-pro-bounds-pointer-arithmetic)
     }
 }
\ No newline at end of file
diff --git a/webserv/socket/ASocket.cpp b/webserv/socket/ASocket.cpp
index 0ffb27b..7bb4469 100644
--- a/webserv/socket/ASocket.cpp
+++ b/webserv/socket/ASocket.cpp
@@ -54,10 +54,52 @@ ssize_t ASocket::write(const void *buf, size_t len) const
 
 void ASocket::setNonBlocking() const
 {
-    if (fcntl(fd_, F_SETFL, O_NONBLOCK) == -1)
+    // Set file status flags (O_NONBLOCK)
+    int flags = fcntl(fd_, F_GETFL, 0);
+    if (flags == -1)
+    {
+        throw std::system_error(errno, std::generic_category(), "ASocket: Failed to get FD flags");
+    }
+    if (fcntl(fd_, F_SETFL, flags | O_NONBLOCK) == -1)
     {
         throw std::system_error(errno, std::generic_category(), "ASocket: Failed to set FD non-blocking");
     }
+
+    // Set file descriptor flags (O_CLOEXEC)
+    int fdFlags = fcntl(fd_, F_GETFD, 0);
+    if (fdFlags == -1)
+    {
+        throw std::system_error(errno, std::generic_category(), "ASocket: Failed to get FD descriptor flags");
+    }
+    if (fcntl(fd_, F_SETFD, fdFlags | FD_CLOEXEC) == -1)
+    {
+        throw std::system_error(errno, std::generic_category(), "ASocket: Failed to set FD close-on-exec");
+    }
+
+    // Debug output
+    flags = fcntl(fd_, F_GETFL, 0);
+    fdFlags = fcntl(fd_, F_GETFD, 0);
+
+    std::string flagStr = "0x" + std::to_string(flags) + " (";
+    int mode = flags & 3;
+    switch (mode)
+    {
+    case 0: flagStr += "O_RDONLY "; break;
+    case 1: flagStr += "O_WRONLY "; break;
+    case 2: flagStr += "O_RDWR "; break;
+    default: flagStr += "UNKNOWN_MODE "; break;
+    }
+    if ((flags & O_NONBLOCK) != 0)
+    {
+        flagStr += "O_NONBLOCK ";
+    }
+    if ((fdFlags & FD_CLOEXEC) != 0)
+    {
+        flagStr += "FD_CLOEXEC ";
+    }
+    flagStr += ")";
+
+    Log::debug("ASocket: FD " + std::to_string(fd_) + " configured. Flags: " + flagStr);
 }
 
 int ASocket::getFd() const noexcept
diff --git a/webserv/socket/ServerSocket.cpp b/webserv/socket/ServerSocket.cpp
index 77a54e9..c63bbc0 100644
--- a/webserv/socket/ServerSocket.cpp
+++ b/webserv/socket/ServerSocket.cpp
@@ -27,7 +27,7 @@ ServerSocket::ServerSocket() : ASocket(socket(AF_INET, SOCK_STREAM, 0), ASocket:
         Log::error("setsockopt failed");
         throw std::runtime_error("setsockopt failed");
     }
-    setNonBlocking();
+    // setNonBlocking();
 }
 
 ServerSocket::ServerSocket(int fd) : ASocket(fd)