fix(http): fix 7 security/correctness bugs + refactor HTTP layer#21
Merged
Conversation
…ayer Security fixes: - Centralize path traversal guard in MethodHandler::handle() — covers GET, POST, DELETE and CGI before any dispatch - Add urlDecode() to detect encoded variants (%2e%2e, %2f, etc.) Correctness fixes: - Return 404 when no location matches (was 405) - Enforce client_max_body_size → 413 Payload Too Large - Fix PATH_INFO to strip query string before passing to CGI - Replace single write() with loop for CGI stdin (prevents body truncation) - Handle WIFSIGNALED in CGI exit status check - Remove second select() in CGI timeout — replaced with O_NONBLOCK + time() to keep exactly one poll() multiplexer in the codebase Refactoring: - Rename all short variables (req→request, loc→location, fd→fileDescriptor, st→fileInfo, ss→contentLength, n→bytesRead, etc.) across all 9 HTTP files - Extract intermediate boolean variables to document intent - Remove scattered hasPathTraversal() copies from PostHandler/DeleteHandler Tests: - Restructure tests/http/ into unit/ and integration/ subdirectories - Add test_adversarial.cpp: 48 tests covering parsing attacks, path traversal (including encoded variants), method attacks, router edge cases, full pipeline - Add test_cgi_advanced.cpp: 58 tests covering env vars, POST body, query strings, custom status codes, large output (100KB), error handling (404/500/ 403/stderr/timeout), full pipeline eval-style scenarios - Add 10 advanced Python CGI scripts for integration testing Co-authored-by: Cursor <cursoragent@cursor.com>
std::string(const char*) stops at the first \0, so raw.find('\0')
returns npos and raw[npos] is an out-of-bounds write. On macOS/clang
this passes silently; on Ubuntu/glibc it triggers 'double free or
corruption' and aborts.
Fix: use the length-based std::string(buf, len) constructor so the
embedded \0 is actually included in the string.
Co-authored-by: Cursor <cursoragent@cursor.com>
MethodHandler.cpp : isMethodAllowed + buildError avant handle() RequestParser.cpp : isValid + parseFirstLine + parseHeaders + parseBody avant parse() output.cpp : buildError avant parseOutput() Co-authored-by: Cursor <cursoragent@cursor.com>
…rror MethodHandler::buildError et CgiHandler::buildError \u00e9taient identiques. Extrait en fonction libre buildHttpError() dans HttpUtils.hpp/.cpp. Supprim\u00e9 des deux classes, tous les appels mis \u00e0 jour. Co-authored-by: Cursor <cursoragent@cursor.com>
StringUtils.hpp/.cpp : urlDecode, hasPathTraversal, extractQueryString, extractFilename HttpUtils.hpp/.cpp : buildHttpError, getContentType Supprime les fonctions statiques dupliquees dans chaque handler/cgi. Ancien HttpUtils a la racine remplace par include/http/utils/ et src/http/utils/. Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
.cppdu layer (nommage verbeux, lisibilité)unit/etintegration/+ 106 nouveaux testsBugs corrigés
root + urisans validation)client_max_body_sizeignoré — jamais de 413%2e%2e) non décodéPATH_INFOincluait la query stringwrite()CGI stdin sans boucleWIFSIGNALEDnon vérifié aprèswaitpid()Violation hard constraint résolue
Le
select()dansexecute.cpp(second multiplexeur I/O) a été remplacé parO_NONBLOCK+time()— la codebase a maintenant exactement un seul appel poll/select/epoll/kqueue (dans le futur core server).Tests
tests/http/unit/— 67 tests (RequestParser, ResponseBuilder, Router) — 0 I/O disquetests/http/integration/— 197 tests (GET/POST/DELETE handlers, CGI basique, CGI avancé, adversarial)Checklist
select()/poll()/epoll()/kqueue()danssrc/fork()uniquement danscgi/execute.cppfcntl()uniquement avecF_SETFL+O_NONBLOCKerrnoaprèsread/write/recv/sendauto,nullptr, lambdas, range-for-Wall -Wextra -Werror -std=c++98sans warningMade with Cursor