diff options
| author | Paul Buetow <paul@buetow.org> | 2026-07-02 00:53:24 +0300 |
|---|---|---|
| committer | Paul Buetow <paul@buetow.org> | 2026-07-02 00:53:24 +0300 |
| commit | 58602a28d2c92b603208f3e01c14c169d28cc7b0 (patch) | |
| tree | 502d2e187ea29f2cc840e22d299d752d59088a3c /yhttpd/src | |
| parent | 6c3a65b577f002f3219d03498c66205434765179 (diff) | |
yhttpd: port ychat fixes, get it building + stable in Docker (task 9s0)
Ports the ychat revival fixes (unsigned/size_t npos truncation, ofstream
== NULL, tool::trim OOB, CGI popen -> execve, www.yChat.org links, g++
version gate, config.pl -I., NCURSES/CLI-disabled build) so yhttpd builds
on modern GCC in a Rocky Linux 9 container, plus yhttpd-specific fixes
found while verifying it under concurrent load:
- listen() backlog was hardcoded to 1; bumped to SOMAXCONN.
- sock::_close() closed sockets with unread request bytes still in the
kernel receive buffer (read_http() only reads the GET line), so Linux
sent an abortive RST instead of a FIN, racing the client's read of the
response ("connection reset by peer" even though it was delivered).
Fixed with a non-blocking, bounded drain before close() - confirmed via
tcpdump: RSTs on every response before, zero after, across 140+
requests / concurrent bursts of 20.
- Removed a duplicate _make_server_socket() call in start() (wrap.cpp's
init_wrapper() already makes it before start() runs) that leaked a fd
and would have double-initialized SSL if OPENSSL is ever enabled;
caught by fresh-context review, documented honestly in DOCKER.md.
- src/configure's dependency-checker predates 64-bit multilib distros
(only checked /usr/lib, never /usr/lib64) and was missing an ncur
move-aside entry for the NCURSES-disabled build.
Added Dockerfile/.dockerignore/DOCKER.md documenting the build, the fixes,
and the one known-but-unfixed landmine (a SIGILL heap corruption in
sock::_close that reproduces on newer host GCC/glibc but not in the
container - latent, not fixed).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Diffstat (limited to 'yhttpd/src')
| -rw-r--r-- | yhttpd/src/conf/conf.cpp | 2 | ||||
| -rwxr-xr-x | yhttpd/src/configure | 14 | ||||
| -rw-r--r-- | yhttpd/src/glob.h | 18 | ||||
| -rw-r--r-- | yhttpd/src/html.cpp | 2 | ||||
| -rw-r--r-- | yhttpd/src/logd.cpp | 15 | ||||
| -rw-r--r-- | yhttpd/src/main.cpp | 2 | ||||
| -rw-r--r-- | yhttpd/src/msgs.h | 8 | ||||
| -rw-r--r-- | yhttpd/src/reqp.cpp | 9 | ||||
| -rw-r--r-- | yhttpd/src/sock/sock.cpp | 55 | ||||
| -rw-r--r-- | yhttpd/src/tool/tool.cpp | 124 |
10 files changed, 173 insertions, 76 deletions
diff --git a/yhttpd/src/conf/conf.cpp b/yhttpd/src/conf/conf.cpp index 25884ca..f987755 100644 --- a/yhttpd/src/conf/conf.cpp +++ b/yhttpd/src/conf/conf.cpp @@ -174,7 +174,7 @@ conf::get_vector(string s_key) vector<string> vec_ret; string s_val = get_elem(s_key); - for (unsigned i_pos = s_val.find(" "); i_pos != string::npos; i_pos = s_val.find(" ")) + for (size_t i_pos = s_val.find(" "); i_pos != string::npos; i_pos = s_val.find(" ")) { vec_ret.push_back(s_val.substr(0, i_pos)); s_val = s_val.substr(i_pos+1); diff --git a/yhttpd/src/configure b/yhttpd/src/configure index 54cf650..b67da46 100755 --- a/yhttpd/src/configure +++ b/yhttpd/src/configure @@ -55,8 +55,11 @@ perl -e ' $ENV{HOME}."/lib", $ENV{HOME}."/usr/lib", "/lib", + "/lib64", "/usr/lib", + "/usr/lib64", "/usr/local/lib", + "/usr/local/lib64", "/usr/pkg/lib", "/opt/lib", "/opt/local/lib" @@ -65,6 +68,7 @@ perl -e ' my %dependfiles = ( database => ["data"], ycurses => ["curses", "ycui.cpp", "ycui.h"], + ncur => ["ncur"], logging => ["logd.cpp", "logd.h"], cli => ["cli"], opnssl => ["sock/sslsock.cpp", "sock/sslsock.h"] @@ -101,6 +105,16 @@ perl -e ' `mv ../backuped/$_ .` for @{$dependfiles{cli}}; } + if ( /\/\/#define NCURSES/ ) { + remove_from_array("ncurses.h",\@headers); + remove_from_array("libncurses.so",\@libs); + mkdir "../backuped" unless -d "../backuped"; + `mv $_ ../backuped` for @{$dependfiles{ncur}}; + } + elsif ( /^#define NCURSES/ && !-d "ncur") { + `mv ../backuped/$_ .` for @{$dependfiles{ncur}}; + } + if ( /\/\/#define YCURSES/ ) { for ("ncurses", "menu", "panel") { remove_from_array("$_.h",\@headers); diff --git a/yhttpd/src/glob.h b/yhttpd/src/glob.h index a2c1334..2283f1e 100644 --- a/yhttpd/src/glob.h +++ b/yhttpd/src/glob.h @@ -24,7 +24,7 @@ /* - CONFIG - Should yhttpd get compiled with comand line interface support? */ -#define CLI +//#define CLI /* - CONFIG - What should be the name of the config file? @@ -70,7 +70,7 @@ /* - CONFIG - Should yhttpd get compiled with ncurses support? */ -#define NCURSES +//#define NCURSES /* - CONFIG - Please specify the maximum length of a HTTP post request. @@ -84,12 +84,20 @@ #define READBUF 2048 /* - CONFIG - - Please specify the maximum length of a line read from a socket + Please specify the maximum length of a line read from a socket or a file. ( config-file, html-template ) */ #define READSOCK 2048 /* - CONFIG - + Upper bound (bytes) on how much unread request data sock::_close() will + drain from a socket before giving up and closing anyway. Bounds the + non-blocking drain loop against a client that keeps streaming data after + we've half-closed the connection. +*/ +#define DRAINMAX 8192 + +/* - CONFIG - In which prefix should yhttpd be installed if typing gmake inst- all? */ @@ -175,7 +183,7 @@ using namespace std; -typedef int function( void *v_arg ); +typedef int mod_func_t( void *v_arg ); struct container { @@ -184,7 +192,7 @@ struct container struct dynmod { - function *the_func ; + mod_func_t *the_func ; void *the_module; }; diff --git a/yhttpd/src/html.cpp b/yhttpd/src/html.cpp index ace041a..5794612 100644 --- a/yhttpd/src/html.cpp +++ b/yhttpd/src/html.cpp @@ -78,7 +78,7 @@ html::parse( map<string,string> &map_params ) } // find %%KEY%% token and substituate those. - unsigned pos[2]; + size_t pos[2]; pos[0] = pos[1] = 0; for(;;) diff --git a/yhttpd/src/logd.cpp b/yhttpd/src/logd.cpp index 664cd14..d65c9bd 100644 --- a/yhttpd/src/logd.cpp +++ b/yhttpd/src/logd.cpp @@ -7,6 +7,9 @@ #ifdef LOGGING #include <fstream> +#include <iostream> +#include <cstring> +#include <cerrno> logd::logd( string s_filename, string s_log_lines ) { @@ -62,9 +65,13 @@ logd::flush() ofstream of_output; of_output.open(s_logfile.c_str(), ios::app); - if( of_output == NULL ) + if ( ! of_output.is_open() ) { - wrap::system_message( LOGERR1 + s_logfile ); + // Log to stderr only; do NOT call wrap::system_message() - that routes + // back through LOGD->log_simple_line -> flush on the same failing logd + // and would recurse infinitely (stack overflow). + cerr << "yhttpd: could not open logfile '" << s_logfile << "' (" + << strerror(errno) << ")" << endl; exit(1); } @@ -118,7 +125,7 @@ void logd::set_logfile( string s_path, string s_filename ) { // Remove "/" from filename! - unsigned i_pos = s_filename.find( "/" ); + size_t i_pos = s_filename.find( "/" ); while ( i_pos != string::npos ) { s_filename.replace( i_pos, 1, "SLASH" ); @@ -149,7 +156,7 @@ logd::flush_logs() string logd::remove_html_tags( string s_logs ) { - unsigned pos[2]; + size_t pos[2]; while ( (pos[0] = s_logs.find("<")) != string::npos ) { diff --git a/yhttpd/src/main.cpp b/yhttpd/src/main.cpp index 87f1cab..b18f6cc 100644 --- a/yhttpd/src/main.cpp +++ b/yhttpd/src/main.cpp @@ -1,5 +1,5 @@ /* - * yhttpd; Contact: www.yChat.org; Mail@yChat.org + * yhttpd; Contact: https://codeberg.org/snonux/ychat; Mail@yChat.org * Copyright (C) 2003 Paul C. Buetow, Volker Richter * Copyright (C) 2004 Paul C. Buetow * Copyright (C) 2005 EXA Digital Solutions GbR diff --git a/yhttpd/src/msgs.h b/yhttpd/src/msgs.h index fdfb68c..bedf4ec 100644 --- a/yhttpd/src/msgs.h +++ b/yhttpd/src/msgs.h @@ -9,7 +9,7 @@ #define HTTPDFLO "Chat: Flooding (" #define CFILEOK "Parsing config file" #define CFILEFA "Failed opening config file!" -#define CONTACT "Contact: http://www.yChat.org, Mail@yChat.org, ICQ: 11655527" +#define CONTACT "Contact: https://codeberg.org/snonux/ychat, Mail@yChat.org, ICQ: 11655527" #define CLRHTML "HTML: Cleared the document cache " #define CLIWELC "Command Line Interface (type help for a list of all commands)" #define CLIPRMO ">> " @@ -125,9 +125,9 @@ #define XMLER1 "XML Error: Unable to load file " #define VERSION "0.8" #define BRANCH "CURRENT" -#define BUILDNR 4003 -#define UNAME "FreeBSD 5.4-DEVEL-p3 i386" -#define COMPOPT "Using built-in specs.; Configured with: FreeBSD/i386 system compiler; Thread model: posix; gcc version 3.4 [FreeBSD] 20040728; 3.4; g++" +#define BUILDNR 4027 +#define UNAME "Linux 7.0.11-200.fc44.x86_64 x86_64" +#define COMPOPT "16; g++" #define YCUSAGE "Usage: ./yhttpd {h|v}|{o confkey confvalue}\n" #define HEADER1 "HTTP/1.1 200 OK\r\n" diff --git a/yhttpd/src/reqp.cpp b/yhttpd/src/reqp.cpp index df9de5d..4ce30d9 100644 --- a/yhttpd/src/reqp.cpp +++ b/yhttpd/src/reqp.cpp @@ -22,7 +22,7 @@ void reqp::get_request_parameters( string s_parameters, map<string,string>& map_params ) { string s_tmp; - unsigned i_pos, i_pos2; + size_t i_pos, i_pos2; while( (i_pos = s_parameters.find("&")) != string::npos ) { @@ -46,7 +46,7 @@ reqp::get_request_parameters( string s_parameters, map<string,string>& map_param string reqp::get_url( string s_req, map<string, string> &map_params, int& i_postpayloadoffset ) { - unsigned i_pos, i_pos2; + size_t i_pos, i_pos2; string s_vars( "" ); string s_ret; int i_req; @@ -111,7 +111,6 @@ reqp::get_url( string s_req, map<string, string> &map_params, int& i_postpayload if ( s_ret.empty() ) s_ret = wrap::CONF->get_elem( "httpd.startsite" ); - else s_ret = remove_dots(s_ret); @@ -211,7 +210,7 @@ reqp::url_decode( string s_url ) string reqp::get_from_header( string s_req, string s_hdr ) { - unsigned i_pos[2]; + size_t i_pos[2]; if ( (i_pos[0] = s_req.find( s_hdr, 0 )) == string::npos ) return ""; @@ -274,7 +273,7 @@ string reqp::remove_dots( string s_ret ) { // remove ".." from the request. - unsigned i_pos; + size_t i_pos; if ( (i_pos = s_ret.find( ".." )) != string::npos ) return remove_dots(s_ret.substr(0, i_pos)); diff --git a/yhttpd/src/sock/sock.cpp b/yhttpd/src/sock/sock.cpp index 5ae0be1..279123d 100644 --- a/yhttpd/src/sock/sock.cpp +++ b/yhttpd/src/sock/sock.cpp @@ -3,6 +3,7 @@ #include <arpa/inet.h> #include <errno.h> +#include <fcntl.h> #include <sys/types.h> #include <unistd.h> @@ -40,8 +41,39 @@ sock::_read(socketcontainer *p_sock, char *sz, int len) int sock::_close(socketcontainer *p_sock) { - shutdown( p_sock->i_sock, 2 ); - close ( p_sock->i_sock ); + int i_sock = p_sock->i_sock; + + // read_http() only ever consumes the GET request line (POST bodies are + // fully read, but GET headers after the first line are intentionally + // skipped) - so the kernel receive buffer usually still has unread + // bytes at this point. Closing a socket with unread data makes Linux + // send an abortive RST instead of a graceful FIN, which can race the + // client's read of the response we just sent and show up as + // "connection reset by peer" even though the response was delivered. + // The client already sent its whole request before we ever got here + // (we already read the first line via blocking reads), so any unread + // header bytes are already sitting in the receive buffer - a + // non-blocking drain picks them up with no added latency, unlike a + // timed wait for a client-initiated close that (for ordinary HTTP + // clients waiting on the response) would rarely arrive in time anyway. + shutdown( i_sock, SHUT_WR ); + + // Skip the drain (fall straight through to close()) if we can't make the + // socket non-blocking - reading below would otherwise risk blocking this + // worker thread on a client that never sends more data. + int i_flags = fcntl( i_sock, F_GETFL, 0 ); + if ( i_flags != -1 && fcntl( i_sock, F_SETFL, i_flags | O_NONBLOCK ) != -1 ) + { + // Bounded: a client that keeps streaming data after our SHUT_WR could + // otherwise keep this non-blocking loop spinning for as long as it + // keeps pushing bytes. + char c_drain[256]; + int i_drained = 0; + while ( i_drained < DRAINMAX && read( i_sock, c_drain, sizeof(c_drain) ) > 0 ) + i_drained += sizeof(c_drain); + } + + close( i_sock ); delete p_sock; } @@ -292,7 +324,7 @@ sock::read_write(socketcontainer* p_sock) // get the s_rep ( s_html response which will be send imediatly to the client struct sockaddr_in client; - size_t size = sizeof(client); + socklen_t size = sizeof(client); getpeername(i_sock, (struct sockaddr *)&client, &size); @@ -353,7 +385,6 @@ sock::start() { wrap::system_message( SOCKSRV ); pool* p_pool = wrap::POOL; - int i_sock = i_server_sock; #ifdef NCURSES @@ -361,15 +392,25 @@ sock::start() p_pool->print_pool_size(); #endif - int i_port = tool::string2int( wrap::CONF->get_elem( "httpd.serverport" ) ); + // i_server_sock is already a bound listening-socket fd here: + // wrap::init_wrapper() (wrap.cpp) calls _make_server_socket() before + // start() ever runs. Do not call it again - besides leaking this fd, a + // second call goes through sslsock's virtual override too (if OPENSSL + // is ever enabled), which would leak the first SSL_CTX and re-read the + // cert/key files. + int i_sock = i_server_sock; _main_loop_init(); int i; fd_set active_fd_set, read_fd_set; struct sockaddr_in clientname; - size_t size; + socklen_t size; - if (listen (i_sock, 1) < 0) + // Backlog was hardcoded to 1: with select()'s single accept()-per-wakeup + // loop below, any connection arriving while one is already queued got a + // kernel-level reset (visible as intermittent connection resets under + // concurrent load). SOMAXCONN lets the kernel queue a normal burst. + if (listen (i_sock, SOMAXCONN) < 0) { wrap::system_message( LISTERR ); exit( EXIT_FAILURE ); diff --git a/yhttpd/src/tool/tool.cpp b/yhttpd/src/tool/tool.cpp index dbb1f22..90314fe 100644 --- a/yhttpd/src/tool/tool.cpp +++ b/yhttpd/src/tool/tool.cpp @@ -7,6 +7,7 @@ #include <sys/wait.h> #include <stdio.h> #include <sys/types.h> +#include <sys/stat.h> #include <fcntl.h> #include "tool.h" @@ -104,7 +105,7 @@ list<string> tool::split_string(string s_string, string s_split) { list<string> list_ret; - unsigned i_pos, i_len = s_split.length(); + size_t i_pos, i_len = s_split.length(); while ( (i_pos = s_string.find(s_split)) != string::npos ) { @@ -120,30 +121,19 @@ tool::split_string(string s_string, string s_split) string tool::trim( string s_str ) { - if( s_str.empty() ) - return s_str; - - char c_cur = s_str[0]; - int i_pos = 0; - - // left trim - while ( c_cur == ' '|| c_cur == '\n' || c_cur == '\r' ) - { - s_str.erase(i_pos,1); - c_cur = s_str[++i_pos]; - } - - // right trim - i_pos = s_str.size(); - c_cur = s_str[s_str.size()]; - - while ( c_cur == ' ' || c_cur == '\n' || c_cur == '\0' || c_cur == '\r' ) - { - s_str.erase(i_pos, 1); - c_cur = s_str[--i_pos]; - } - - return s_str; + // Left trim: find first non-whitespace char. + size_t b = 0; + while ( b < s_str.size() && + ( s_str[b] == ' ' || s_str[b] == '\n' || s_str[b] == '\r' ) ) + ++b; + + // Right trim: find last non-whitespace char. + size_t e = s_str.size(); + while ( e > b && + ( s_str[e-1] == ' ' || s_str[e-1] == '\n' || s_str[e-1] == '\r' ) ) + --e; + + return s_str.substr( b, e - b ); } char* @@ -160,7 +150,7 @@ tool::clean_char( char* c_str ) string tool::replace( string s_string, string s_search, string s_replace ) { - unsigned i_pos[2]; + size_t i_pos[2]; for ( i_pos[0] = s_string.find( s_search ); i_pos[0] != string::npos; @@ -201,37 +191,75 @@ tool::int2char( int i_int ) string tool::shell_command( string s_command, method m_method ) { - FILE *file; - char buf[READBUF]; - char *c_pos; - string s_ret = ""; + // Execute the CGI file directly via fork/execve - NOT through a shell. + // The old popen() ran `/bin/sh -c <s_command>`, so any shell metacharacter + // in the URL-derived request path was a command-injection / RCE vector + // when httpd.enablecgi=true. s_command must be the full path to an + // executable file (reqp's remove_dots already prevents ".." escaping the + // template dir, and stat() rejects non-files). + (void) m_method; // only METH_RETSTRING is used by the CGI path wrap::system_message(SHELLEX); wrap::system_message(s_command); - if( (file=popen(s_command.c_str(), "r")) == NULL ) + struct stat st; + if ( stat(s_command.c_str(), &st) != 0 || ! S_ISREG(st.st_mode) ) + { + wrap::system_message( SHELLER ); + return ""; + } + + int fd[2]; + if ( pipe(fd) != 0 ) + { + wrap::system_message( SHELLER ); + return ""; + } + + pid_t pid = fork(); + if ( pid < 0 ) { + close(fd[0]); close(fd[1]); wrap::system_message( SHELLER ); + return ""; } - else + + if ( pid == 0 ) + { + // child: stdout -> pipe, close inherited fds, exec the file (no shell). + close(fd[0]); + dup2(fd[1], STDOUT_FILENO); + close(fd[1]); + long l_maxfd = sysconf(_SC_OPEN_MAX); + if ( l_maxfd < 0 ) l_maxfd = 256; + for ( long i = STDERR_FILENO + 1; i < l_maxfd; ++i ) + close((int)i); + char* argv[] = { const_cast<char*>(s_command.c_str()), (char*) NULL }; + char* envp[] = { (char*) NULL }; + execve( s_command.c_str(), argv, envp ); + _exit(127); + } + + // parent: read the CGI output. + close(fd[1]); + string s_ret; + char buf[READBUF]; + for (;;) { - while(true) - { - if(fgets(buf, READBUF, file) == NULL) - break; - - switch (m_method) - { - case METH_NCURSES: - wrap::system_message( clean_char(buf) ); - break; - default: - s_ret.append("\n" + string(buf)); - } // switch - } - - pclose(file); + ssize_t n = read(fd[0], buf, sizeof(buf)); + if ( n > 0 ) + s_ret.append( buf, n ); + else if ( n == 0 ) + break; + else if ( errno == EINTR ) + continue; + else + break; } + close(fd[0]); + + int i_status; + waitpid( pid, &i_status, 0 ); return s_ret; } |
