Loading src/http.cpp +53 −12 Changes for src/http.cpp: 53 added lines, 12 removed lines. Original line number Diff line number Diff line Loading @@ -2015,7 +2015,9 @@ size_t libhttppp::HttpClient::_streamReadH3(char *buf, size_t bufsize, bool bloc // Unlike the non-blocking variant, this must not return 0 when data // is simply not available yet -- that would cause the caller to // think the stream is done. auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(60); // Honor setTimeout() (_recvTimeoutSec) instead of a fixed 60s, same // as every other blocking recv path in this file. auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(_recvTimeoutSec); while (_streamH3BodyPos >= _streamH3Body.size()) { bool got_any = false; Loading Loading @@ -2454,7 +2456,9 @@ const std::vector<char> libhttppp::HttpClient::_h2Request( try { // Helper: blocking recv using socketwait::waitRead() auto recv_deadline = std::chrono::steady_clock::now() + std::chrono::seconds(60); // Honor setTimeout() (_recvTimeoutSec) instead of a fixed 60s, same // as every other blocking recv path in this file. auto recv_deadline = std::chrono::steady_clock::now() + std::chrono::seconds(_recvTimeoutSec); auto recv_blocking = [&](netplus::buffer& b) -> size_t { // b is _recvScratch; recvData() shrinks b.size to bytes read, so // reset before every attempt (see _recvBlocking() for why). Loading Loading @@ -2924,8 +2928,15 @@ const std::vector<char> libhttppp::HttpClient::_h2Request( uint64_t data_remaining = 0; size_t raw_off = 0; // consumed offset into raw (avoids erase-from-front) auto last_data = std::chrono::steady_clock::now(); auto last_activity = last_data; // request_start never advances (an overall deadline for the whole // response, however much intermittent trickle activity there is); // last_activity resets on any received bytes (a silence-gap deadline). // These used to be the same variable pair, both reset together on // every trickle of data -- since idle_timeout < response_timeout, // that made the idle_timeout check always fire first, so the // response_timeout branch below was dead code. const auto request_start = std::chrono::steady_clock::now(); auto last_activity = request_start; const auto idle_timeout = std::chrono::seconds(15); const auto response_timeout = std::chrono::seconds(60); Loading @@ -2940,9 +2951,7 @@ const std::vector<char> libhttppp::HttpClient::_h2Request( got_any = true; } if (got_any) { auto now = std::chrono::steady_clock::now(); last_data = now; last_activity = now; last_activity = std::chrono::steady_clock::now(); } return got_any; }; Loading Loading @@ -3072,7 +3081,7 @@ const std::vector<char> libhttppp::HttpClient::_h2Request( ee[HTTPException::Error] << "HTTP/3 connection idle timeout"; throw ee; } if (now - last_data > response_timeout) { if (now - request_start > response_timeout) { HTTPException ee; ee[HTTPException::Error] << "HTTP/3 response timeout"; throw ee; Loading Loading @@ -3158,8 +3167,29 @@ const std::vector<char> libhttppp::HttpClient::_doH1Request( ++redirects; // Update request URL for relative or absolute redirect if (_lastLocation.find("://") != std::string::npos) { _url = HttpUrl(_lastLocation); HttpUrl newUrl(_lastLocation); // A same-origin absolute redirect (rare but legal) must // not be treated as cross-origin -- only strip/replace // Host and credentials when the destination actually // differs, so a same-origin redirect keeps working // exactly as before. const bool crossOrigin = newUrl.getHost() != _url.getHost() || newUrl.getPort() != _url.getPort(); _url = newUrl; resetConnection(); if (crossOrigin) { // The Host header set for the original origin (and // any credential-bearing header the caller put on // nreq before calling Get()/Post()/etc., e.g. // Authorization or Cookie) must not silently follow // a redirect to a different origin. Host gets // re-added below (top-of-loop) for the new _url; // deleting it here is what makes that re-add fire // instead of being skipped as "already set". nreq.deldata("host"); nreq.deldata("authorization"); nreq.deldata("cookie"); } } nreq.setRequestURL(_lastLocation.find("://") != std::string::npos ? _url.getPath() : _lastLocation); Loading Loading @@ -3556,6 +3586,15 @@ void libhttppp::HttpResponse::setState(const std ::string &httpstate){ } void libhttppp::HttpResponse::setContentLength(size_t len){ // _ContentLength can be null here: sendChunkedHeaders() explicitly clears // it (there's no Content-Length on a chunked response) without forbidding // further use of this HttpResponse object (e.g. an error-handling path // that falls back to a normal, non-chunked response on the same object). if(!_ContentLength) _ContentLength=setHeaderData("content-length"); if(_ContentLength->empty()) _ContentLength->push_back(len); else _ContentLength->at(0)=len; } Loading Loading @@ -4794,9 +4833,11 @@ bool libhttppp::HttpRequest::isMobile(){ } void libhttppp::HttpRequest::setRequestData(const std::string &data, size_t len){ _cachedRequest.clear(); _cachedRequest.resize(len); _cachedRequest.insert(0,data); // Previously: clear() + resize(len) (fills with `len` NUL bytes) then // insert(0,data) (inserts *all* of data before those NUL bytes) -- that // produced data + string(len,'\0') instead of the first `len` bytes of // data, corrupting the request line with trailing NUL bytes. _cachedRequest.assign(data, 0, std::min(len, data.size())); _cachedRequestURL = extractPath(_cachedRequest); // Update :path pseudo-header in _firstHeaderData auto *hPath = setHeaderData(":path"); Loading src/httpd.cpp +79 −8 Changes for src/httpd.cpp: 79 added lines, 8 removed lines. Original line number Diff line number Diff line Loading @@ -68,6 +68,17 @@ static constexpr uint8_t H2_FLAG_PRIORITY = 0x20; static constexpr size_t H2_FRAME_HEADER_LEN = 9; static constexpr size_t H2_MAX_FRAME_SIZE = 16384; // A HEADERS frame without END_HEADERS defers decoding and just accumulates // raw HPACK bytes across as many CONTINUATION frames as the peer cares to // send (RFC 7540 §6.10), with nothing capping either the total bytes or the // frame count -- a "CONTINUATION flood": a client can hold one HEADERS // frame open forever and stream unlimited CONTINUATION frames, growing // H2PendingIncoming::rawHpack without bound. This is deliberately generous // (real header blocks compress well below this even for large cookie/auth // headers) rather than tight, since legitimate large-but-finite header // blocks must still work. static constexpr size_t H2_MAX_PENDING_HPACK_SIZE = 256 * 1024; // RFC 7540 §6.9.1: a flow-control window must never exceed 2^31-1. static constexpr int64_t H2_MAX_WINDOW_SIZE = 0x7fffffff; Loading Loading @@ -899,6 +910,11 @@ void libhttppp::HttpEvent::_finishH2Dispatch(HttpRequest &cureq, uint8_t rst[4] = {0, 0, 0, 0x08}; // CANCEL out += h2BuildFrame(H2_FRAME_RST_STREAM, 0, sid, std::string(reinterpret_cast<char*>(rst), 4)); // Without this, peerStreamWindows keeps one entry for // sid per rejected duplicate dispatch forever — this // stream id is never going to get its own WINDOW_UPDATE // credit consumed now that it's been rejected. cureq.h2state().peerStreamWindows.erase(sid); return; } // Store for resumption on WINDOW_UPDATE Loading Loading @@ -976,7 +992,21 @@ void libhttppp::HttpEvent::_finishH2Dispatch(HttpRequest &cureq, boff += chunk_sz; } if (boff < body.size()) { // Window exhausted — park remainder for WINDOW_UPDATE // Window exhausted — park remainder for WINDOW_UPDATE. // Same duplicate-stream hazard as the streaming-response // branch above: overwriting activeStreams[sid] via // operator[] while an entry already exists for sid would // destroy the existing H2StreamingResponse/tempreq while // ResponseEvent may still reference it. The HEADERS/partial // DATA above are already queued in `out` by this point, so // this can't cleanly reject the whole response the way the // sibling guard does -- but refusing to clobber the // existing entry still avoids the dangling-reference risk. if (cureq.h2state().activeStreams.count(sid) > 0) { std::cerr << "[H2-DISPATCH] WARNING: sid=" << sid << " already has an active stream, skipping" << std::endl; } else { auto ss = std::make_shared<HttpRequest::H2StreamingResponse>(); ss->streamId = sid; ss->contentLength = body.size(); Loading @@ -987,6 +1017,7 @@ void libhttppp::HttpEvent::_finishH2Dispatch(HttpRequest &cureq, ss->tid = tid; ss->args = args; cureq.h2state().activeStreams[sid] = ss; } } else { cureq.h2state().peerStreamWindows.erase(sid); } Loading Loading @@ -1456,6 +1487,14 @@ reprocess: const uint8_t *cont_data = reinterpret_cast<const uint8_t*>(data + off); it->second.rawHpack.insert(it->second.rawHpack.end(), cont_data, cont_data + flen); if (it->second.rawHpack.size() > H2_MAX_PENDING_HPACK_SIZE) { // CONTINUATION flood guard -- see H2_MAX_PENDING_HPACK_SIZE. uint8_t rst[4] = {0, 0, 0, 0x0b}; // ENHANCE_YOUR_CALM out += h2BuildFrame(H2_FRAME_RST_STREAM, 0, sid, std::string(reinterpret_cast<char*>(rst), 4)); cureq.h2state().pendingIncoming.erase(it); break; } if (fflags & H2_FLAG_END_HEADERS) { // Full HPACK block received — decode now it->second.headers = cureq.h2state().hpackDecoder.decode( Loading Loading @@ -1507,6 +1546,23 @@ reprocess: } else { it->second.body.append(data + off, flen); // The WINDOW_UPDATE above was already sent unconditionally, // so the client can keep sending regardless of this check -- // but without this cap, a non-streaming (buffered) upload // that never sets END_STREAM would grow `body` in memory // without bound: flow control alone provides no real // backpressure here since the window is refilled as fast // as it's spent. Reuse the same per-request upload-size // limit HttpRequest already exposes (getMaxUploadSize(), // default DEFAULT_UPLOADSIZE) rather than a separate cap. if (it->second.body.size() > cureq.getMaxUploadSize()) { uint8_t rst[4] = {0, 0, 0, 0x0b}; // ENHANCE_YOUR_CALM out += h2BuildFrame(H2_FRAME_RST_STREAM, 0, sid, std::string(reinterpret_cast<char*>(rst), 4)); cureq.h2state().pendingIncoming.erase(it); break; } if (endStream) { if (it->second.headersComplete) { off += flen; Loading Loading @@ -1989,14 +2045,20 @@ void libhttppp::HttpEvent::Http3StreamEvent(netplus::socket *sock, empty_streak = 0; } else { q->pumpIncoming(); if (tr->slots[0].ResponsePending.exchange(false)) { // Upstream signalled more data is coming // — retry immediately without sleeping. empty_streak = 0; continue; } else { // `tr` is a synthetic per-stream request, not a real // connection tracked by the event loop -- nothing // external ever sets its ResponsePending. The // app-level ResponseEvent just called above (e.g. // mediadb's/blogi's streaming handlers) sets it on // *every* empty read as a "rearm me" signal meant // for a real connection; reading it back here as if // it meant "new data arrived" was self-referential // (same bug already fixed for the H2 path above — // see the NOTE there) and reset empty_streak every // time, so a permanently-stalled upstream spun this // pool thread forever instead of ever hitting // max_empty. ++empty_streak; } if (empty_streak >= max_empty) { break; } Loading Loading @@ -2266,6 +2328,15 @@ void libhttppp::HttpEvent::ResponseEvent(netplus::con &curcon,const int tid,ULON std::string out; out.reserve(4096); _resumeH2Streams(cureq, out, tid, args); // _reapStalledH2Streams()'s only other call site is // Http2RequestEvent, which only runs when a new frame // arrives from the peer. A peer that stops crediting window // *and* stops sending any other frame at all never triggers // that path again, so the 30s stall it's meant to catch // would otherwise never be detected on a connection driven // purely by writable/poll events (e.g. one already parked // here waiting on window credit). _reapStalledH2Streams(cureq, out); if (!out.empty()) { cureq.SendData.append(out.data(), out.size()); } Loading Loading
src/http.cpp +53 −12 Changes for src/http.cpp: 53 added lines, 12 removed lines. Original line number Diff line number Diff line Loading @@ -2015,7 +2015,9 @@ size_t libhttppp::HttpClient::_streamReadH3(char *buf, size_t bufsize, bool bloc // Unlike the non-blocking variant, this must not return 0 when data // is simply not available yet -- that would cause the caller to // think the stream is done. auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(60); // Honor setTimeout() (_recvTimeoutSec) instead of a fixed 60s, same // as every other blocking recv path in this file. auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(_recvTimeoutSec); while (_streamH3BodyPos >= _streamH3Body.size()) { bool got_any = false; Loading Loading @@ -2454,7 +2456,9 @@ const std::vector<char> libhttppp::HttpClient::_h2Request( try { // Helper: blocking recv using socketwait::waitRead() auto recv_deadline = std::chrono::steady_clock::now() + std::chrono::seconds(60); // Honor setTimeout() (_recvTimeoutSec) instead of a fixed 60s, same // as every other blocking recv path in this file. auto recv_deadline = std::chrono::steady_clock::now() + std::chrono::seconds(_recvTimeoutSec); auto recv_blocking = [&](netplus::buffer& b) -> size_t { // b is _recvScratch; recvData() shrinks b.size to bytes read, so // reset before every attempt (see _recvBlocking() for why). Loading Loading @@ -2924,8 +2928,15 @@ const std::vector<char> libhttppp::HttpClient::_h2Request( uint64_t data_remaining = 0; size_t raw_off = 0; // consumed offset into raw (avoids erase-from-front) auto last_data = std::chrono::steady_clock::now(); auto last_activity = last_data; // request_start never advances (an overall deadline for the whole // response, however much intermittent trickle activity there is); // last_activity resets on any received bytes (a silence-gap deadline). // These used to be the same variable pair, both reset together on // every trickle of data -- since idle_timeout < response_timeout, // that made the idle_timeout check always fire first, so the // response_timeout branch below was dead code. const auto request_start = std::chrono::steady_clock::now(); auto last_activity = request_start; const auto idle_timeout = std::chrono::seconds(15); const auto response_timeout = std::chrono::seconds(60); Loading @@ -2940,9 +2951,7 @@ const std::vector<char> libhttppp::HttpClient::_h2Request( got_any = true; } if (got_any) { auto now = std::chrono::steady_clock::now(); last_data = now; last_activity = now; last_activity = std::chrono::steady_clock::now(); } return got_any; }; Loading Loading @@ -3072,7 +3081,7 @@ const std::vector<char> libhttppp::HttpClient::_h2Request( ee[HTTPException::Error] << "HTTP/3 connection idle timeout"; throw ee; } if (now - last_data > response_timeout) { if (now - request_start > response_timeout) { HTTPException ee; ee[HTTPException::Error] << "HTTP/3 response timeout"; throw ee; Loading Loading @@ -3158,8 +3167,29 @@ const std::vector<char> libhttppp::HttpClient::_doH1Request( ++redirects; // Update request URL for relative or absolute redirect if (_lastLocation.find("://") != std::string::npos) { _url = HttpUrl(_lastLocation); HttpUrl newUrl(_lastLocation); // A same-origin absolute redirect (rare but legal) must // not be treated as cross-origin -- only strip/replace // Host and credentials when the destination actually // differs, so a same-origin redirect keeps working // exactly as before. const bool crossOrigin = newUrl.getHost() != _url.getHost() || newUrl.getPort() != _url.getPort(); _url = newUrl; resetConnection(); if (crossOrigin) { // The Host header set for the original origin (and // any credential-bearing header the caller put on // nreq before calling Get()/Post()/etc., e.g. // Authorization or Cookie) must not silently follow // a redirect to a different origin. Host gets // re-added below (top-of-loop) for the new _url; // deleting it here is what makes that re-add fire // instead of being skipped as "already set". nreq.deldata("host"); nreq.deldata("authorization"); nreq.deldata("cookie"); } } nreq.setRequestURL(_lastLocation.find("://") != std::string::npos ? _url.getPath() : _lastLocation); Loading Loading @@ -3556,6 +3586,15 @@ void libhttppp::HttpResponse::setState(const std ::string &httpstate){ } void libhttppp::HttpResponse::setContentLength(size_t len){ // _ContentLength can be null here: sendChunkedHeaders() explicitly clears // it (there's no Content-Length on a chunked response) without forbidding // further use of this HttpResponse object (e.g. an error-handling path // that falls back to a normal, non-chunked response on the same object). if(!_ContentLength) _ContentLength=setHeaderData("content-length"); if(_ContentLength->empty()) _ContentLength->push_back(len); else _ContentLength->at(0)=len; } Loading Loading @@ -4794,9 +4833,11 @@ bool libhttppp::HttpRequest::isMobile(){ } void libhttppp::HttpRequest::setRequestData(const std::string &data, size_t len){ _cachedRequest.clear(); _cachedRequest.resize(len); _cachedRequest.insert(0,data); // Previously: clear() + resize(len) (fills with `len` NUL bytes) then // insert(0,data) (inserts *all* of data before those NUL bytes) -- that // produced data + string(len,'\0') instead of the first `len` bytes of // data, corrupting the request line with trailing NUL bytes. _cachedRequest.assign(data, 0, std::min(len, data.size())); _cachedRequestURL = extractPath(_cachedRequest); // Update :path pseudo-header in _firstHeaderData auto *hPath = setHeaderData(":path"); Loading
src/httpd.cpp +79 −8 Changes for src/httpd.cpp: 79 added lines, 8 removed lines. Original line number Diff line number Diff line Loading @@ -68,6 +68,17 @@ static constexpr uint8_t H2_FLAG_PRIORITY = 0x20; static constexpr size_t H2_FRAME_HEADER_LEN = 9; static constexpr size_t H2_MAX_FRAME_SIZE = 16384; // A HEADERS frame without END_HEADERS defers decoding and just accumulates // raw HPACK bytes across as many CONTINUATION frames as the peer cares to // send (RFC 7540 §6.10), with nothing capping either the total bytes or the // frame count -- a "CONTINUATION flood": a client can hold one HEADERS // frame open forever and stream unlimited CONTINUATION frames, growing // H2PendingIncoming::rawHpack without bound. This is deliberately generous // (real header blocks compress well below this even for large cookie/auth // headers) rather than tight, since legitimate large-but-finite header // blocks must still work. static constexpr size_t H2_MAX_PENDING_HPACK_SIZE = 256 * 1024; // RFC 7540 §6.9.1: a flow-control window must never exceed 2^31-1. static constexpr int64_t H2_MAX_WINDOW_SIZE = 0x7fffffff; Loading Loading @@ -899,6 +910,11 @@ void libhttppp::HttpEvent::_finishH2Dispatch(HttpRequest &cureq, uint8_t rst[4] = {0, 0, 0, 0x08}; // CANCEL out += h2BuildFrame(H2_FRAME_RST_STREAM, 0, sid, std::string(reinterpret_cast<char*>(rst), 4)); // Without this, peerStreamWindows keeps one entry for // sid per rejected duplicate dispatch forever — this // stream id is never going to get its own WINDOW_UPDATE // credit consumed now that it's been rejected. cureq.h2state().peerStreamWindows.erase(sid); return; } // Store for resumption on WINDOW_UPDATE Loading Loading @@ -976,7 +992,21 @@ void libhttppp::HttpEvent::_finishH2Dispatch(HttpRequest &cureq, boff += chunk_sz; } if (boff < body.size()) { // Window exhausted — park remainder for WINDOW_UPDATE // Window exhausted — park remainder for WINDOW_UPDATE. // Same duplicate-stream hazard as the streaming-response // branch above: overwriting activeStreams[sid] via // operator[] while an entry already exists for sid would // destroy the existing H2StreamingResponse/tempreq while // ResponseEvent may still reference it. The HEADERS/partial // DATA above are already queued in `out` by this point, so // this can't cleanly reject the whole response the way the // sibling guard does -- but refusing to clobber the // existing entry still avoids the dangling-reference risk. if (cureq.h2state().activeStreams.count(sid) > 0) { std::cerr << "[H2-DISPATCH] WARNING: sid=" << sid << " already has an active stream, skipping" << std::endl; } else { auto ss = std::make_shared<HttpRequest::H2StreamingResponse>(); ss->streamId = sid; ss->contentLength = body.size(); Loading @@ -987,6 +1017,7 @@ void libhttppp::HttpEvent::_finishH2Dispatch(HttpRequest &cureq, ss->tid = tid; ss->args = args; cureq.h2state().activeStreams[sid] = ss; } } else { cureq.h2state().peerStreamWindows.erase(sid); } Loading Loading @@ -1456,6 +1487,14 @@ reprocess: const uint8_t *cont_data = reinterpret_cast<const uint8_t*>(data + off); it->second.rawHpack.insert(it->second.rawHpack.end(), cont_data, cont_data + flen); if (it->second.rawHpack.size() > H2_MAX_PENDING_HPACK_SIZE) { // CONTINUATION flood guard -- see H2_MAX_PENDING_HPACK_SIZE. uint8_t rst[4] = {0, 0, 0, 0x0b}; // ENHANCE_YOUR_CALM out += h2BuildFrame(H2_FRAME_RST_STREAM, 0, sid, std::string(reinterpret_cast<char*>(rst), 4)); cureq.h2state().pendingIncoming.erase(it); break; } if (fflags & H2_FLAG_END_HEADERS) { // Full HPACK block received — decode now it->second.headers = cureq.h2state().hpackDecoder.decode( Loading Loading @@ -1507,6 +1546,23 @@ reprocess: } else { it->second.body.append(data + off, flen); // The WINDOW_UPDATE above was already sent unconditionally, // so the client can keep sending regardless of this check -- // but without this cap, a non-streaming (buffered) upload // that never sets END_STREAM would grow `body` in memory // without bound: flow control alone provides no real // backpressure here since the window is refilled as fast // as it's spent. Reuse the same per-request upload-size // limit HttpRequest already exposes (getMaxUploadSize(), // default DEFAULT_UPLOADSIZE) rather than a separate cap. if (it->second.body.size() > cureq.getMaxUploadSize()) { uint8_t rst[4] = {0, 0, 0, 0x0b}; // ENHANCE_YOUR_CALM out += h2BuildFrame(H2_FRAME_RST_STREAM, 0, sid, std::string(reinterpret_cast<char*>(rst), 4)); cureq.h2state().pendingIncoming.erase(it); break; } if (endStream) { if (it->second.headersComplete) { off += flen; Loading Loading @@ -1989,14 +2045,20 @@ void libhttppp::HttpEvent::Http3StreamEvent(netplus::socket *sock, empty_streak = 0; } else { q->pumpIncoming(); if (tr->slots[0].ResponsePending.exchange(false)) { // Upstream signalled more data is coming // — retry immediately without sleeping. empty_streak = 0; continue; } else { // `tr` is a synthetic per-stream request, not a real // connection tracked by the event loop -- nothing // external ever sets its ResponsePending. The // app-level ResponseEvent just called above (e.g. // mediadb's/blogi's streaming handlers) sets it on // *every* empty read as a "rearm me" signal meant // for a real connection; reading it back here as if // it meant "new data arrived" was self-referential // (same bug already fixed for the H2 path above — // see the NOTE there) and reset empty_streak every // time, so a permanently-stalled upstream spun this // pool thread forever instead of ever hitting // max_empty. ++empty_streak; } if (empty_streak >= max_empty) { break; } Loading Loading @@ -2266,6 +2328,15 @@ void libhttppp::HttpEvent::ResponseEvent(netplus::con &curcon,const int tid,ULON std::string out; out.reserve(4096); _resumeH2Streams(cureq, out, tid, args); // _reapStalledH2Streams()'s only other call site is // Http2RequestEvent, which only runs when a new frame // arrives from the peer. A peer that stops crediting window // *and* stops sending any other frame at all never triggers // that path again, so the 30s stall it's meant to catch // would otherwise never be detected on a connection driven // purely by writable/poll events (e.g. one already parked // here waiting on window credit). _reapStalledH2Streams(cureq, out); if (!out.empty()) { cureq.SendData.append(out.data(), out.size()); } Loading