Commit 588d9a40 authored by jan.koester's avatar jan.koester
Browse files

test

parent 5cfa21b6
Loading
Loading
Loading
Loading
+43 −10
Changes for src/http/HttpProxyServer.cpp: 43 added lines, 10 removed lines.
Original line number Diff line number Diff line
@@ -8,6 +8,7 @@
#include <iostream>
#include <set>
#include <stdexcept>
#include <thread>

#include <netplus/socket.h>
#include <netplus/crypto/cert_verify.h>
@@ -116,8 +117,26 @@ HttpProxyController::HttpProxyController(std::vector<netplus::socket *> serverSo
                                          http::UpstreamConnectionPool &connectionPool,
                                          tls::AcmeChallengeStore *acmeChallenges,
                                          int idleTimeoutSeconds)
    : libhttppp::HttpEvent(std::move(serverSockets), /*timeout=*/1000, /*h2OffloadThreads=*/0,
                            idleTimeoutSeconds)
    // h2OffloadThreads/h1OffloadThreads non-zero (see shouldOffloadH2Dispatch/
    // shouldOffloadH1Dispatch overrides below): every RequestEvent here does a real backend
    // round-trip (HttpClient::Get/Post/etc., possibly against a slow/overloaded upstream) and
    // would otherwise run synchronously on this listener's fixed, hardware_concurrency()-sized
    // worker pool -- shared across every domain/route on the same listener, not just the one
    // being proxied. Left at 0 (fully synchronous) this meant a handful of slow backend
    // requests (observed live: a crawler hammering a proxied git server's /users/sign_in and
    // deep commit/blame history) could tie up every worker thread at once, making completely
    // unrelated traffic on the same listener (e.g. a WebSocket upgrade to a different backend)
    // appear to hang -- see [[project_proxyplus_tls_hang_recurrence_20260816]]. H2 streams
    // always went through _h2DispatchPool; H1 requests (including a WebSocket upgrade's
    // blocking connect+handshake read) had no such option until _dispatchH1Request/
    // detachConnection+reattachConnection existed to make it safe for H1's one-request-per-
    // connection model. Both pools sized like h3StreamPool (libhttppp/src/httpd.cpp) and
    // quicDispatchPool() (libnetplus/src/quic.cpp): oversubscribed 2x hardware_concurrency()
    // since this work is dominated by waiting on the network, not CPU.
    : libhttppp::HttpEvent(std::move(serverSockets), /*timeout=*/1000,
                            /*h2OffloadThreads=*/(std::max<size_t>)(4, std::thread::hardware_concurrency() * 2),
                            idleTimeoutSeconds,
                            /*h1OffloadThreads=*/(std::max<size_t>)(4, std::thread::hardware_concurrency() * 2))
    , _listenerLabel(std::move(listenerLabel))
    , _tlsActive(tlsActive)
    , _configStore(configStore)
@@ -357,8 +376,12 @@ void HttpProxyController::RequestEvent(libhttppp::HttpRequest &curreq, const int
        if (method == "GET" && tryServeAcmeChallenge(curreq, url)) return;

        std::string requestHost;
        std::string rawHostHeader;
        if (auto *hostHdr = curreq.getHeaderData("host")) {
            if (auto *v = hostHdr->getfirstValue()) requestHost = normalizeHost(v->getvalue());
            if (auto *v = hostHdr->getfirstValue()) {
                rawHostHeader = v->getvalue();
                requestHost = normalizeHost(rawHostHeader);
            }
        }

        // Domain is a hard filter, not a tiebreak: a route whose domain doesn't match this
@@ -495,7 +518,16 @@ void HttpProxyController::RequestEvent(libhttppp::HttpRequest &curreq, const int
                    out->push_back(v->getvalue());
                }
            }
            outreq.setHeaderData("host")->push_back(upstream.getHost());
            // Forward the client's original Host header, not upstream.getHost() (the backend's
            // own address) -- matches nginx's actual default behavior (equivalent to
            // `proxy_set_header Host $host;`, which most HA-behind-reverse-proxy guides call
            // out explicitly) and what any virtual-hosted backend expects: Home Assistant in
            // particular validates the Host header it receives and uses it to build absolute
            // URLs, so handing it the upstream's raw host:port instead of the hostname the
            // client actually used breaks that. Falls back to upstream.getHost() only if the
            // client somehow sent no Host header at all (shouldn't happen for HTTP/1.1, but an
            // empty Host would be worse than the pre-existing behavior).
            outreq.setHeaderData("host")->push_back(!rawHostHeader.empty() ? rawHostHeader : upstream.getHost());
            // Deliberately no "Connection: close" here (unlike before pooling existed) --
            // forcing that on every request is exactly what defeated reuse; the upstream is now
            // allowed to keep the connection alive so UpstreamConnectionPool can hand it back
@@ -505,8 +537,10 @@ void HttpProxyController::RequestEvent(libhttppp::HttpRequest &curreq, const int

            // Standard reverse-proxy hints for the upstream: X-Forwarded-For chains (append to
            // whatever a prior proxy already put there, via the copy the loop above already
            // made); Host/Proto describe *this* hop only, so always set from what we actually
            // observed rather than trusting whatever value a client sent.
            // made); X-Real-IP is the single-address equivalent some backends check instead of
            // (or in addition to) X-Forwarded-For; Host/Proto describe *this* hop only, so
            // always set from what we actually observed rather than trusting whatever value a
            // client sent.
            std::string xff = sourceIp;
            if (auto *existingXff = outreq.getHeaderData("x-forwarded-for")) {
                if (auto *v = existingXff->getfirstValue()) {
@@ -514,10 +548,9 @@ void HttpProxyController::RequestEvent(libhttppp::HttpRequest &curreq, const int
                }
            }
            outreq.setHeaderData("x-forwarded-for")->push_back(xff);
            if (auto *hostHdr = curreq.getHeaderData("host")) {
                if (auto *v = hostHdr->getfirstValue()) {
                    outreq.setHeaderData("x-forwarded-host")->push_back(v->getvalue());
                }
            outreq.setHeaderData("x-real-ip")->push_back(sourceIp);
            if (!rawHostHeader.empty()) {
                outreq.setHeaderData("x-forwarded-host")->push_back(rawHostHeader);
            }
            outreq.setHeaderData("x-forwarded-proto")->push_back(_tlsActive ? "https" : "http");

+24 −0
Changes for src/http/HttpProxyServer.h: 24 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -42,6 +42,30 @@ public:

    void RequestEvent(libhttppp::HttpRequest &curreq, const int tid, ULONG_PTR args) override;

    // Always offload: RequestEvent's real work here (see the constructor's h2OffloadThreads
    // comment in HttpProxyServer.cpp) is a backend network round-trip for every route this
    // class ever forwards, so there's no synchronous-vs-offload split to make per-route --
    // unlike libhttppp::HttpEvent's own base default (which assumes most callers have a mix
    // of cheap and slow routes), a reverse proxy doesn't have a "cheap" H2 stream to keep off
    // the offload pool.
    bool shouldOffloadH2Dispatch(libhttppp::HttpRequest &tempreq, uint32_t streamId) const override {
        return true;
    }

    // Same rationale as shouldOffloadH2Dispatch() above, for HTTP/1.x: every route here does a
    // real backend round-trip, and a WebSocket upgrade's initial connect+handshake read (see
    // WebSocketProxy.cpp's handleWebSocketUpgrade) is itself dispatched through this same H1
    // RequestEvent() path -- without this, that blocking work ran directly on one of the
    // process's few shared epoll/kqueue worker threads, the same class of thread-starvation
    // bug tracked in [[project_proxyplus_tls_hang_recurrence_20260816]] (a burst of slow H1/H2
    // traffic on those threads made an unrelated WebSocket upgrade on the same listener appear
    // to hang). See httpd.h's shouldOffloadH1Dispatch/_dispatchH1Request doc comments for the
    // detach/reattach mechanics that make offloading a single-request-per-connection H1 socket
    // safe (unlike H2's per-stream tempreq trick).
    bool shouldOffloadH1Dispatch(libhttppp::HttpRequest &) const override {
        return true;
    }

private:
    struct AuthOutcome {
        bool authenticated = false;
+17 −2
Changes for src/http/WebSocketProxy.cpp: 17 added lines, 2 removed lines.
Original line number Diff line number Diff line
@@ -291,12 +291,26 @@ bool handleWebSocketUpgrade(libhttppp::HttpRequest &curreq,
            "host", "content-length", "transfer-encoding", "keep-alive",
            "proxy-authenticate", "proxy-authorization", "te", "trailer",
        };
        std::string rawHostHeader;
        if (auto *hostHdr = curreq.getHeaderData("host")) {
            if (auto *v = hostHdr->getfirstValue()) rawHostHeader = v->getvalue();
        }

        std::ostringstream req;
        req << "GET " << upstream.getPath() << " HTTP/1.1\r\n";
        // Forward the client's original Host header rather than the upstream's own
        // host:port -- same rationale as HttpProxyServer.cpp's normal proxying path (see its
        // comment): Home Assistant validates Host and uses it for absolute-URL generation, so
        // a raw backend address here breaks that. Falls back to the upstream address only if
        // the client sent no Host header at all.
        if (!rawHostHeader.empty()) {
            req << "Host: " << rawHostHeader;
        } else {
            req << "Host: " << upstream.getHost();
            if (upstream.getPort() != (upstream.getOriginalProtocol() == libhttppp::HttpUrl::HTTPS ? 443 : 80)) {
                req << ":" << upstream.getPort();
            }
        }
        req << "\r\n";
        for (auto *hd = curreq.getfirstHeaderData(); hd; hd = hd->nextHeaderData()) {
            const std::string &key = hd->getkey();
@@ -310,8 +324,9 @@ bool handleWebSocketUpgrade(libhttppp::HttpRequest &curreq,
            if (auto *v = hd->getfirstValue()) xff = v->getvalue() + ", " + sourceIp;
        }
        req << "X-Forwarded-For: " << xff << "\r\n";
        if (auto *hostHdr = curreq.getHeaderData("host")) {
            if (auto *v = hostHdr->getfirstValue()) req << "X-Forwarded-Host: " << v->getvalue() << "\r\n";
        req << "X-Real-IP: " << sourceIp << "\r\n";
        if (!rawHostHeader.empty()) {
            req << "X-Forwarded-Host: " << rawHostHeader << "\r\n";
        }
        req << "\r\n";