From e44b0746530da47fa462bdccfa3a3de3653b09c5 Mon Sep 17 00:00:00 2001 From: ibrohimovmuhammad2020 Date: Thu, 28 May 2026 21:20:44 +0200 Subject: [PATCH 01/13] test: add Phase 1.5 integration test harness --- tests/phase15_tests.sh | 85 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) create mode 100755 tests/phase15_tests.sh diff --git a/tests/phase15_tests.sh b/tests/phase15_tests.sh new file mode 100755 index 0000000..0fe5873 --- /dev/null +++ b/tests/phase15_tests.sh @@ -0,0 +1,85 @@ +#!/usr/bin/env bash +# Phase 1.5 integration tests. +# +# Each test launches ./webserv with a fixture config, captures stdout+stderr, +# and asserts exit code and (optionally) a log substring. We launch with a +# short sleep + kill for the "happy path" cases because ./webserv normally +# runs forever; we only want to confirm it got to "listening" state. +# +# Run from the project root: ./tests/phase15_tests.sh + +set -u +PASS=0 +FAIL=0 +FAILED_TESTS=() + +assert_exits_nonzero_with() { + local name=$1 + local config=$2 + local expect_substr=$3 + + local out + out=$(./webserv "$config" 2>&1) + local rc=$? + if [ $rc -eq 0 ]; then + FAIL=$((FAIL+1)); FAILED_TESTS+=("$name: exit code was 0, expected non-zero") + return + fi + if ! echo "$out" | grep -qF "$expect_substr"; then + FAIL=$((FAIL+1)) + FAILED_TESTS+=("$name: missing substring '$expect_substr' in output: +$out") + return + fi + PASS=$((PASS+1)) +} + +assert_listens_on_ports() { + local name=$1 + local config=$2 + shift 2 + local ports=("$@") + + ./webserv "$config" >/tmp/webserv.out 2>&1 & + local pid=$! + sleep 1 + + if ! kill -0 "$pid" 2>/dev/null; then + FAIL=$((FAIL+1)) + FAILED_TESTS+=("$name: webserv died at startup. Output: +$(cat /tmp/webserv.out)") + return + fi + + local missing=() + for port in "${ports[@]}"; do + if ! lsof -iTCP:"$port" -sTCP:LISTEN -P -n 2>/dev/null | grep -q webserv; then + missing+=("$port") + fi + done + + kill -INT "$pid" 2>/dev/null + wait "$pid" 2>/dev/null + + if [ ${#missing[@]} -gt 0 ]; then + FAIL=$((FAIL+1)) + FAILED_TESTS+=("$name: did not listen on ports: ${missing[*]}") + return + fi + PASS=$((PASS+1)) +} + +# Sanity: binary exists +if [ ! -x ./webserv ]; then + echo "fatal: ./webserv missing — run 'make' first" + exit 2 +fi + +# --- tests appear here, added per task --- + +echo +echo "Phase 1.5: $PASS passed, $FAIL failed" +if [ $FAIL -gt 0 ]; then + printf '%s\n' "${FAILED_TESTS[@]}" + exit 1 +fi From e8d91b21744f98b036c28ba87598f2ac5c1df603 Mon Sep 17 00:00:00 2001 From: ibrohimovmuhammad2020 Date: Thu, 28 May 2026 21:27:28 +0200 Subject: [PATCH 02/13] test: add cleanup trap to harness for SIGINT safety --- tests/phase15_tests.sh | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/phase15_tests.sh b/tests/phase15_tests.sh index 0fe5873..d7286bf 100755 --- a/tests/phase15_tests.sh +++ b/tests/phase15_tests.sh @@ -13,6 +13,13 @@ PASS=0 FAIL=0 FAILED_TESTS=() +# Kill any backgrounded ./webserv on script exit (normal, error, or Ctrl+C) +# so a failed run never leaves stray listeners holding ports. +cleanup() { + pkill -P $$ -f "./webserv" 2>/dev/null || true +} +trap cleanup EXIT INT TERM + assert_exits_nonzero_with() { local name=$1 local config=$2 From c4e934620f42291918ee9c70d48192668d2fcc86 Mon Sep 17 00:00:00 2001 From: ibrohimovmuhammad2020 Date: Thu, 28 May 2026 21:39:37 +0200 Subject: [PATCH 03/13] test: add duplicate-listen fixture (harness wiring deferred to Task 4) --- config/test_duplicate.conf | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 config/test_duplicate.conf diff --git a/config/test_duplicate.conf b/config/test_duplicate.conf new file mode 100644 index 0000000..895fa6e --- /dev/null +++ b/config/test_duplicate.conf @@ -0,0 +1,12 @@ +server { + listen 8080; + server_name a; + root ./www; + location / { allowed_methods GET; } +} +server { + listen 8080; + server_name b; + root ./www; + location / { allowed_methods GET; } +} From 89060db3291be7ba39b3d113f2cfbfd92838e2a5 Mon Sep 17 00:00:00 2001 From: ibrohimovmuhammad2020 Date: Thu, 28 May 2026 21:45:21 +0200 Subject: [PATCH 04/13] feat(socket): add (host, port) ctor with SO_REUSEADDR and cleanup contract --- includes/socket.hpp | 9 +++++ src/socket/socket.cpp | 80 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+) diff --git a/includes/socket.hpp b/includes/socket.hpp index 72adafa..6cf7345 100644 --- a/includes/socket.hpp +++ b/includes/socket.hpp @@ -2,6 +2,7 @@ #include #include +#include #include #include #include @@ -23,6 +24,14 @@ class Socket { public: Socket(int port); + // Phase 1.5: config-driven listener constructor. + // Resolves `host` (a numeric IPv4 string like "0.0.0.0" or "127.0.0.1") + // via getaddrinfo with AI_NUMERICHOST so no DNS happens. Sets + // SO_REUSEADDR before bind so we can restart inside TIME_WAIT without + // EADDRINUSE. Throws SocketException on any setup failure; the body + // guarantees the fd and addrinfo chain are released on every throw + // path (see socket.cpp for the cleanup contract). + Socket(const std::string& host, int port); ~Socket(); void startListening(int backlog = SOMAXCONN); void close(); diff --git a/src/socket/socket.cpp b/src/socket/socket.cpp index bf042a0..5f35fd4 100644 --- a/src/socket/socket.cpp +++ b/src/socket/socket.cpp @@ -1,4 +1,6 @@ #include "../../includes/webserv.hpp" +#include +#include // Constructor with port number and create a socket with AF_INET and SOCK_STREAM @@ -78,4 +80,82 @@ int Socket::acceptClient() { // get the file descriptor of the socket int Socket::getFileDescriptor() const { return fd_socket; +} + +// Phase 1.5: config-driven ctor. Two resources are acquired here and must +// be released on every throw path: +// 1. the addrinfo* chain returned by getaddrinfo -> freeaddrinfo +// 2. the fd returned by socket() -> ::close(fd) +// We nest a try/catch around steps 2-4 so any throw runs freeaddrinfo before +// propagating. The fd is closed inline at each failure point, matching the +// existing single-arg ctor pattern above. +Socket::Socket(const std::string& host, int port) : fd_socket(-1) { + struct addrinfo hints; + struct addrinfo* res = NULL; + + std::memset(&hints, 0, sizeof(hints)); + hints.ai_family = AF_INET; + hints.ai_socktype = SOCK_STREAM; + hints.ai_flags = AI_NUMERICHOST | AI_PASSIVE; + + // service NULL + port written manually into the sockaddr below; we don't + // pass a service string because we already have the integer port. + if (getaddrinfo(host.c_str(), NULL, &hints, &res) != 0 || res == NULL) { + LOG_ERROR(" getaddrinfo failed for " + host); + throw SocketException("getaddrinfo failed for host '" + host + "'"); + } + + try { + fd_socket = socket(res->ai_family, res->ai_socktype, res->ai_protocol); + if (fd_socket == -1) { + LOG_ERROR(" Failed to create socket"); + throw SocketException("Failed to create socket"); + } + + // SO_REUSEADDR: lets us re-bind a port that is still in TIME_WAIT from + // a previous instance. Subject p.6 lists setsockopt in the allowed + // functions table. Must be set BEFORE bind(). + int yes = 1; + if (setsockopt(fd_socket, SOL_SOCKET, SO_REUSEADDR, + &yes, sizeof(yes)) == -1) + { + LOG_ERROR(" setsockopt SO_REUSEADDR failed"); + ::close(fd_socket); + fd_socket = -1; + throw SocketException("setsockopt SO_REUSEADDR failed"); + } + + if (fcntl(fd_socket, F_SETFL, O_NONBLOCK) == -1) { + LOG_ERROR(" Failed to set socket to non-blocking"); + ::close(fd_socket); + fd_socket = -1; + throw SocketException("Failed to set socket to non-blocking"); + } + + // Copy the resolved address into our member (so we don't depend on + // res after freeaddrinfo) and overwrite the port field with the + // caller's integer port (getaddrinfo left it at 0 since service was + // NULL). htons because sin_port is network byte order. + std::memcpy(&_address, res->ai_addr, sizeof(struct sockaddr_in)); + _address.sin_port = htons(static_cast(port)); + + if (bind(fd_socket, (struct sockaddr*)&_address, + sizeof(_address)) == -1) + { + LOG_ERROR(" Failed to bind socket on " + + host + ":" + toString(port)); + ::close(fd_socket); + fd_socket = -1; + throw SocketException("bind failed on " + host + ":" + + toString(port)); + } + + LOG_DEBUG(" Socket bound to " + host + ":" + + toString(port) + " fd=" + toString(fd_socket)); + } catch (...) { + // any throw after getaddrinfo succeeded must free the chain + freeaddrinfo(res); + throw; + } + freeaddrinfo(res); } \ No newline at end of file From ee24c7c36d8ecb645dd2acfb4015228f9927bc19 Mon Sep 17 00:00:00 2001 From: ibrohimovmuhammad2020 Date: Thu, 28 May 2026 21:54:30 +0200 Subject: [PATCH 05/13] fix(socket): guard port range and document getaddrinfo single-result assumption --- src/socket/socket.cpp | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/socket/socket.cpp b/src/socket/socket.cpp index 5f35fd4..15a5071 100644 --- a/src/socket/socket.cpp +++ b/src/socket/socket.cpp @@ -90,6 +90,14 @@ int Socket::getFileDescriptor() const { // propagating. The fd is closed inline at each failure point, matching the // existing single-arg ctor pattern above. Socket::Socket(const std::string& host, int port) : fd_socket(-1) { + // The config validator (Config::validate) does not yet enforce port + // range, so guard here. A silent uint16_t truncation later (htons cast) + // would let `listen 70000;` bind to a different port without warning. + if (port <= 0 || port > 65535) { + throw SocketException("invalid port (must be 1-65535): " + + toString(port)); + } + struct addrinfo hints; struct addrinfo* res = NULL; @@ -105,6 +113,8 @@ Socket::Socket(const std::string& host, int port) : fd_socket(-1) { throw SocketException("getaddrinfo failed for host '" + host + "'"); } + // AI_NUMERICHOST + AF_INET => getaddrinfo returns a chain of length 1, + // so we walk only res (ignore res->ai_next). try { fd_socket = socket(res->ai_family, res->ai_socktype, res->ai_protocol); if (fd_socket == -1) { @@ -158,4 +168,4 @@ Socket::Socket(const std::string& host, int port) : fd_socket(-1) { throw; } freeaddrinfo(res); -} \ No newline at end of file +} From f4bfe819dc69da8828e5c81d96ca0b3cfdc6514e Mon Sep 17 00:00:00 2001 From: ibrohimovmuhammad2020 Date: Thu, 28 May 2026 22:00:01 +0200 Subject: [PATCH 06/13] feat(server): add Config-based ctor with partial-init RAII --- includes/server.hpp | 33 ++++++-- src/server/server.cpp | 191 ++++++++++++++++++++++++++++++------------ 2 files changed, 163 insertions(+), 61 deletions(-) diff --git a/includes/server.hpp b/includes/server.hpp index 24f092d..3b4a75b 100644 --- a/includes/server.hpp +++ b/includes/server.hpp @@ -1,7 +1,9 @@ #pragma once #include "socket.hpp" +#include "config.hpp" #include +#include #include #include #include @@ -16,22 +18,37 @@ extern volatile sig_atomic_t g_shutdown; class Server { private: std::vector poll_fds; - // Reference, not value: Server borrows the listening Socket owned - // by main(). With ownership in main, only main's destructor calls - // ::close() on the listening fd, avoiding a double close. - Socket &socket; - // Server is not copyable: a reference member would alias the same - // Socket anyway, and there is exactly one Server in main(). + // Phase 1.5 members -------------------------------------------------- + // `config_` is a borrow: it lives in main()'s stack frame, longer + // than this Server, so the pointer is safe for our lifetime. + // Phase 2.4's router will read fd_to_server_ to know which server + // block a request lands on. + const Config* config_; + std::vector listeners_; + std::map fd_to_server_; + // -------------------------------------------------------------------- + + // Legacy single-socket reference — to be removed in Task 5 once + // nothing constructs the legacy way. Pointer not reference so the + // new ctor can leave it NULL. + Socket* legacy_socket_; + Server(const Server&); Server& operator=(const Server&); public: + // Legacy ctor — slated for removal in Task 5. Server(Socket &_socket); + + // Phase 1.5 ctor. Walks cfg.servers(), opens one Socket per + // ListenSpec, populates fd_to_server_. Partial-init failures are + // cleaned up inside the ctor so the caller never sees a half-built + // Server (ctor exceptions skip the dtor). + Server(const Config& cfg); + ~Server(); void run(); - // Returns false when the client has closed the connection (or recv - // failed) so the poll loop knows to remove it. True means keep the fd. bool handle_client_data_read(int client_fd); void handle_client_data_write(int client_fd); void cleanup_sockets(); diff --git a/src/server/server.cpp b/src/server/server.cpp index 7ac5676..2476fdf 100644 --- a/src/server/server.cpp +++ b/src/server/server.cpp @@ -24,85 +24,162 @@ void Server::setup_signal_handlers() { } } -// constructor -Server::Server(Socket &_socket) : socket(_socket) { +// Legacy ctor — to be removed in Task 5. Sets up the single-Socket reference +// path so existing main.cpp still works while we stage the refactor. +Server::Server(Socket &_socket) + : poll_fds() + , config_(NULL) + , listeners_() + , fd_to_server_() + , legacy_socket_(&_socket) +{ setup_signal_handlers(); - LOG_DEBUG(" server() : socket received " + toString(socket.getFileDescriptor())); + LOG_DEBUG(" legacy ctor> socket fd " + + toString(_socket.getFileDescriptor())); } -// destructor -// Note: we do NOT close `socket` here — Server only borrows it (reference -// member). The listening Socket is owned by main() and will close itself -// when its destructor runs. Closing here as well caused a double ::close() -// on the same fd, which is unsafe (the kernel may have reused that fd -// number for an unrelated file by then). + +// Phase 1.5 ctor. The body has three phases: +// 1. Walk cfg.servers() and allocate one Socket per ListenSpec. +// 2. Each allocation that succeeds is pushed into listeners_ and registered +// in fd_to_server_ immediately, so a later failure has a precise list of +// what to clean up. +// 3. setup_signal_handlers() runs last so the invariant "handlers installed +// iff Server fully constructed" holds. +// +// If any Socket ctor throws, we catch (...) and delete every Socket we +// already own, then rethrow. This is necessary because a constructor that +// throws does NOT trigger its own destructor — without manual cleanup we +// leak. +Server::Server(const Config& cfg) + : poll_fds() + , config_(&cfg) + , listeners_() + , fd_to_server_() + , legacy_socket_(NULL) +{ + try { + const std::vector& servers = cfg.servers(); + for (std::size_t i = 0; i < servers.size(); ++i) { + const ServerConfig& srv = servers[i]; + for (std::size_t j = 0; j < srv.listens.size(); ++j) { + const ListenSpec& ls = srv.listens[j]; + + Socket* s = new Socket(ls.host, ls.port); + listeners_.push_back(s); + fd_to_server_[s->getFileDescriptor()] = &srv; + s->startListening(); + } + } + } catch (...) { + // Partial-init cleanup: a Socket ctor or startListening() threw. + // listeners_ contains only the ones that fully succeeded; delete + // each (their dtors close their fds). + for (std::size_t k = 0; k < listeners_.size(); ++k) + delete listeners_[k]; + listeners_.clear(); + fd_to_server_.clear(); + throw; + } + + setup_signal_handlers(); + LOG_DEBUG(" ready, " + toString(listeners_.size()) + + " listener(s)"); +} + +// Destructor: only runs when the ctor completed. We close any still-open +// client fds via the existing cleanup_sockets() helper, then delete every +// owned listener Socket (each Socket's own dtor closes its fd). +// +// Note: under the legacy ctor path (legacy_socket_ != NULL, listeners_ +// empty) we still avoid double-closing the borrowed socket — the loop +// just iterates over an empty listeners_ vector. Server::~Server() { + cleanup_sockets(); + for (std::size_t i = 0; i < listeners_.size(); ++i) + delete listeners_[i]; + listeners_.clear(); } // main server loop void Server::run() { - // add the listening socket to the poll_fds vector - LOG_DEBUG(" run() : adding listening socket to poll_fds"); - struct pollfd pfd_listener = {socket.getFileDescriptor(), POLLIN, 0}; - poll_fds.push_back(pfd_listener); - - // start the server loop + // Seed poll_fds with all listening fds. We support both the legacy + // single-socket path (until Task 5 removes it) and the new vector path. + LOG_DEBUG(" run() : adding listening sockets to poll_fds"); + if (!listeners_.empty()) { + for (std::size_t i = 0; i < listeners_.size(); ++i) { + struct pollfd pfd = {listeners_[i]->getFileDescriptor(), + POLLIN, 0}; + poll_fds.push_back(pfd); + } + } else if (legacy_socket_ != NULL) { + struct pollfd pfd = {legacy_socket_->getFileDescriptor(), POLLIN, 0}; + poll_fds.push_back(pfd); + } + while (g_shutdown != 1) { LOG_DEBUG(" run() : polling for events"); int ret = poll(&poll_fds[0], poll_fds.size(), TIME_OUT_MS); if (ret == -1) { if (errno == EINTR) { - LOG_DEBUG(" run() : EINTR received, checking shutdown flag"); - continue; // Interrupted by signal, check shutdown flag and continue + LOG_DEBUG(" run() : EINTR, checking shutdown"); + continue; } LOG_ERROR(" Poll error"); throw SocketException(" Poll error"); - } - // timeout occurred + } if (ret == 0) continue; - // check for events + for (size_t i = 0; i < poll_fds.size(); ++i) { - if ((poll_fds[i].revents & (POLLHUP | POLLERR)) && poll_fds[i].fd != socket.getFileDescriptor()) { - // handle disconnection or error - LOG_DEBUG(" run(): Client disconnected or error on fd: " + toString(poll_fds[i].fd)); - ::close(poll_fds[i].fd); + const int fd = poll_fds[i].fd; + const bool is_listener = + fd_to_server_.count(fd) > 0 + || (legacy_socket_ != NULL + && fd == legacy_socket_->getFileDescriptor()); + + if ((poll_fds[i].revents & (POLLHUP | POLLERR)) && !is_listener) { + LOG_DEBUG(" run(): Client disconnected/error fd " + + toString(fd)); + ::close(fd); poll_fds.erase(poll_fds.begin() + i--); continue; } if (poll_fds[i].revents & POLLIN) { - if (poll_fds[i].fd == socket.getFileDescriptor()) { - // New incoming connection on the listening socket. - LOG_DEBUG(" run() : new incoming connection"); - int client_fd = socket.acceptClient(); + if (is_listener) { + LOG_DEBUG(" run() : new incoming connection on fd " + + toString(fd)); + int client_fd = -1; + // find the Socket* that owns this fd and accept on it + for (std::size_t k = 0; k < listeners_.size(); ++k) { + if (listeners_[k]->getFileDescriptor() == fd) { + client_fd = listeners_[k]->acceptClient(); + break; + } + } + if (client_fd == -1 && legacy_socket_ != NULL + && fd == legacy_socket_->getFileDescriptor()) + { + client_fd = legacy_socket_->acceptClient(); + } if (client_fd != -1) { - // Subscribe to POLLIN only. A fresh TCP socket is - // already writable, so registering POLLOUT here - // would fire on the very next poll() iteration - // with nothing to actually send. POLLOUT is - // requested later, in Phase 2.5, only once a - // response is queued for this client. struct pollfd pfd_client = {client_fd, POLLIN, 0}; poll_fds.push_back(pfd_client); - LOG_DEBUG(" run() : New client connected, fd: " + toString(client_fd)); + LOG_DEBUG(" run() : New client fd " + + toString(client_fd)); } } else { - // Data available from an existing client. - LOG_DEBUG(" run() : Data available to read on client fd: " + toString(poll_fds[i].fd)); - if (!handle_client_data_read(poll_fds[i].fd)) { - // recv() returned 0 (peer closed) or -1 (error). - // Per the subject we cannot inspect errno after - // read/write, so any failure here means "drop it". - ::close(poll_fds[i].fd); + LOG_DEBUG(" run() : Data ready on client fd " + + toString(fd)); + if (!handle_client_data_read(fd)) { + ::close(fd); poll_fds.erase(poll_fds.begin() + i--); - continue; // skip the POLLOUT check below for this slot + continue; } } } if (poll_fds[i].revents & POLLOUT) { - // Phase 1 stub: nothing is queued for writing yet, and we - // no longer register POLLOUT on accept, so in practice this - // branch is dormant until Phase 2 starts queuing responses. - LOG_DEBUG(" run() : Ready to write on client fd: " + toString(poll_fds[i].fd)); - handle_client_data_write(poll_fds[i].fd); + LOG_DEBUG(" run() : Ready to write on fd " + + toString(fd)); + handle_client_data_write(fd); } } } @@ -142,13 +219,21 @@ void Server::handle_client_data_write(int client_fd) { LOG_DEBUG(" Writing data to client fd: " + toString(client_fd)); } -// Cleanup function to close all client sockets +// Closes any still-open *client* fds in poll_fds. Listener fds are NOT +// closed here — their lifetime belongs to the owning Socket objects (either +// legacy_socket_ or the listeners_ vector); closing here would double-close. void Server::cleanup_sockets() { - LOG_DEBUG(" cleanup_sockets() : Cleaning up client sockets"); + LOG_DEBUG(" cleanup_sockets() : closing client fds"); for (size_t i = 0; i < poll_fds.size(); ++i) { - if (poll_fds[i].fd != socket.getFileDescriptor()) { - LOG_DEBUG(" cleanup_sockets() : Closing client socket fd: " + toString(poll_fds[i].fd)); - ::close(poll_fds[i].fd); + const int fd = poll_fds[i].fd; + const bool is_listener = + fd_to_server_.count(fd) > 0 + || (legacy_socket_ != NULL + && fd == legacy_socket_->getFileDescriptor()); + if (!is_listener) { + LOG_DEBUG(" cleanup_sockets() : closing client fd " + + toString(fd)); + ::close(fd); } } poll_fds.clear(); From 2a8625c058d2ec737f60dd3e9622f5ad24b30f5b Mon Sep 17 00:00:00 2001 From: ibrohimovmuhammad2020 Date: Thu, 28 May 2026 22:28:25 +0200 Subject: [PATCH 07/13] feat(main): argv + default-path config loading, top-level try/catch --- src/main.cpp | 25 ++++++++++++++++--------- tests/phase15_tests.sh | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 9 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index cf92fa2..df3c943 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -92,9 +92,6 @@ static int dumpConfig(const std::string& path) { // --- end DEV-ONLY --------------------------------------------------------- int main(int argc, char **argv) { - (void)argc; - (void)argv; - if (getenv("DEBUG")) { Logger::setLevel(Logger::DEBUG); LOG_INFO("Debug mode enabled."); @@ -104,10 +101,20 @@ int main(int argc, char **argv) { if (const char* path = getenv("CONFIG_DUMP")) return dumpConfig(path); - std::cout << "WebServ starting..." << std::endl; - Socket server_socket(8080); - server_socket.startListening(); - Server web_server(server_socket); - web_server.run(); + if (argc > 2) { + std::cerr << "usage: ./webserv [config_file]" << std::endl; + return 1; + } + const std::string path = (argc == 2) ? argv[1] : "config/default.conf"; + + try { + Config cfg; + cfg.load(path); + Server srv(cfg); + srv.run(); + } catch (const std::exception& e) { + std::cerr << "fatal: " << e.what() << std::endl; + return 1; + } return 0; -} \ No newline at end of file +} diff --git a/tests/phase15_tests.sh b/tests/phase15_tests.sh index d7286bf..2b98d0a 100755 --- a/tests/phase15_tests.sh +++ b/tests/phase15_tests.sh @@ -82,6 +82,44 @@ if [ ! -x ./webserv ]; then exit 2 fi +# Task 1 (deferred): duplicate listen rejected when validator runs through main() +assert_exits_nonzero_with \ + "task1_duplicate_listen_rejected" \ + "config/test_duplicate.conf" \ + "duplicate listen 0.0.0.0:8080" + +# Task 4: missing config file +assert_exits_nonzero_with \ + "task4_missing_config" \ + "/tmp/definitely-does-not-exist.conf" \ + "fatal:" + +# Task 4: argc > 2 (too many args) — bespoke check because helper takes 1 path arg +out=$(./webserv config/default.conf foo bar 2>&1) +rc=$? +if [ $rc -ne 0 ] && echo "$out" | grep -qF "usage:"; then + PASS=$((PASS+1)) +else + FAIL=$((FAIL+1)) + FAILED_TESTS+=("task4_too_many_args: rc=$rc, output=$out") +fi + +# Task 4: default-path happy case — webserv with no arg listens on 8080 +./webserv >/tmp/webserv.out 2>&1 & +PID=$! +sleep 1 +if ! kill -0 "$PID" 2>/dev/null; then + FAIL=$((FAIL+1)); FAILED_TESTS+=("task4_default_path: webserv exited: +$(cat /tmp/webserv.out)") +else + if lsof -iTCP:8080 -sTCP:LISTEN -P -n 2>/dev/null | grep -q webserv; then + PASS=$((PASS+1)) + else + FAIL=$((FAIL+1)); FAILED_TESTS+=("task4_default_path: not listening on 8080") + fi + kill -INT "$PID" 2>/dev/null; wait "$PID" 2>/dev/null +fi + # --- tests appear here, added per task --- echo From 6923124c4ee9302be8a0e925f325c349918b2296 Mon Sep 17 00:00:00 2001 From: ibrohimovmuhammad2020 Date: Thu, 28 May 2026 22:41:36 +0200 Subject: [PATCH 08/13] refactor: remove legacy single-socket code paths --- includes/server.hpp | 8 ------- includes/socket.hpp | 1 - src/server/server.cpp | 52 +++++++------------------------------------ src/socket/socket.cpp | 28 ----------------------- 4 files changed, 8 insertions(+), 81 deletions(-) diff --git a/includes/server.hpp b/includes/server.hpp index 3b4a75b..3bac4be 100644 --- a/includes/server.hpp +++ b/includes/server.hpp @@ -29,18 +29,10 @@ class Server { std::map fd_to_server_; // -------------------------------------------------------------------- - // Legacy single-socket reference — to be removed in Task 5 once - // nothing constructs the legacy way. Pointer not reference so the - // new ctor can leave it NULL. - Socket* legacy_socket_; - Server(const Server&); Server& operator=(const Server&); public: - // Legacy ctor — slated for removal in Task 5. - Server(Socket &_socket); - // Phase 1.5 ctor. Walks cfg.servers(), opens one Socket per // ListenSpec, populates fd_to_server_. Partial-init failures are // cleaned up inside the ctor so the caller never sees a half-built diff --git a/includes/socket.hpp b/includes/socket.hpp index 6cf7345..61b95b9 100644 --- a/includes/socket.hpp +++ b/includes/socket.hpp @@ -23,7 +23,6 @@ class Socket { Socket& operator=(const Socket&); public: - Socket(int port); // Phase 1.5: config-driven listener constructor. // Resolves `host` (a numeric IPv4 string like "0.0.0.0" or "127.0.0.1") // via getaddrinfo with AI_NUMERICHOST so no DNS happens. Sets diff --git a/src/server/server.cpp b/src/server/server.cpp index 2476fdf..391c6c0 100644 --- a/src/server/server.cpp +++ b/src/server/server.cpp @@ -24,20 +24,6 @@ void Server::setup_signal_handlers() { } } -// Legacy ctor — to be removed in Task 5. Sets up the single-Socket reference -// path so existing main.cpp still works while we stage the refactor. -Server::Server(Socket &_socket) - : poll_fds() - , config_(NULL) - , listeners_() - , fd_to_server_() - , legacy_socket_(&_socket) -{ - setup_signal_handlers(); - LOG_DEBUG(" legacy ctor> socket fd " - + toString(_socket.getFileDescriptor())); -} - // Phase 1.5 ctor. The body has three phases: // 1. Walk cfg.servers() and allocate one Socket per ListenSpec. // 2. Each allocation that succeeds is pushed into listeners_ and registered @@ -55,7 +41,6 @@ Server::Server(const Config& cfg) , config_(&cfg) , listeners_() , fd_to_server_() - , legacy_socket_(NULL) { try { const std::vector& servers = cfg.servers(); @@ -89,10 +74,6 @@ Server::Server(const Config& cfg) // Destructor: only runs when the ctor completed. We close any still-open // client fds via the existing cleanup_sockets() helper, then delete every // owned listener Socket (each Socket's own dtor closes its fd). -// -// Note: under the legacy ctor path (legacy_socket_ != NULL, listeners_ -// empty) we still avoid double-closing the borrowed socket — the loop -// just iterates over an empty listeners_ vector. Server::~Server() { cleanup_sockets(); for (std::size_t i = 0; i < listeners_.size(); ++i) @@ -102,17 +83,11 @@ Server::~Server() { // main server loop void Server::run() { - // Seed poll_fds with all listening fds. We support both the legacy - // single-socket path (until Task 5 removes it) and the new vector path. + // Seed poll_fds with all listening fds. LOG_DEBUG(" run() : adding listening sockets to poll_fds"); - if (!listeners_.empty()) { - for (std::size_t i = 0; i < listeners_.size(); ++i) { - struct pollfd pfd = {listeners_[i]->getFileDescriptor(), - POLLIN, 0}; - poll_fds.push_back(pfd); - } - } else if (legacy_socket_ != NULL) { - struct pollfd pfd = {legacy_socket_->getFileDescriptor(), POLLIN, 0}; + for (std::size_t i = 0; i < listeners_.size(); ++i) { + struct pollfd pfd = {listeners_[i]->getFileDescriptor(), + POLLIN, 0}; poll_fds.push_back(pfd); } @@ -131,10 +106,7 @@ void Server::run() { for (size_t i = 0; i < poll_fds.size(); ++i) { const int fd = poll_fds[i].fd; - const bool is_listener = - fd_to_server_.count(fd) > 0 - || (legacy_socket_ != NULL - && fd == legacy_socket_->getFileDescriptor()); + const bool is_listener = fd_to_server_.count(fd) > 0; if ((poll_fds[i].revents & (POLLHUP | POLLERR)) && !is_listener) { LOG_DEBUG(" run(): Client disconnected/error fd " @@ -155,11 +127,6 @@ void Server::run() { break; } } - if (client_fd == -1 && legacy_socket_ != NULL - && fd == legacy_socket_->getFileDescriptor()) - { - client_fd = legacy_socket_->acceptClient(); - } if (client_fd != -1) { struct pollfd pfd_client = {client_fd, POLLIN, 0}; poll_fds.push_back(pfd_client); @@ -220,16 +187,13 @@ void Server::handle_client_data_write(int client_fd) { } // Closes any still-open *client* fds in poll_fds. Listener fds are NOT -// closed here — their lifetime belongs to the owning Socket objects (either -// legacy_socket_ or the listeners_ vector); closing here would double-close. +// closed here — their lifetime belongs to the owning Socket objects in the +// listeners_ vector; closing here would double-close. void Server::cleanup_sockets() { LOG_DEBUG(" cleanup_sockets() : closing client fds"); for (size_t i = 0; i < poll_fds.size(); ++i) { const int fd = poll_fds[i].fd; - const bool is_listener = - fd_to_server_.count(fd) > 0 - || (legacy_socket_ != NULL - && fd == legacy_socket_->getFileDescriptor()); + const bool is_listener = fd_to_server_.count(fd) > 0; if (!is_listener) { LOG_DEBUG(" cleanup_sockets() : closing client fd " + toString(fd)); diff --git a/src/socket/socket.cpp b/src/socket/socket.cpp index 15a5071..1365dfd 100644 --- a/src/socket/socket.cpp +++ b/src/socket/socket.cpp @@ -3,34 +3,6 @@ #include -// Constructor with port number and create a socket with AF_INET and SOCK_STREAM -Socket::Socket(int port) : fd_socket(socket(AF_INET, SOCK_STREAM, 0)) { - if (fd_socket == -1) { - LOG_ERROR(" Failed to create socket"); - ::close(fd_socket); - throw SocketException("Failed to create socket"); - } - - // set the socket to non-blocking - if (fcntl(fd_socket, F_SETFL, O_NONBLOCK) == -1) { - LOG_ERROR(" Failed to set socket to non-blocking"); - ::close(fd_socket); - throw SocketException("Failed to set socket to non-blocking"); - } - - // bind the socket to the port - _address.sin_family = AF_INET; - _address.sin_port = htons(port); - _address.sin_addr.s_addr = INADDR_ANY; - if (bind(fd_socket, (struct sockaddr *)&_address, sizeof(_address)) == -1) { - LOG_ERROR(" Failed to bind socket"); - ::close(fd_socket); - throw SocketException("Failed to bind socket"); - } - LOG_DEBUG(" Socket bound to port " + toString(port)); - LOG_DEBUG(" Socket file descriptor: " + toString(fd_socket)); -} - // destructor to close the socket will call the close method Socket::~Socket() { Socket::close(); From f4a1e7bac1530d35e8ef985dc352a1f26839c12d Mon Sep 17 00:00:00 2001 From: ibrohimovmuhammad2020 Date: Thu, 28 May 2026 22:44:23 +0200 Subject: [PATCH 09/13] test: cover multi-port and multi-server bind paths --- config/test_multi_port.conf | 12 ++++++++++++ config/test_two_servers.conf | 13 +++++++++++++ tests/phase15_tests.sh | 12 ++++++++++++ 3 files changed, 37 insertions(+) create mode 100644 config/test_multi_port.conf create mode 100644 config/test_two_servers.conf diff --git a/config/test_multi_port.conf b/config/test_multi_port.conf new file mode 100644 index 0000000..2a8a97d --- /dev/null +++ b/config/test_multi_port.conf @@ -0,0 +1,12 @@ +server { + listen 8080; + listen 8081; + server_name localhost; + root ./www; + client_max_body_size 10M; + + location / { + allowed_methods GET; + index index.html; + } +} diff --git a/config/test_two_servers.conf b/config/test_two_servers.conf new file mode 100644 index 0000000..96d7a43 --- /dev/null +++ b/config/test_two_servers.conf @@ -0,0 +1,13 @@ +server { + listen 8090; + server_name a; + root ./www; + location / { allowed_methods GET; } +} + +server { + listen 9090; + server_name b; + root ./www; + location / { allowed_methods GET; } +} diff --git a/tests/phase15_tests.sh b/tests/phase15_tests.sh index 2b98d0a..d86a56b 100755 --- a/tests/phase15_tests.sh +++ b/tests/phase15_tests.sh @@ -120,6 +120,18 @@ else kill -INT "$PID" 2>/dev/null; wait "$PID" 2>/dev/null fi +# Task 6: one server, two listen directives +assert_listens_on_ports \ + "task6_multi_port_same_server" \ + "config/test_multi_port.conf" \ + 8080 8081 + +# Task 6: two server blocks on distinct ports +assert_listens_on_ports \ + "task6_two_servers" \ + "config/test_two_servers.conf" \ + 8090 9090 + # --- tests appear here, added per task --- echo From aacd242677c70e27bf5a84287003e647f268b59f Mon Sep 17 00:00:00 2001 From: ibrohimovmuhammad2020 Date: Thu, 28 May 2026 22:55:39 +0200 Subject: [PATCH 10/13] polish: document Config-immutability invariant, add bad-syntax test, drop unused forward decl --- config/test_bad_syntax.conf | 7 +++++++ includes/server.hpp | 8 ++++++-- tests/phase15_tests.sh | 6 ++++++ 3 files changed, 19 insertions(+), 2 deletions(-) create mode 100644 config/test_bad_syntax.conf diff --git a/config/test_bad_syntax.conf b/config/test_bad_syntax.conf new file mode 100644 index 0000000..fa22deb --- /dev/null +++ b/config/test_bad_syntax.conf @@ -0,0 +1,7 @@ +server { + listen 8080 + server_name broken; + root ./www; + location / { + allowed_methods GET; + } diff --git a/includes/server.hpp b/includes/server.hpp index 3bac4be..314a39b 100644 --- a/includes/server.hpp +++ b/includes/server.hpp @@ -11,8 +11,6 @@ #include "string_utils.hpp" -class Socket; - extern volatile sig_atomic_t g_shutdown; class Server { @@ -24,6 +22,12 @@ class Server { // than this Server, so the pointer is safe for our lifetime. // Phase 2.4's router will read fd_to_server_ to know which server // block a request lands on. + // + // INVARIANT: `Config` must NOT be mutated after `Config::load()` for + // the lifetime of this Server. fd_to_server_ stores raw pointers to + // elements of cfg.servers()'s internal vector; any push_back would + // reallocate that vector and invalidate every pointer in the map. + // Today main() loads cfg once and never touches it again — keep that. const Config* config_; std::vector listeners_; std::map fd_to_server_; diff --git a/tests/phase15_tests.sh b/tests/phase15_tests.sh index d86a56b..570e243 100755 --- a/tests/phase15_tests.sh +++ b/tests/phase15_tests.sh @@ -132,6 +132,12 @@ assert_listens_on_ports \ "config/test_two_servers.conf" \ 8090 9090 +# Polish: bad syntax — config parser must reject cleanly +assert_exits_nonzero_with \ + "polish_bad_syntax_rejected" \ + "config/test_bad_syntax.conf" \ + "fatal:" + # --- tests appear here, added per task --- echo From e3510af7cd222377635038571c82ec24c308f3e5 Mon Sep 17 00:00:00 2001 From: ibrohimovmuhammad2020 Date: Thu, 28 May 2026 23:04:21 +0200 Subject: [PATCH 11/13] chore: gitignore docs/superpowers/ (specs and plans are local-only artifacts) --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index 2dec756..6af7ddf 100644 --- a/.gitignore +++ b/.gitignore @@ -40,3 +40,6 @@ webserv # debug information files *.dwo + +# superpowers working artifacts (specs + plans live locally only) +docs/superpowers/ From 0f6649173bf5dcd64a4f62439309abd27eacb8ce Mon Sep 17 00:00:00 2001 From: ibrohimovmuhammad2020 Date: Thu, 28 May 2026 23:08:18 +0200 Subject: [PATCH 12/13] docs(kanban): mark Phase 1.5 completed --- kanban.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/kanban.md b/kanban.md index 8283003..1f8f0f0 100644 --- a/kanban.md +++ b/kanban.md @@ -195,7 +195,7 @@ --- -### [ ] 1.5: Load Config & Initialize Server on Startup +### [ Completed ] 1.5: Load Config & Initialize Server on Startup **Assignee:** Dev B **Description:** - Implement `main(int argc, char** argv)` @@ -207,10 +207,10 @@ - Graceful error handling: print to stderr and exit(1) on config error **DoD:** -- [ ] `./webserv` loads default.conf -- [ ] `./webserv custom.conf` loads custom config -- [ ] All listening ports open successfully -- [ ] Error messages clear on config failure +- [x] `./webserv` loads default.conf (path: `config/default.conf`) +- [x] `./webserv custom.conf` loads custom config +- [x] All listening ports open successfully +- [x] Error messages clear on config failure --- From 84aaa8f4c0520e3344488d87e357306621f8514a Mon Sep 17 00:00:00 2001 From: ibrohimovmuhammad2020 Date: Thu, 28 May 2026 23:22:01 +0200 Subject: [PATCH 13/13] refactor(server): O(log N) listener lookup via fd_to_listener_ map run() used to walk listeners_ linearly to find which Socket* owned an incoming POLLIN fd. Cheap with one listener, painful by Phase 4 stress testing. New parallel map is kept in lock-step with fd_to_server_ in the ctor, so the existing is_listener check still works unchanged. --- includes/server.hpp | 5 +++++ src/server/server.cpp | 17 ++++++++--------- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/includes/server.hpp b/includes/server.hpp index 314a39b..e90fb2d 100644 --- a/includes/server.hpp +++ b/includes/server.hpp @@ -31,6 +31,11 @@ class Server { const Config* config_; std::vector listeners_; std::map fd_to_server_; + // Parallel map keyed on the same listener fds as fd_to_server_. Lets + // run() jump straight to the owning Socket on POLLIN instead of + // scanning listeners_ linearly — matters once Phase 4's stress test + // pushes the listener count up. + std::map fd_to_listener_; // -------------------------------------------------------------------- Server(const Server&); diff --git a/src/server/server.cpp b/src/server/server.cpp index 391c6c0..0fbd951 100644 --- a/src/server/server.cpp +++ b/src/server/server.cpp @@ -41,6 +41,7 @@ Server::Server(const Config& cfg) , config_(&cfg) , listeners_() , fd_to_server_() + , fd_to_listener_() { try { const std::vector& servers = cfg.servers(); @@ -51,7 +52,8 @@ Server::Server(const Config& cfg) Socket* s = new Socket(ls.host, ls.port); listeners_.push_back(s); - fd_to_server_[s->getFileDescriptor()] = &srv; + fd_to_server_[s->getFileDescriptor()] = &srv; + fd_to_listener_[s->getFileDescriptor()] = s; s->startListening(); } } @@ -63,6 +65,7 @@ Server::Server(const Config& cfg) delete listeners_[k]; listeners_.clear(); fd_to_server_.clear(); + fd_to_listener_.clear(); throw; } @@ -119,14 +122,10 @@ void Server::run() { if (is_listener) { LOG_DEBUG(" run() : new incoming connection on fd " + toString(fd)); - int client_fd = -1; - // find the Socket* that owns this fd and accept on it - for (std::size_t k = 0; k < listeners_.size(); ++k) { - if (listeners_[k]->getFileDescriptor() == fd) { - client_fd = listeners_[k]->acceptClient(); - break; - } - } + // O(log N) jump straight to the owning Socket. The map is + // kept in lock-step with fd_to_server_ in the ctor, so a + // hit here is guaranteed whenever is_listener is true. + int client_fd = fd_to_listener_[fd]->acceptClient(); if (client_fd != -1) { struct pollfd pfd_client = {client_fd, POLLIN, 0}; poll_fds.push_back(pfd_client);