diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index bd5f63f..834465e 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -104,10 +104,7 @@ jobs: LSAN_OPTIONS: suppressions=lsan_suppressions.txt - name: build and run tests (Mbed TLS) if: matrix.tls_backend == 'mbedtls' - # Run mbedTLS shards with reduced parallelism — under ASAN+mbedTLS the - # default 4 shards overload CI runners enough that timing-sensitive - # ServerTest cases flake on first-request keep-alive reuse. - run: cd test && make test_split_mbedtls && SHARDS=2 make test_mbedtls_parallel + run: cd test && make test_split_mbedtls && make test_mbedtls_parallel - name: build and run tests (wolfSSL) if: matrix.tls_backend == 'wolfssl' run: cd test && make test_split_wolfssl && make test_wolfssl_parallel @@ -142,10 +139,7 @@ jobs: - name: install Mbed TLS run: sudo apt-get install -y libmbedtls-dev - name: build and run tests (Mbed TLS) - # Run mbedTLS shards with reduced parallelism — under ASAN+mbedTLS the - # default 4 shards overload CI runners enough that timing-sensitive - # ServerTest cases flake on first-request keep-alive reuse. - run: cd test && make test_split_mbedtls && SHARDS=2 make test_mbedtls_parallel + run: cd test && make test_split_mbedtls && make test_mbedtls_parallel # BoringSSL is Google's fork of OpenSSL. It has no API stability guarantee # and is not packaged by distros, so we build it from source. cpp-httplib @@ -410,10 +404,7 @@ jobs: LSAN_OPTIONS: suppressions=lsan_suppressions.txt - name: build and run tests (Mbed TLS) if: matrix.tls_backend == 'mbedtls' - # macOS runners under ASAN+mbedTLS still flake at SHARDS=2 (rapid - # bind/connect on the fixture's fixed port races on the slower - # macos-latest runner). Serialize fully here; ubuntu stays at 2. - run: cd test && make test_split_mbedtls && SHARDS=1 make test_mbedtls_parallel + run: cd test && make test_split_mbedtls && make test_mbedtls_parallel - name: build and run tests (wolfSSL) if: matrix.tls_backend == 'wolfssl' run: cd test && make test_split_wolfssl && make test_wolfssl_parallel diff --git a/httplib.h b/httplib.h index 9375bdb..1b07788 100644 --- a/httplib.h +++ b/httplib.h @@ -18038,6 +18038,13 @@ struct MbedTlsSession { std::string hostname; // For client: set via set_sni std::string sni_hostname; // For server: received from client via SNI callback + // Mbed TLS has no SSL_peek() equivalent, so is_peer_closed() must probe with + // a real 1-byte mbedtls_ssl_read(). If that probe lands on application data + // (e.g. a response that arrived while this side was still in its post-write + // check), the byte is pushed back here and served by the next read(). + unsigned char peeked_byte = 0; + bool has_peeked_byte = false; + MbedTlsSession() { mbedtls_ssl_init(&ssl); } ~MbedTlsSession() { mbedtls_ssl_free(&ssl); } @@ -18743,6 +18750,23 @@ inline ssize_t read(session_t session, void *buf, size_t len, TlsError &err) { } auto msession = static_cast(session); + + // Serve a byte consumed by the is_peer_closed() probe before reading more. + if (msession->has_peeked_byte) { + if (len == 0) { return 0; } + auto p = static_cast(buf); + p[0] = msession->peeked_byte; + msession->has_peeked_byte = false; + size_t n = 1; + // Top up with any already-decrypted bytes without risking a block. + if (len > 1 && mbedtls_ssl_get_bytes_avail(&msession->ssl) > 0) { + int extra = mbedtls_ssl_read(&msession->ssl, p + 1, len - 1); + if (extra > 0) { n += static_cast(extra); } + } + err.code = ErrorCode::Success; + return static_cast(n); + } + int ret; do { ret = mbedtls_ssl_read(&msession->ssl, static_cast(buf), @@ -18802,7 +18826,8 @@ inline int pending(const_session_t session) { if (!session) { return 0; } auto msession = static_cast(const_cast(session)); - return static_cast(mbedtls_ssl_get_bytes_avail(&msession->ssl)); + return static_cast(mbedtls_ssl_get_bytes_avail(&msession->ssl)) + + (msession->has_peeked_byte ? 1 : 0); } inline void shutdown(session_t session, bool graceful) { @@ -18828,19 +18853,21 @@ inline bool is_peer_closed(session_t session, socket_t sock) { if (!session || sock == INVALID_SOCKET) { return true; } auto msession = static_cast(session); - // Check if there's already decrypted data available in the TLS buffer - // If so, the connection is definitely alive - if (mbedtls_ssl_get_bytes_avail(&msession->ssl) > 0) { return false; } + // Check if there's already decrypted or pushed-back data available. + // If so, the connection is definitely alive. + if (msession->has_peeked_byte || + mbedtls_ssl_get_bytes_avail(&msession->ssl) > 0) { + return false; + } // Set socket to non-blocking to avoid blocking on read detail::set_nonblocking(sock, true); auto cleanup = detail::scope_exit([&]() { detail::set_nonblocking(sock, false); }); - // Try a 1-byte read to check connection status - // Note: This will consume the byte if data is available, but for the - // purpose of checking if peer is closed, this should be acceptable - // since we're only called when we expect the connection might be closing + // Probe with a 1-byte read (Mbed TLS has no peek API). If the probe lands + // on application data — e.g. a response that already arrived — push the + // byte back so the next read() delivers it instead of losing it. unsigned char buf; int ret; do { @@ -18848,7 +18875,12 @@ inline bool is_peer_closed(session_t session, socket_t sock) { } while (impl::mbedtls_is_session_ticket(ret)); // If we got data or WANT_READ (would block), connection is alive - if (ret > 0 || ret == MBEDTLS_ERR_SSL_WANT_READ) { return false; } + if (ret > 0) { + msession->peeked_byte = buf; + msession->has_peeked_byte = true; + return false; + } + if (ret == MBEDTLS_ERR_SSL_WANT_READ) { return false; } // If we get a peer close notify or a connection reset, the peer is closed return ret == MBEDTLS_ERR_SSL_PEER_CLOSE_NOTIFY || diff --git a/test/test.cc b/test/test.cc index 479d9f8..651fb38 100644 --- a/test/test.cc +++ b/test/test.cc @@ -20893,9 +20893,13 @@ TEST(ProxyTunnelTest, OriginReturning407InsideTunnelDoesNotLeakProxyDigest) { proxy_tunnel_test::ScopedConnectProxy proxy(origin.port()); ASSERT_NE(0, proxy.port()); - SSLClient cli(HOST, origin.port()); + // Pin to 127.0.0.1: the proxy listens on 127.0.0.1 only, while "localhost" + // may resolve to ::1 first. A concurrent test (e.g. another gtest shard) + // holding ::1 with the same ephemeral port number would hijack the CONNECT + // and answer with its own status, making this test flaky. + SSLClient cli("127.0.0.1", origin.port()); cli.enable_server_certificate_verification(false); - cli.set_proxy(HOST, proxy.port()); + cli.set_proxy("127.0.0.1", proxy.port()); cli.set_proxy_digest_auth("proxy-user", "proxy-pass"); auto res = cli.Get("/x");