From d1fa9b3eb7adb6a7986b916d0e287bfd833a8a6d Mon Sep 17 00:00:00 2001 From: Edmond <1571649+EdmondDantes@users.noreply.github.com> Date: Wed, 26 Aug 2026 10:06:46 +0300 Subject: [PATCH 1/2] test(windows): assert what the server did, not what the client said (#307) 060 named two output lines while printing one per built encoding, so a build with brotli and no zstd failed for something the server got right. It now reports a verdict that does not depend on how many encodings the build has. chaos/002 drove H2TestClient into a socket the server had reset, and every further fwrite() raised a warning into the test transcript. The client latches the dead peer and throws once, which is what the flood loop already catches. tls/006 counted "Re-using existing connection" in curl's verbose log, wording that changes between curl releases. Keep-alive is now read from the peer port the server itself sees, one per request. --- .../compression/060-h1-request-malformed.phpt | 17 +++++++++--- tests/phpt/server/h2/_h2_client.inc | 26 ++++++++++++++++++- tests/phpt/server/tls/006-tls-keepalive.phpt | 18 ++++++------- 3 files changed, 47 insertions(+), 14 deletions(-) diff --git a/tests/phpt/server/compression/060-h1-request-malformed.phpt b/tests/phpt/server/compression/060-h1-request-malformed.phpt index e97fbcd..b5dd46e 100644 --- a/tests/phpt/server/compression/060-h1-request-malformed.phpt +++ b/tests/phpt/server/compression/060-h1-request-malformed.phpt @@ -58,18 +58,28 @@ $enc = HttpServerConfig::getSupportedEncodings(); $has_br = in_array('br', $enc, true); $has_zstd = in_array('zstd', $enc, true); +/* One line per encoding the build has, and a verdict that does not depend on + * how many that is: a build with brotli but not zstd is a supported build, and + * an expectation naming two lines fails it for something the server did right. */ $client = spawn(function () use ($port, $server, $has_br, $has_zstd) { delay(20); + $status = []; + if ($has_br) { /* random bytes are not valid brotli → decoder error → 400 */ - echo "br garbage: ", post($port, str_repeat("\xAA", 32), 'br'), "\n"; + $status['br'] = post($port, str_repeat("\xAA", 32), 'br'); + echo "br garbage: ", $status['br'], "\n"; } if ($has_zstd) { /* random bytes — no zstd magic → 400 */ - echo "zstd garbage: ", post($port, str_repeat("\xAA", 32), 'zstd'), "\n"; + $status['zstd'] = post($port, str_repeat("\xAA", 32), 'zstd'); + echo "zstd garbage: ", $status['zstd'], "\n"; } + echo "every malformed body refused: ", + ($status !== [] && array_unique(array_values($status)) === [400]) ? 'yes' : 'no', "\n"; + delay(50); $server->stop(); }); @@ -79,6 +89,5 @@ await($client); echo "Done\n"; ?> --EXPECTF-- -%Agarbage: 400 -%Agarbage: 400 +%Aevery malformed body refused: yes Done diff --git a/tests/phpt/server/h2/_h2_client.inc b/tests/phpt/server/h2/_h2_client.inc index 9e80034..31f3a99 100644 --- a/tests/phpt/server/h2/_h2_client.inc +++ b/tests/phpt/server/h2/_h2_client.inc @@ -57,6 +57,13 @@ class H2TestClient * a reset is the exception, and nine call sites destructure that tuple. */ private ?int $last_reset_code = null; + /* Latched once a frame write reaches a peer that is gone. A server under a + * flood is entitled to tear the connection down mid-write, and on Windows + * every further fwrite() on a reset socket raises a warning that lands in + * the test's own output. The latch turns the run of warnings into one + * exception the caller can catch. */ + private bool $peer_gone = false; + /** @param int $timeout_sec read timeout */ public function __construct(string $host, int $port, int $timeout_sec = 5) { @@ -115,8 +122,15 @@ class H2TestClient /* ----- Frame primitives ----- */ + /** + * @throws RuntimeException once the peer has gone, and on every later call. + */ private function writeFrame(int $type, int $flags, int $stream_id, string $payload): void { + if ($this->peer_gone) { + throw new RuntimeException('h2 client: peer is gone'); + } + $len = strlen($payload); $hdr = chr(($len >> 16) & 0xff) . chr(($len >> 8) & 0xff) @@ -124,7 +138,17 @@ class H2TestClient . chr($type) . chr($flags) . pack('N', $stream_id & 0x7fffffff); - fwrite($this->sock, $hdr . $payload); + $frame = $hdr . $payload; + + /* @ rather than a warning per frame: the failure is reported once, + * through the exception below. */ + $written = @fwrite($this->sock, $frame); + + if ($written === false || $written < strlen($frame)) { + $this->peer_gone = true; + throw new RuntimeException('h2 client: peer closed the connection mid-write'); + } + fflush($this->sock); } diff --git a/tests/phpt/server/tls/006-tls-keepalive.phpt b/tests/phpt/server/tls/006-tls-keepalive.phpt index 01bd5d4..5a4f5ca 100644 --- a/tests/phpt/server/tls/006-tls-keepalive.phpt +++ b/tests/phpt/server/tls/006-tls-keepalive.phpt @@ -39,8 +39,14 @@ $config = (new HttpServerConfig()) $server = new HttpServer($config); $request_count = 0; -$server->addHttpHandler(function ($req, $res) use (&$request_count, $server) { +/* Peer port per request. Three requests over one kept-alive connection share a + * port; three fresh connections get three. This is the server's own view of + * reuse — which is what the test is about, and what curl's verbose log only + * reports at second hand, in wording that changes between curl releases. */ +$peer_ports = []; +$server->addHttpHandler(function ($req, $res) use (&$request_count, &$peer_ports, $server) { $request_count++; + $peer_ports[] = $req->getRemotePort(); $res->setStatusCode(200) ->setHeader('Content-Type', 'text/plain') ->setHeader('X-Seq', (string)$request_count) @@ -56,10 +62,6 @@ $client = spawn(function () use ($port) { /* curl --next fires 3 requests in sequence on the SAME connection. * Without --next, curl still keeps the conn alive by default, * but --next makes the reuse explicit and comprehensible. */ - /* Use curl's verbose output; the string "Re-using existing connection" - * (sometimes "Reusing existing connection") is curl's own indicator - * that it kept the TLS/TCP session. More robust than parsing - * num_connects, which is per-transfer. */ $cmd = sprintf( 'curl -kv --http1.1 -m 5 ' . 'https://127.0.0.1:%d/first ' @@ -78,10 +80,8 @@ spawn(function () use ($server) { $server->start(); $out = await($client); -$reuse_count = preg_match_all('/Re-?using existing connection/i', $out); - echo "count: $request_count\n"; -echo "reuses: $reuse_count\n"; +echo "connections: ", count(array_unique($peer_ports)), "\n"; echo "bodies: " . (strpos($out, 'r1:/first') !== false ? '1' : '_') . (strpos($out, 'r2:/second') !== false ? '2' : '_') @@ -91,6 +91,6 @@ echo "bodies: " echo "Done\n"; --EXPECT-- count: 3 -reuses: 2 +connections: 1 bodies: 123 Done From 8f1493e6d4029bc85182f90a6d1f3186fffa1765 Mon Sep 17 00:00:00 2001 From: Edmond <1571649+EdmondDantes@users.noreply.github.com> Date: Wed, 26 Aug 2026 10:20:26 +0300 Subject: [PATCH 2/2] test(h2): keep the client's own acks from throwing into collectResponse collectResponse writes SETTINGS and PING acks and window refills from inside its read loop, and nine call sites destructure its tuple without a catch. A server that closes right after sending one of those frames would have reached them as an uncaught exception where a closed connection used to give them an empty response. Frames the caller asked for still throw; the ones the client emits by itself latch and return false. chaos/001 drives the same open-then-RST burst chaos/002 does, with no catch: the day the server answers rapid reset by closing, that loop breaks for the reason chaos/002 broke. The write-failure message no longer claims the peer closed, since a send timeout reaches the same branch. --- .../phpt/server/chaos/001-h2-rapid-reset.phpt | 16 +++-- .../compression/060-h1-request-malformed.phpt | 7 +- tests/phpt/server/h2/_h2_client.inc | 71 +++++++++++++------ 3 files changed, 67 insertions(+), 27 deletions(-) diff --git a/tests/phpt/server/chaos/001-h2-rapid-reset.phpt b/tests/phpt/server/chaos/001-h2-rapid-reset.phpt index 0ff5e53..0b06450 100644 --- a/tests/phpt/server/chaos/001-h2-rapid-reset.phpt +++ b/tests/phpt/server/chaos/001-h2-rapid-reset.phpt @@ -57,10 +57,18 @@ $client = spawn(function () use ($port, $server) { } /* Open-then-RST burst: HEADERS(END_STREAM) immediately followed by - * RST_STREAM(CANCEL) on the same stream id, BURST times. */ - for ($i = 0; $i < BURST; $i++) { - $sid = $c->sendRequest('GET', '/', 'x'); - $c->sendRstStream($sid, 0x8 /* CANCEL */); + * RST_STREAM(CANCEL) on the same stream id, BURST times. + * + * Tearing the connection down is one legitimate answer to a rapid-reset + * attack; the burst then writes into a socket the server has closed, and + * the client reports that by throwing. What this test asserts is the + * liveness probe below, which opens a connection of its own. */ + try { + for ($i = 0; $i < BURST; $i++) { + $sid = $c->sendRequest('GET', '/', 'x'); + $c->sendRstStream($sid, 0x8 /* CANCEL */); + } + } catch (\Throwable $e) { } $c->close(); diff --git a/tests/phpt/server/compression/060-h1-request-malformed.phpt b/tests/phpt/server/compression/060-h1-request-malformed.phpt index b5dd46e..0549ade 100644 --- a/tests/phpt/server/compression/060-h1-request-malformed.phpt +++ b/tests/phpt/server/compression/060-h1-request-malformed.phpt @@ -58,9 +58,10 @@ $enc = HttpServerConfig::getSupportedEncodings(); $has_br = in_array('br', $enc, true); $has_zstd = in_array('zstd', $enc, true); -/* One line per encoding the build has, and a verdict that does not depend on - * how many that is: a build with brotli but not zstd is a supported build, and - * an expectation naming two lines fails it for something the server did right. */ +/* One line per encoding the build has, for the failure diff, and a verdict that + * holds whatever that set is. Brotli without zstd is a supported build, and the + * property under test — a malformed body is refused — says nothing about how + * many encodings were compiled in. */ $client = spawn(function () use ($port, $server, $has_br, $has_zstd) { delay(20); diff --git a/tests/phpt/server/h2/_h2_client.inc b/tests/phpt/server/h2/_h2_client.inc index 31f3a99..a17d0c4 100644 --- a/tests/phpt/server/h2/_h2_client.inc +++ b/tests/phpt/server/h2/_h2_client.inc @@ -123,33 +123,57 @@ class H2TestClient /* ----- Frame primitives ----- */ /** - * @throws RuntimeException once the peer has gone, and on every later call. + * A frame the caller asked for: reaching a peer that is gone is that + * caller's business, so it arrives as an exception. + * + * @throws RuntimeException when the peer is gone, on this call and every + * later one. */ private function writeFrame(int $type, int $flags, int $stream_id, string $payload): void + { + if (!$this->tryWriteFrame($type, $flags, $stream_id, $payload)) { + throw new RuntimeException( + 'h2 client: the peer is gone, or stopped reading long enough to time the write out'); + } + } + + /** + * A frame this client emits on its own — a SETTINGS or PING ack answering + * what it just read. Those run inside collectResponse, whose callers + * destructure its tuple and would meet an exception where they expect the + * empty response a closed connection gives them. + * + * @return bool false once the peer is gone, on this call and every later one. + */ + private function tryWriteFrame(int $type, int $flags, int $stream_id, string $payload): bool { if ($this->peer_gone) { - throw new RuntimeException('h2 client: peer is gone'); + return false; } $len = strlen($payload); - $hdr = chr(($len >> 16) & 0xff) - . chr(($len >> 8) & 0xff) - . chr( $len & 0xff) - . chr($type) - . chr($flags) - . pack('N', $stream_id & 0x7fffffff); - $frame = $hdr . $payload; - - /* @ rather than a warning per frame: the failure is reported once, - * through the exception below. */ + $frame = chr(($len >> 16) & 0xff) + . chr(($len >> 8) & 0xff) + . chr( $len & 0xff) + . chr($type) + . chr($flags) + . pack('N', $stream_id & 0x7fffffff) + . $payload; + $frame_len = strlen($frame); + + /* @ rather than a warning per frame: a peer that went away is reported + * once, by the latch, and a run of warnings would land in the output + * the test is compared against. */ $written = @fwrite($this->sock, $frame); - if ($written === false || $written < strlen($frame)) { + if ($written === false || $written < $frame_len) { $this->peer_gone = true; - throw new RuntimeException('h2 client: peer closed the connection mid-write'); + return false; } fflush($this->sock); + + return true; } /** @@ -262,8 +286,14 @@ class H2TestClient */ public function sendWindowUpdate(int $stream_id, int $increment): void { - $payload = pack('N', $increment & 0x7fffffff); - $this->writeFrame(H2_FRAME_WINDOW_UPDATE, 0, $stream_id, $payload); + $this->writeFrame(H2_FRAME_WINDOW_UPDATE, 0, $stream_id, + self::windowUpdatePayload($increment)); + } + + /** @param int $increment 1..2^31-1; the reserved high bit is masked off. */ + private static function windowUpdatePayload(int $increment): string + { + return pack('N', $increment & 0x7fffffff); } public function sendSettingsAck(): void @@ -381,7 +411,7 @@ class H2TestClient if ($type === H2_FRAME_SETTINGS) { if (!($flags & H2_FLAG_ACK)) { /* Server settings — ack. */ - $this->sendSettingsAck(); + $this->tryWriteFrame(H2_FRAME_SETTINGS, H2_FLAG_ACK, 0, ''); } continue; } @@ -394,7 +424,7 @@ class H2TestClient if ($type === H2_FRAME_PING) { if (!($flags & H2_FLAG_ACK)) { /* Reflect payload with ACK bit set. */ - $this->writeFrame(H2_FRAME_PING, H2_FLAG_ACK, 0, $payload); + $this->tryWriteFrame(H2_FRAME_PING, H2_FLAG_ACK, 0, $payload); } continue; } @@ -424,8 +454,9 @@ class H2TestClient if ($auto_window && $consumed_since_ack >= $ack_threshold) { /* Refill both stream + connection windows. */ - $this->sendWindowUpdate($stream_id, $consumed_since_ack); - $this->sendWindowUpdate(0, $consumed_since_ack); + $credit = self::windowUpdatePayload($consumed_since_ack); + $this->tryWriteFrame(H2_FRAME_WINDOW_UPDATE, 0, $stream_id, $credit); + $this->tryWriteFrame(H2_FRAME_WINDOW_UPDATE, 0, 0, $credit); $consumed_since_ack = 0; }