From d26d7a678edb8e4eae1ca492960ee4670e46a5ae Mon Sep 17 00:00:00 2001 From: Volodymyr Panivko Date: Thu, 21 May 2026 23:30:22 +0200 Subject: [PATCH 01/12] THRIFT-6023: Add HTTP transport support to PHP cross-tests Client: php PHP was excluded from the cross-test HTTP matrix while other languages (Python, Go, Node, C++, Java, Lua, Dart, JS, D, Haskell) already declare "http" in their cross-test transports and verify interop. Server: TestServer.php detects --transport=http and pcntl_exec()s into PHP's built-in web server (php -S 127.0.0.1:$port TestServer.php). The same script re-enters under SAPI cli-server, reads the Thrift request via TPhpStream from php://input, dispatches it through the existing processor, and writes the response to php://output. Original -d flags from the launcher cmdline are preserved so the accel/sockets extensions stay loaded. Client: TestClient.php gains an --transport=http branch using TPsrHttpClient (THRIFT-6010) against http://127.0.0.1:\$port/, with no buffered/framed wrapper since TPsrHttpClient buffers internally. tests.json: adds "http" to PHP server and client transports and raises the client timeout from 6s to 10s to absorb HTTP overhead. composer.json: adds guzzlehttp/guzzle ^7.8 to require-dev so the PSR-18 auto-discovery in TPsrHttpClient resolves at cross-test time. Generated-by: Claude Opus 4.7 --- composer.json | 1 + test/known_failures_Linux.json | 4 ++ test/php/TestClient.php | 37 ++++++---- test/php/TestServer.php | 119 +++++++++++++++++++++++++++------ test/tests.json | 8 ++- 5 files changed, 132 insertions(+), 37 deletions(-) diff --git a/composer.json b/composer.json index f41bb1f1f1d..8cc315d3981 100644 --- a/composer.json +++ b/composer.json @@ -31,6 +31,7 @@ "php-mock/php-mock-phpunit": "^2.10", "phpstan/phpstan": "^1.12", "nyholm/psr7": "^1.8", + "guzzlehttp/guzzle": "^7.8", "ext-json": "*", "ext-xml": "*", "ext-curl": "*", diff --git a/test/known_failures_Linux.json b/test/known_failures_Linux.json index e4cd98bd963..ccdb5de776d 100644 --- a/test/known_failures_Linux.json +++ b/test/known_failures_Linux.json @@ -617,6 +617,10 @@ "nodejs-nodejs_compact_websocket-domain", "nodejs-nodejs_header_websocket-domain", "nodejs-nodejs_json_websocket-domain", + "nodejs-php_binary-accel_http-ip", + "nodejs-php_binary_http-ip", + "nodejs-php_compact_http-ip", + "nodejs-php_json_http-ip", "nodejs-py_binary-accel_http-domain", "nodejs-py_binary-accel_http-ip", "nodejs-py_binary-accel_http-ip-ssl", diff --git a/test/php/TestClient.php b/test/php/TestClient.php index c9e1ba7e5ec..42868a0bc8e 100755 --- a/test/php/TestClient.php +++ b/test/php/TestClient.php @@ -59,6 +59,7 @@ /** Include the socket layer */ use Thrift\Transport\TFramedTransport; use Thrift\Transport\TBufferedTransport; +use Thrift\Transport\TPsrHttpClient; /** * Minimal PSR-3 logger that forwards every message to PHP's error_log @@ -122,22 +123,32 @@ function makeProtocol($transport, $PROTO) $hosts = array('localhost'); $logger = new StderrLogger(); -$socket = new TSocket($host, $port, false, $logger); -$socket = new TSocketPool($hosts, $port, false, $logger); - -if ($MODE == 'inline') { - $transport = $socket; - $testClient = new \ThriftTest\ThriftTestClient($transport); -} else if ($MODE == 'framed') { - $framedSocket = new TFramedTransport($socket); - $transport = $framedSocket; + +if ($MODE == 'http') { + // HTTP cross-test peer (e.g. PHP, Python, Go) is bound to 127.0.0.1 by the + // cross-runner; talk to it via PSR-18. TPsrHttpClient buffers internally, + // so no TBufferedTransport/TFramedTransport wrapping. + $transport = new TPsrHttpClient(sprintf('http://127.0.0.1:%d/', $port)); $protocol = makeProtocol($transport, $PROTO); $testClient = new \ThriftTest\ThriftTestClient($protocol); } else { - $bufferedSocket = new TBufferedTransport($socket, 1024, 1024); - $transport = $bufferedSocket; - $protocol = makeProtocol($transport, $PROTO); - $testClient = new \ThriftTest\ThriftTestClient($protocol); + $socket = new TSocket($host, $port, false, $logger); + $socket = new TSocketPool($hosts, $port, false, $logger); + + if ($MODE == 'inline') { + $transport = $socket; + $testClient = new \ThriftTest\ThriftTestClient($transport); + } else if ($MODE == 'framed') { + $framedSocket = new TFramedTransport($socket); + $transport = $framedSocket; + $protocol = makeProtocol($transport, $PROTO); + $testClient = new \ThriftTest\ThriftTestClient($protocol); + } else { + $bufferedSocket = new TBufferedTransport($socket, 1024, 1024); + $transport = $bufferedSocket; + $protocol = makeProtocol($transport, $PROTO); + $testClient = new \ThriftTest\ThriftTestClient($protocol); + } } $transport->open(); diff --git a/test/php/TestServer.php b/test/php/TestServer.php index 4ab773cb88c..c755e391fb1 100644 --- a/test/php/TestServer.php +++ b/test/php/TestServer.php @@ -4,6 +4,61 @@ require_once __DIR__ . '/../../vendor/autoload.php'; +/** + * Build a protocol factory by name. Shared between the CLI socket server + * and the cli-server HTTP request handler. + */ +function thrift_test_protocol_factory(string $protocol): \Thrift\Factory\TProtocolFactory +{ + switch ($protocol) { + case 'binary': + return new \Thrift\Factory\TBinaryProtocolFactory(false, true); + case 'accel': + if (!function_exists('thrift_protocol_write_binary')) { + fwrite(STDERR, "Acceleration extension is not loaded\n"); + exit(1); + } + return new \Thrift\Factory\TBinaryProtocolAcceleratedFactory(); + case 'compact': + return new \Thrift\Factory\TCompactProtocolFactory(); + case 'json': + return new \Thrift\Factory\TJSONProtocolFactory(); + default: + fwrite(STDERR, "--protocol must be one of {binary|compact|json|accel}\n"); + exit(1); + } +} + +// When invoked under PHP's built-in web server (php -S) we are a per-request +// router: read the Thrift request from php://input, dispatch it through the +// processor, and write the response to php://output. +if (PHP_SAPI === 'cli-server') { + $loader = new \Thrift\ClassLoader\ThriftClassLoader(); + $loader->registerDefinition('ThriftTest', __DIR__ . '/gen-php-classmap'); + $loader->register(); + + require_once __DIR__ . '/Handler.php'; + + $protocolFactory = thrift_test_protocol_factory(getenv('THRIFT_TEST_PROTOCOL') ?: 'binary'); + + $handler = new Handler(); + $processor = new ThriftTest\ThriftTestProcessor($handler); + + header('Content-Type: application/x-thrift'); + + $transport = new \Thrift\Transport\TPhpStream( + \Thrift\Transport\TPhpStream::MODE_R | \Thrift\Transport\TPhpStream::MODE_W + ); + $inputProtocol = $protocolFactory->getProtocol($transport); + $outputProtocol = $protocolFactory->getProtocol($transport); + + $transport->open(); + $processor->process($inputProtocol, $outputProtocol); + $transport->close(); + + return true; +} + $opts = getopt( 'h::', [ @@ -47,6 +102,48 @@ $transport = $opts['transport'] ?? 'buffered'; $protocol = $opts['protocol'] ?? 'binary'; +// HTTP transport: re-exec under PHP's built-in web server, which TCP-binds +// the port so the cross-runner's readiness probe (socket.connect_ex) succeeds. +// We re-enter this same file as the router; the cli-server branch above +// handles each request. +if ($transport === 'http') { + if (!function_exists('pcntl_exec')) { + fwrite(STDERR, "PHP HTTP cross-test requires ext-pcntl.\n"); + exit(1); + } + $cmdline = @file_get_contents('/proc/self/cmdline'); + if ($cmdline === false) { + fwrite(STDERR, "PHP HTTP cross-test requires /proc/self/cmdline (Linux).\n"); + exit(1); + } + $parts = explode("\0", rtrim($cmdline, "\0")); + // parts[0] = php binary, then -d/-z flags, then this script, then script args. + $scriptIdx = null; + foreach ($parts as $idx => $part) { + if ($idx > 0 && basename($part) === basename(__FILE__)) { + $scriptIdx = $idx; + break; + } + } + if ($scriptIdx === null) { + fwrite(STDERR, "Cannot locate script name in /proc/self/cmdline.\n"); + exit(1); + } + // pcntl_exec does NOT do PATH resolution — use the absolute path from + // PHP_BINARY rather than argv[0], which may be a bare "php". + $phpBinary = PHP_BINARY; + $phpFlags = array_slice($parts, 1, $scriptIdx - 1); + + putenv('THRIFT_TEST_PROTOCOL=' . $protocol); + + $newArgs = array_merge($phpFlags, ['-S', '127.0.0.1:' . $port, __FILE__]); + + echo "Starting the Test server (HTTP via php -S)...\n"; + + pcntl_exec($phpBinary, $newArgs, $_ENV); + fwrite(STDERR, "pcntl_exec failed\n"); + exit(1); +} $loader = new Thrift\ClassLoader\ThriftClassLoader(); $loader->registerDefinition('ThriftTest', __DIR__ . '/gen-php-classmap'); @@ -71,27 +168,7 @@ $serverTransportFactory = new \Thrift\Factory\TTransportFactory(); } -switch ($protocol) { - case 'binary': - $protocolFactory = new \Thrift\Factory\TBinaryProtocolFactory(false, true); - break; - case 'accel': - if (!function_exists('thrift_protocol_write_binary')) { - fwrite(STDERR, "Acceleration extension is not loaded\n"); - exit(1); - } - $protocolFactory = new \Thrift\Factory\TBinaryProtocolAcceleratedFactory(); - break; - case 'compact': - $protocolFactory = new \Thrift\Factory\TCompactProtocolFactory(); - break; - case 'json': - $protocolFactory = new \Thrift\Factory\TJSONProtocolFactory(); - break; - default: - fwrite(STDERR, "--protocol must be one of {binary|compact|json|accel}\n"); - exit(1); -} +$protocolFactory = thrift_test_protocol_factory($protocol); // `localhost` may resolve to an IPv6-only listener in newer PHP/runtime combinations, // while some cross-test clients still connect via 127.0.0.1. Bind explicitly to IPv4. diff --git a/test/tests.json b/test/tests.json index d0abdeb9a3d..1ef58bcab65 100644 --- a/test/tests.json +++ b/test/tests.json @@ -555,7 +555,8 @@ "server": { "transports": [ "buffered", - "framed" + "framed", + "http" ], "sockets": [ "ip" @@ -580,10 +581,11 @@ ] }, "client": { - "timeout": 6, + "timeout": 10, "transports": [ "buffered", - "framed" + "framed", + "http" ], "sockets": [ "ip" From c7f2193d7578ada28b6c9ec0e1aa72a38863da87 Mon Sep 17 00:00:00 2001 From: Volodymyr Panivko Date: Fri, 22 May 2026 05:54:47 +0200 Subject: [PATCH 02/12] DEBUG: log request/response bytes for HTTP cross-test diagnosis --- test/php/TestServer.php | 40 +++++++++++++++++++++++++++++++--------- 1 file changed, 31 insertions(+), 9 deletions(-) diff --git a/test/php/TestServer.php b/test/php/TestServer.php index c755e391fb1..7a5f8e04371 100644 --- a/test/php/TestServer.php +++ b/test/php/TestServer.php @@ -46,15 +46,37 @@ function thrift_test_protocol_factory(string $protocol): \Thrift\Factory\TProtoc header('Content-Type: application/x-thrift'); - $transport = new \Thrift\Transport\TPhpStream( - \Thrift\Transport\TPhpStream::MODE_R | \Thrift\Transport\TPhpStream::MODE_W - ); - $inputProtocol = $protocolFactory->getProtocol($transport); - $outputProtocol = $protocolFactory->getProtocol($transport); - - $transport->open(); - $processor->process($inputProtocol, $outputProtocol); - $transport->close(); + // DEBUG: capture request body and response body, write hex to stderr. + $requestBody = file_get_contents('php://input'); + fwrite(STDERR, '[DEBUG req] proto=' . (getenv('THRIFT_TEST_PROTOCOL') ?: 'binary') + . ' len=' . strlen($requestBody) + . ' hex=' . bin2hex(substr($requestBody, 0, 64)) . "\n"); + // Re-expose the consumed body so TPhpStream can read it. + $bodyStream = fopen('php://temp', 'r+'); + fwrite($bodyStream, $requestBody); + rewind($bodyStream); + $GLOBALS['__thrift_debug_in'] = $bodyStream; + + ob_start(); + try { + $transport = new \Thrift\Transport\TPhpStream( + \Thrift\Transport\TPhpStream::MODE_R | \Thrift\Transport\TPhpStream::MODE_W + ); + $inputProtocol = $protocolFactory->getProtocol($transport); + $outputProtocol = $protocolFactory->getProtocol($transport); + + $transport->open(); + $processor->process($inputProtocol, $outputProtocol); + $transport->close(); + } catch (\Throwable $e) { + fwrite(STDERR, '[DEBUG exc] ' . get_class($e) . ': ' . $e->getMessage() + . ' @ ' . $e->getFile() . ':' . $e->getLine() . "\n"); + fwrite(STDERR, $e->getTraceAsString() . "\n"); + } + $responseBody = ob_get_clean(); + fwrite(STDERR, '[DEBUG res] len=' . strlen($responseBody) + . ' hex=' . bin2hex(substr($responseBody, 0, 64)) . "\n"); + echo $responseBody; return true; } From da84c5d67c93213337850ccc4ee2810783c7c398 Mon Sep 17 00:00:00 2001 From: Volodymyr Panivko Date: Fri, 22 May 2026 06:39:42 +0200 Subject: [PATCH 03/12] DEBUG: STDERR is undefined in cli-server SAPI, write hex log to test/log/ --- test/php/TestServer.php | 37 ++++++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/test/php/TestServer.php b/test/php/TestServer.php index 7a5f8e04371..7c5fb4761a7 100644 --- a/test/php/TestServer.php +++ b/test/php/TestServer.php @@ -46,16 +46,18 @@ function thrift_test_protocol_factory(string $protocol): \Thrift\Factory\TProtoc header('Content-Type: application/x-thrift'); - // DEBUG: capture request body and response body, write hex to stderr. + // DEBUG: write into the cross-test log dir so the GHA artifact captures it. + $debugLog = __DIR__ . '/../log/thrift_debug.log'; + @mkdir(dirname($debugLog), 0777, true); + $proto = getenv('THRIFT_TEST_PROTOCOL') ?: 'binary'; $requestBody = file_get_contents('php://input'); - fwrite(STDERR, '[DEBUG req] proto=' . (getenv('THRIFT_TEST_PROTOCOL') ?: 'binary') + file_put_contents( + $debugLog, + '[req] proto=' . $proto . ' len=' . strlen($requestBody) - . ' hex=' . bin2hex(substr($requestBody, 0, 64)) . "\n"); - // Re-expose the consumed body so TPhpStream can read it. - $bodyStream = fopen('php://temp', 'r+'); - fwrite($bodyStream, $requestBody); - rewind($bodyStream); - $GLOBALS['__thrift_debug_in'] = $bodyStream; + . ' hex=' . bin2hex(substr($requestBody, 0, 64)) . "\n", + FILE_APPEND + ); ob_start(); try { @@ -69,13 +71,22 @@ function thrift_test_protocol_factory(string $protocol): \Thrift\Factory\TProtoc $processor->process($inputProtocol, $outputProtocol); $transport->close(); } catch (\Throwable $e) { - fwrite(STDERR, '[DEBUG exc] ' . get_class($e) . ': ' . $e->getMessage() - . ' @ ' . $e->getFile() . ':' . $e->getLine() . "\n"); - fwrite(STDERR, $e->getTraceAsString() . "\n"); + file_put_contents( + $debugLog, + '[exc] ' . get_class($e) . ': ' . $e->getMessage() + . ' @ ' . $e->getFile() . ':' . $e->getLine() . "\n" + . $e->getTraceAsString() . "\n", + FILE_APPEND + ); } $responseBody = ob_get_clean(); - fwrite(STDERR, '[DEBUG res] len=' . strlen($responseBody) - . ' hex=' . bin2hex(substr($responseBody, 0, 64)) . "\n"); + file_put_contents( + $debugLog, + '[res] proto=' . $proto + . ' len=' . strlen($responseBody) + . ' hex=' . bin2hex(substr($responseBody, 0, 64)) . "\n", + FILE_APPEND + ); echo $responseBody; return true; From f396d8d27c649e553c648b3cc8a5aa9bf5896cb4 Mon Sep 17 00:00:00 2001 From: Volodymyr Panivko Date: Fri, 22 May 2026 06:53:18 +0200 Subject: [PATCH 04/12] Fix: pass THRIFT_TEST_PROTOCOL through pcntl_exec; remove debug logging --- test/php/TestServer.php | 54 +++++++++-------------------------------- 1 file changed, 12 insertions(+), 42 deletions(-) diff --git a/test/php/TestServer.php b/test/php/TestServer.php index 7c5fb4761a7..9c78a5fcf63 100644 --- a/test/php/TestServer.php +++ b/test/php/TestServer.php @@ -46,48 +46,15 @@ function thrift_test_protocol_factory(string $protocol): \Thrift\Factory\TProtoc header('Content-Type: application/x-thrift'); - // DEBUG: write into the cross-test log dir so the GHA artifact captures it. - $debugLog = __DIR__ . '/../log/thrift_debug.log'; - @mkdir(dirname($debugLog), 0777, true); - $proto = getenv('THRIFT_TEST_PROTOCOL') ?: 'binary'; - $requestBody = file_get_contents('php://input'); - file_put_contents( - $debugLog, - '[req] proto=' . $proto - . ' len=' . strlen($requestBody) - . ' hex=' . bin2hex(substr($requestBody, 0, 64)) . "\n", - FILE_APPEND + $transport = new \Thrift\Transport\TPhpStream( + \Thrift\Transport\TPhpStream::MODE_R | \Thrift\Transport\TPhpStream::MODE_W ); + $inputProtocol = $protocolFactory->getProtocol($transport); + $outputProtocol = $protocolFactory->getProtocol($transport); - ob_start(); - try { - $transport = new \Thrift\Transport\TPhpStream( - \Thrift\Transport\TPhpStream::MODE_R | \Thrift\Transport\TPhpStream::MODE_W - ); - $inputProtocol = $protocolFactory->getProtocol($transport); - $outputProtocol = $protocolFactory->getProtocol($transport); - - $transport->open(); - $processor->process($inputProtocol, $outputProtocol); - $transport->close(); - } catch (\Throwable $e) { - file_put_contents( - $debugLog, - '[exc] ' . get_class($e) . ': ' . $e->getMessage() - . ' @ ' . $e->getFile() . ':' . $e->getLine() . "\n" - . $e->getTraceAsString() . "\n", - FILE_APPEND - ); - } - $responseBody = ob_get_clean(); - file_put_contents( - $debugLog, - '[res] proto=' . $proto - . ' len=' . strlen($responseBody) - . ' hex=' . bin2hex(substr($responseBody, 0, 64)) . "\n", - FILE_APPEND - ); - echo $responseBody; + $transport->open(); + $processor->process($inputProtocol, $outputProtocol); + $transport->close(); return true; } @@ -167,13 +134,16 @@ function thrift_test_protocol_factory(string $protocol): \Thrift\Factory\TProtoc $phpBinary = PHP_BINARY; $phpFlags = array_slice($parts, 1, $scriptIdx - 1); - putenv('THRIFT_TEST_PROTOCOL=' . $protocol); + // $_ENV is empty under the default variables_order=GPCS, so build the env + // for pcntl_exec from getenv() and inject our protocol selector. + $env = getenv(); + $env['THRIFT_TEST_PROTOCOL'] = $protocol; $newArgs = array_merge($phpFlags, ['-S', '127.0.0.1:' . $port, __FILE__]); echo "Starting the Test server (HTTP via php -S)...\n"; - pcntl_exec($phpBinary, $newArgs, $_ENV); + pcntl_exec($phpBinary, $newArgs, $env); fwrite(STDERR, "pcntl_exec failed\n"); exit(1); } From 93b7dcaccc2b70513a70895c0d7f3c244ca3928f Mon Sep 17 00:00:00 2001 From: Volodymyr Panivko Date: Fri, 22 May 2026 07:04:53 +0200 Subject: [PATCH 05/12] Add php-java HTTP oneway timing tests to known failures --- test/known_failures_Linux.json | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/known_failures_Linux.json b/test/known_failures_Linux.json index ccdb5de776d..ee0f8026b05 100644 --- a/test/known_failures_Linux.json +++ b/test/known_failures_Linux.json @@ -648,6 +648,10 @@ "perl-netstd_multi-binary_buffered-ip-ssl", "perl-netstd_multi-binary_framed-ip", "perl-netstd_multi-binary_framed-ip-ssl", + "php-java_accel-binary_http-ip", + "php-java_binary_http-ip", + "php-java_compact_http-ip", + "php-java_json_http-ip", "py-cpp_accel-binary_http-domain", "py-cpp_accel-binary_http-ip", "py-cpp_accel-binary_http-ip-ssl", From 8489c84ffc7cf584c2bd3e2e7db9ad3984b17884 Mon Sep 17 00:00:00 2001 From: Volodymyr Panivko Date: Fri, 22 May 2026 07:24:09 +0200 Subject: [PATCH 06/12] Add php-cpp HTTP known failures (cpp HTTP client incompatible with most servers) --- test/known_failures_Linux.json | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/known_failures_Linux.json b/test/known_failures_Linux.json index ee0f8026b05..508e4d45b3e 100644 --- a/test/known_failures_Linux.json +++ b/test/known_failures_Linux.json @@ -648,6 +648,10 @@ "perl-netstd_multi-binary_buffered-ip-ssl", "perl-netstd_multi-binary_framed-ip", "perl-netstd_multi-binary_framed-ip-ssl", + "php-cpp_accel-binary_http-ip", + "php-cpp_binary_http-ip", + "php-cpp_compact_http-ip", + "php-cpp_json_http-ip", "php-java_accel-binary_http-ip", "php-java_binary_http-ip", "php-java_compact_http-ip", From b7ff5f037c244fbbf31da31380006212c1b377b2 Mon Sep 17 00:00:00 2001 From: Volodymyr Panivko Date: Fri, 22 May 2026 22:00:29 +0200 Subject: [PATCH 07/12] Simplify: single getProtocol() call, scope $logger to socket path --- test/php/TestClient.php | 3 +-- test/php/TestServer.php | 5 ++--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/test/php/TestClient.php b/test/php/TestClient.php index 42868a0bc8e..9a4f4e3ed3b 100755 --- a/test/php/TestClient.php +++ b/test/php/TestClient.php @@ -122,8 +122,6 @@ function makeProtocol($transport, $PROTO) $hosts = array('localhost'); -$logger = new StderrLogger(); - if ($MODE == 'http') { // HTTP cross-test peer (e.g. PHP, Python, Go) is bound to 127.0.0.1 by the // cross-runner; talk to it via PSR-18. TPsrHttpClient buffers internally, @@ -132,6 +130,7 @@ function makeProtocol($transport, $PROTO) $protocol = makeProtocol($transport, $PROTO); $testClient = new \ThriftTest\ThriftTestClient($protocol); } else { + $logger = new StderrLogger(); $socket = new TSocket($host, $port, false, $logger); $socket = new TSocketPool($hosts, $port, false, $logger); diff --git a/test/php/TestServer.php b/test/php/TestServer.php index 9c78a5fcf63..2238f613844 100644 --- a/test/php/TestServer.php +++ b/test/php/TestServer.php @@ -49,11 +49,10 @@ function thrift_test_protocol_factory(string $protocol): \Thrift\Factory\TProtoc $transport = new \Thrift\Transport\TPhpStream( \Thrift\Transport\TPhpStream::MODE_R | \Thrift\Transport\TPhpStream::MODE_W ); - $inputProtocol = $protocolFactory->getProtocol($transport); - $outputProtocol = $protocolFactory->getProtocol($transport); + $protocol = $protocolFactory->getProtocol($transport); $transport->open(); - $processor->process($inputProtocol, $outputProtocol); + $processor->process($protocol, $protocol); $transport->close(); return true; From 025863b5e9207a726a880f85c1315fd232f6c4a8 Mon Sep 17 00:00:00 2001 From: Volodymyr Panivko Date: Fri, 22 May 2026 22:04:56 +0200 Subject: [PATCH 08/12] Address review: fast oneway response in PHP HTTP server; simplify TestClient MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - TestServer.php cli-server branch peeks the Thrift message type; for ONEWAY it sends an empty HTTP 200 immediately (Content-Length: 0) and forks the handler so the client's oneway call returns fast — matches the THttpServer pattern from THRIFT-6021 and lets java-php HTTP work without known_failures entries. - Switch cli-server I/O to TMemoryBuffer; TPhpStream is no longer needed. - TestClient.php: replace 4-way if/elseif over \$MODE with two match expressions; drop the dead TSocket assignment and unused \$hosts/\$logger. - Remove php-cpp HTTP known_failures (unblocked by THRIFT-6021 / #3514). - Remove php-java HTTP known_failures (unblocked by fast oneway). --- test/known_failures_Linux.json | 8 ----- test/php/TestClient.php | 53 ++++++++++++---------------------- test/php/TestServer.php | 48 +++++++++++++++++++++++------- 3 files changed, 56 insertions(+), 53 deletions(-) diff --git a/test/known_failures_Linux.json b/test/known_failures_Linux.json index 508e4d45b3e..ccdb5de776d 100644 --- a/test/known_failures_Linux.json +++ b/test/known_failures_Linux.json @@ -648,14 +648,6 @@ "perl-netstd_multi-binary_buffered-ip-ssl", "perl-netstd_multi-binary_framed-ip", "perl-netstd_multi-binary_framed-ip-ssl", - "php-cpp_accel-binary_http-ip", - "php-cpp_binary_http-ip", - "php-cpp_compact_http-ip", - "php-cpp_json_http-ip", - "php-java_accel-binary_http-ip", - "php-java_binary_http-ip", - "php-java_compact_http-ip", - "php-java_json_http-ip", "py-cpp_accel-binary_http-domain", "py-cpp_accel-binary_http-ip", "py-cpp_accel-binary_http-ip-ssl", diff --git a/test/php/TestClient.php b/test/php/TestClient.php index 9a4f4e3ed3b..f9b51c5f855 100755 --- a/test/php/TestClient.php +++ b/test/php/TestClient.php @@ -52,14 +52,10 @@ use Thrift\Protocol\TCompactProtocol; use Thrift\Protocol\TJSONProtocol; -/** Include the socket layer */ -use Thrift\Transport\TSocket; -use Thrift\Transport\TSocketPool; - -/** Include the socket layer */ -use Thrift\Transport\TFramedTransport; use Thrift\Transport\TBufferedTransport; +use Thrift\Transport\TFramedTransport; use Thrift\Transport\TPsrHttpClient; +use Thrift\Transport\TSocketPool; /** * Minimal PSR-3 logger that forwards every message to PHP's error_log @@ -120,35 +116,22 @@ function makeProtocol($transport, $PROTO) } } -$hosts = array('localhost'); - -if ($MODE == 'http') { - // HTTP cross-test peer (e.g. PHP, Python, Go) is bound to 127.0.0.1 by the - // cross-runner; talk to it via PSR-18. TPsrHttpClient buffers internally, - // so no TBufferedTransport/TFramedTransport wrapping. - $transport = new TPsrHttpClient(sprintf('http://127.0.0.1:%d/', $port)); - $protocol = makeProtocol($transport, $PROTO); - $testClient = new \ThriftTest\ThriftTestClient($protocol); -} else { - $logger = new StderrLogger(); - $socket = new TSocket($host, $port, false, $logger); - $socket = new TSocketPool($hosts, $port, false, $logger); - - if ($MODE == 'inline') { - $transport = $socket; - $testClient = new \ThriftTest\ThriftTestClient($transport); - } else if ($MODE == 'framed') { - $framedSocket = new TFramedTransport($socket); - $transport = $framedSocket; - $protocol = makeProtocol($transport, $PROTO); - $testClient = new \ThriftTest\ThriftTestClient($protocol); - } else { - $bufferedSocket = new TBufferedTransport($socket, 1024, 1024); - $transport = $bufferedSocket; - $protocol = makeProtocol($transport, $PROTO); - $testClient = new \ThriftTest\ThriftTestClient($protocol); - } -} +// TPsrHttpClient buffers internally, so no framed/buffered wrapper is needed. +// Inline mode passes the raw transport to the generated client without a +// protocol wrapper, matching the legacy code path. +$transport = match ($MODE) { + 'http' => new TPsrHttpClient(sprintf('http://127.0.0.1:%d/', $port)), + default => new TSocketPool(['localhost'], $port, false, new StderrLogger()), +}; +$transport = match ($MODE) { + 'framed' => new TFramedTransport($transport), + 'http', 'inline' => $transport, + default => new TBufferedTransport($transport, 1024, 1024), +}; + +$testClient = new \ThriftTest\ThriftTestClient( + $MODE === 'inline' ? $transport : makeProtocol($transport, $PROTO) +); $transport->open(); diff --git a/test/php/TestServer.php b/test/php/TestServer.php index 2238f613844..e765f00f286 100644 --- a/test/php/TestServer.php +++ b/test/php/TestServer.php @@ -40,20 +40,48 @@ function thrift_test_protocol_factory(string $protocol): \Thrift\Factory\TProtoc require_once __DIR__ . '/Handler.php'; $protocolFactory = thrift_test_protocol_factory(getenv('THRIFT_TEST_PROTOCOL') ?: 'binary'); - - $handler = new Handler(); - $processor = new ThriftTest\ThriftTestProcessor($handler); + $processor = new ThriftTest\ThriftTestProcessor(new Handler()); + $requestBody = file_get_contents('php://input'); + + // Peek the message type so oneway calls can return an empty HTTP 200 + // before the handler runs — strict clients (Java) measure oneway latency. + $messageType = null; + try { + $peek = new \Thrift\Transport\TMemoryBuffer($requestBody); + $protocolFactory->getProtocol($peek)->readMessageBegin($name, $messageType, $seqid); + } catch (\Throwable) { + // Let the full processor surface any real parse error below. + } header('Content-Type: application/x-thrift'); - $transport = new \Thrift\Transport\TPhpStream( - \Thrift\Transport\TPhpStream::MODE_R | \Thrift\Transport\TPhpStream::MODE_W - ); - $protocol = $protocolFactory->getProtocol($transport); + if ($messageType === \Thrift\Type\TMessageType::ONEWAY && function_exists('pcntl_fork')) { + header('Content-Length: 0'); + $pid = pcntl_fork(); + if ($pid > 0) { + // Parent: php -S sends the empty 200 as soon as we return. + return true; + } + if ($pid === 0) { + // Child: detach from the cli-server stdio and run the handler async. + fclose(STDIN); + fclose(STDOUT); + fclose(STDERR); + $processor->process( + $protocolFactory->getProtocol(new \Thrift\Transport\TMemoryBuffer($requestBody)), + $protocolFactory->getProtocol(new \Thrift\Transport\TMemoryBuffer()), + ); + exit(0); + } + // pcntl_fork failed; fall through to synchronous handling. + } - $transport->open(); - $processor->process($protocol, $protocol); - $transport->close(); + $output = new \Thrift\Transport\TMemoryBuffer(); + $processor->process( + $protocolFactory->getProtocol(new \Thrift\Transport\TMemoryBuffer($requestBody)), + $protocolFactory->getProtocol($output), + ); + echo $output->getBuffer(); return true; } From 7d7bc070ecbdf096f283249ac1bd1e7de98e34c6 Mon Sep 17 00:00:00 2001 From: Volodymyr Panivko Date: Fri, 22 May 2026 23:26:26 +0200 Subject: [PATCH 09/12] Extract THttpServer to lib/php; split TestServer router into HttpRouter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * lib/php/lib/Server/THttpServer.php (new): single-request Thrift HTTP server. Reads php://input, peeks the message type, and for ONEWAY sends an empty HTTP 200 immediately while running the handler in a forked child. Falls back to synchronous handling if pcntl is missing. Reusable outside the cross-test under any SAPI exposing php://input and php://output (php-fpm, mod_php, cli-server). * test/php/HttpRouter.php (new): minimal cli-server router invoked by `php -S`. Builds the processor + protocol factory and calls THttpServer::serve(). Replaces the SAPI-based branching in TestServer. * test/php/protocols.php (new): shared protocol-name → factory helper consumed by both TestServer.php (socket path) and HttpRouter.php (HTTP path). Uses error_log() rather than STDERR for cross-SAPI safety (STDERR is undefined under cli-server). * TestServer.php: drops the cli-server branch and the inline factory helper; the HTTP launcher now pcntl_exec's php -S with HttpRouter.php as the router script. --- lib/php/lib/Server/THttpServer.php | 130 +++++++++++++++++++++++++++++ test/php/HttpRouter.php | 41 +++++++++ test/php/TestServer.php | 93 ++------------------- test/php/protocols.php | 49 +++++++++++ 4 files changed, 226 insertions(+), 87 deletions(-) create mode 100644 lib/php/lib/Server/THttpServer.php create mode 100644 test/php/HttpRouter.php create mode 100644 test/php/protocols.php diff --git a/lib/php/lib/Server/THttpServer.php b/lib/php/lib/Server/THttpServer.php new file mode 100644 index 00000000000..465a6eeb809 --- /dev/null +++ b/lib/php/lib/Server/THttpServer.php @@ -0,0 +1,130 @@ +contentType); + + if ($this->peekMessageType($requestBody) === TMessageType::ONEWAY + && function_exists('pcntl_fork') + && $this->dispatchOnewayAsync($requestBody) + ) { + return; + } + + $this->dispatchSync($requestBody); + } + + private function peekMessageType(string $body): ?int + { + if ($body === '') { + return null; + } + try { + $type = null; + $name = null; + $seqid = null; + $this->protocolFactory + ->getProtocol(new TMemoryBuffer($body)) + ->readMessageBegin($name, $type, $seqid); + + return $type; + } catch (\Throwable) { + return null; + } + } + + /** + * Send an empty HTTP 200 and fork the handler so the client's one-way + * call returns immediately. Returns true if the parent should exit; + * false if the fork failed and the caller should fall back to sync. + */ + private function dispatchOnewayAsync(string $body): bool + { + header('Content-Length: 0'); + $pid = pcntl_fork(); + if ($pid > 0) { + return true; + } + if ($pid === 0) { + // Detach from SAPI stdio so child output cannot leak into the + // response of a subsequent request handled by the same worker. + if (defined('STDIN')) { + fclose(STDIN); + } + if (defined('STDOUT')) { + fclose(STDOUT); + } + if (defined('STDERR')) { + fclose(STDERR); + } + $this->processor->process( + $this->protocolFactory->getProtocol(new TMemoryBuffer($body)), + $this->protocolFactory->getProtocol(new TMemoryBuffer()), + ); + exit(0); + } + + return false; + } + + private function dispatchSync(string $body): void + { + $output = new TMemoryBuffer(); + $this->processor->process( + $this->protocolFactory->getProtocol(new TMemoryBuffer($body)), + $this->protocolFactory->getProtocol($output), + ); + echo $output->getBuffer(); + } +} diff --git a/test/php/HttpRouter.php b/test/php/HttpRouter.php new file mode 100644 index 00000000000..a46c8980124 --- /dev/null +++ b/test/php/HttpRouter.php @@ -0,0 +1,41 @@ +registerDefinition('ThriftTest', __DIR__ . '/gen-php-classmap'); +$loader->register(); + +$protocolFactory = thrift_test_protocol_factory(getenv('THRIFT_TEST_PROTOCOL') ?: 'binary'); +$processor = new ThriftTest\ThriftTestProcessor(new Handler()); + +(new \Thrift\Server\THttpServer($processor, $protocolFactory))->serve(); diff --git a/test/php/TestServer.php b/test/php/TestServer.php index e765f00f286..ac9542011a5 100644 --- a/test/php/TestServer.php +++ b/test/php/TestServer.php @@ -3,88 +3,7 @@ error_reporting(E_ALL); require_once __DIR__ . '/../../vendor/autoload.php'; - -/** - * Build a protocol factory by name. Shared between the CLI socket server - * and the cli-server HTTP request handler. - */ -function thrift_test_protocol_factory(string $protocol): \Thrift\Factory\TProtocolFactory -{ - switch ($protocol) { - case 'binary': - return new \Thrift\Factory\TBinaryProtocolFactory(false, true); - case 'accel': - if (!function_exists('thrift_protocol_write_binary')) { - fwrite(STDERR, "Acceleration extension is not loaded\n"); - exit(1); - } - return new \Thrift\Factory\TBinaryProtocolAcceleratedFactory(); - case 'compact': - return new \Thrift\Factory\TCompactProtocolFactory(); - case 'json': - return new \Thrift\Factory\TJSONProtocolFactory(); - default: - fwrite(STDERR, "--protocol must be one of {binary|compact|json|accel}\n"); - exit(1); - } -} - -// When invoked under PHP's built-in web server (php -S) we are a per-request -// router: read the Thrift request from php://input, dispatch it through the -// processor, and write the response to php://output. -if (PHP_SAPI === 'cli-server') { - $loader = new \Thrift\ClassLoader\ThriftClassLoader(); - $loader->registerDefinition('ThriftTest', __DIR__ . '/gen-php-classmap'); - $loader->register(); - - require_once __DIR__ . '/Handler.php'; - - $protocolFactory = thrift_test_protocol_factory(getenv('THRIFT_TEST_PROTOCOL') ?: 'binary'); - $processor = new ThriftTest\ThriftTestProcessor(new Handler()); - $requestBody = file_get_contents('php://input'); - - // Peek the message type so oneway calls can return an empty HTTP 200 - // before the handler runs — strict clients (Java) measure oneway latency. - $messageType = null; - try { - $peek = new \Thrift\Transport\TMemoryBuffer($requestBody); - $protocolFactory->getProtocol($peek)->readMessageBegin($name, $messageType, $seqid); - } catch (\Throwable) { - // Let the full processor surface any real parse error below. - } - - header('Content-Type: application/x-thrift'); - - if ($messageType === \Thrift\Type\TMessageType::ONEWAY && function_exists('pcntl_fork')) { - header('Content-Length: 0'); - $pid = pcntl_fork(); - if ($pid > 0) { - // Parent: php -S sends the empty 200 as soon as we return. - return true; - } - if ($pid === 0) { - // Child: detach from the cli-server stdio and run the handler async. - fclose(STDIN); - fclose(STDOUT); - fclose(STDERR); - $processor->process( - $protocolFactory->getProtocol(new \Thrift\Transport\TMemoryBuffer($requestBody)), - $protocolFactory->getProtocol(new \Thrift\Transport\TMemoryBuffer()), - ); - exit(0); - } - // pcntl_fork failed; fall through to synchronous handling. - } - - $output = new \Thrift\Transport\TMemoryBuffer(); - $processor->process( - $protocolFactory->getProtocol(new \Thrift\Transport\TMemoryBuffer($requestBody)), - $protocolFactory->getProtocol($output), - ); - echo $output->getBuffer(); - - return true; -} +require_once __DIR__ . '/protocols.php'; $opts = getopt( 'h::', @@ -129,10 +48,10 @@ function thrift_test_protocol_factory(string $protocol): \Thrift\Factory\TProtoc $transport = $opts['transport'] ?? 'buffered'; $protocol = $opts['protocol'] ?? 'binary'; -// HTTP transport: re-exec under PHP's built-in web server, which TCP-binds -// the port so the cross-runner's readiness probe (socket.connect_ex) succeeds. -// We re-enter this same file as the router; the cli-server branch above -// handles each request. +// HTTP transport: re-exec under PHP's built-in web server with HttpRouter.php +// as the per-request handler. php -S TCP-binds the port so the cross-runner's +// readiness probe (socket.connect_ex) succeeds; the router script runs the +// Thrift HTTP server (see Thrift\Server\THttpServer) for each request. if ($transport === 'http') { if (!function_exists('pcntl_exec')) { fwrite(STDERR, "PHP HTTP cross-test requires ext-pcntl.\n"); @@ -166,7 +85,7 @@ function thrift_test_protocol_factory(string $protocol): \Thrift\Factory\TProtoc $env = getenv(); $env['THRIFT_TEST_PROTOCOL'] = $protocol; - $newArgs = array_merge($phpFlags, ['-S', '127.0.0.1:' . $port, __FILE__]); + $newArgs = array_merge($phpFlags, ['-S', '127.0.0.1:' . $port, __DIR__ . '/HttpRouter.php']); echo "Starting the Test server (HTTP via php -S)...\n"; diff --git a/test/php/protocols.php b/test/php/protocols.php new file mode 100644 index 00000000000..64f2287202b --- /dev/null +++ b/test/php/protocols.php @@ -0,0 +1,49 @@ + Date: Fri, 22 May 2026 23:40:27 +0200 Subject: [PATCH 10/12] Move HTTP request handler to test/php/HttpServer.php The class is test-infrastructure: the pcntl_fork + close-stdio pattern is cli-server specific and does not translate to php-fpm or mod_php (where fastcgi_finish_request() is the proper mechanism). Production PHP-Thrift over HTTP belongs in a framework controller, not this raw helper. Drop the lib/php/ entry and the Thrift\\Server namespace; the class lives next to its only caller (HttpRouter.php) as a global-scoped HttpServer, matching the convention of Handler.php in the same dir. --- test/php/HttpRouter.php | 3 +- .../php/HttpServer.php | 33 ++++++------------- 2 files changed, 12 insertions(+), 24 deletions(-) rename lib/php/lib/Server/THttpServer.php => test/php/HttpServer.php (72%) diff --git a/test/php/HttpRouter.php b/test/php/HttpRouter.php index a46c8980124..48b8f4e4d8a 100644 --- a/test/php/HttpRouter.php +++ b/test/php/HttpRouter.php @@ -29,6 +29,7 @@ require_once __DIR__ . '/../../vendor/autoload.php'; require_once __DIR__ . '/protocols.php'; +require_once __DIR__ . '/HttpServer.php'; require_once __DIR__ . '/Handler.php'; $loader = new \Thrift\ClassLoader\ThriftClassLoader(); @@ -38,4 +39,4 @@ $protocolFactory = thrift_test_protocol_factory(getenv('THRIFT_TEST_PROTOCOL') ?: 'binary'); $processor = new ThriftTest\ThriftTestProcessor(new Handler()); -(new \Thrift\Server\THttpServer($processor, $protocolFactory))->serve(); +(new HttpServer($processor, $protocolFactory))->serve(); diff --git a/lib/php/lib/Server/THttpServer.php b/test/php/HttpServer.php similarity index 72% rename from lib/php/lib/Server/THttpServer.php rename to test/php/HttpServer.php index 465a6eeb809..0e074ba62da 100644 --- a/lib/php/lib/Server/THttpServer.php +++ b/test/php/HttpServer.php @@ -17,35 +17,29 @@ * KIND, either express or implied. See the License for the * specific language governing permissions and limitations * under the License. - * - * @package thrift.server */ declare(strict_types=1); -namespace Thrift\Server; - use Thrift\Factory\TProtocolFactory; use Thrift\Transport\TMemoryBuffer; use Thrift\Type\TMessageType; /** - * Single-request Thrift HTTP server. Invoked once per HTTP request under any - * SAPI that exposes the request body via `php://input` and the response via - * `php://output` — `php -S`, php-fpm, Apache mod_php, etc. + * Cross-test HTTP request handler for PHP's built-in web server (`php -S`). * - * For one-way calls, if `ext-pcntl` is available, an empty HTTP 200 is sent - * immediately and the handler runs in a forked child so the client's one-way - * call returns without waiting for handler execution. + * Reads php://input, peeks the Thrift message type, and for ONEWAY sends an + * empty HTTP 200 immediately while running the handler in a forked child so + * the client's one-way call returns without waiting for handler execution. * - * @package thrift.server + * Cli-server specific: closing STDIN/STDOUT/STDERR in the child relies on + * php -S worker semantics and does not translate to php-fpm or mod_php. */ -class THttpServer +class HttpServer { public function __construct( - protected object $processor, - protected TProtocolFactory $protocolFactory, - protected string $contentType = 'application/x-thrift', + private object $processor, + private TProtocolFactory $protocolFactory, ) { } @@ -53,7 +47,7 @@ public function serve(): void { $requestBody = (string) file_get_contents('php://input'); - header('Content-Type: ' . $this->contentType); + header('Content-Type: application/x-thrift'); if ($this->peekMessageType($requestBody) === TMessageType::ONEWAY && function_exists('pcntl_fork') @@ -84,11 +78,6 @@ private function peekMessageType(string $body): ?int } } - /** - * Send an empty HTTP 200 and fork the handler so the client's one-way - * call returns immediately. Returns true if the parent should exit; - * false if the fork failed and the caller should fall back to sync. - */ private function dispatchOnewayAsync(string $body): bool { header('Content-Length: 0'); @@ -97,8 +86,6 @@ private function dispatchOnewayAsync(string $body): bool return true; } if ($pid === 0) { - // Detach from SAPI stdio so child output cannot leak into the - // response of a subsequent request handled by the same worker. if (defined('STDIN')) { fclose(STDIN); } From 3c2c14b9a290b253e24c35f133afee9c292f7658 Mon Sep 17 00:00:00 2001 From: Volodymyr Panivko Date: Fri, 22 May 2026 23:55:58 +0200 Subject: [PATCH 11/12] HttpRouter: register ThriftClassLoader before requiring Handler.php ThriftTest generated classes are not in Composer's PSR-4 map, so the classmap loader must be registered before Handler.php is required; otherwise `class Handler implements ThriftTestIf` fails at autoload and php -S falls back to a default response that confuses cross clients. --- test/php/HttpRouter.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/php/HttpRouter.php b/test/php/HttpRouter.php index 48b8f4e4d8a..8b7299cdb3c 100644 --- a/test/php/HttpRouter.php +++ b/test/php/HttpRouter.php @@ -30,12 +30,16 @@ require_once __DIR__ . '/../../vendor/autoload.php'; require_once __DIR__ . '/protocols.php'; require_once __DIR__ . '/HttpServer.php'; -require_once __DIR__ . '/Handler.php'; +// ThriftTest generated classes are not in Composer's PSR-4 map; register the +// classmap loader before requiring Handler.php so its `implements ThriftTestIf` +// resolves correctly. $loader = new \Thrift\ClassLoader\ThriftClassLoader(); $loader->registerDefinition('ThriftTest', __DIR__ . '/gen-php-classmap'); $loader->register(); +require_once __DIR__ . '/Handler.php'; + $protocolFactory = thrift_test_protocol_factory(getenv('THRIFT_TEST_PROTOCOL') ?: 'binary'); $processor = new ThriftTest\ThriftTestProcessor(new Handler()); From b3d3f6c12b1a769bf2338578327be4053e7c1b93 Mon Sep 17 00:00:00 2001 From: Volodymyr Panivko Date: Sat, 23 May 2026 00:23:08 +0200 Subject: [PATCH 12/12] Re-add php-cpp HTTP known failures pending THRIFT-6021 (#3514) The cpp HTTP client cannot drain the empty oneway response that THttpServer-style servers (including our HttpServer.php) emit; per the maintainer note on #3515, THRIFT-6021 / #3514 fixes the cpp client side. Until that lands, these 4 combinations stay in known_failures to keep the matrix green; can be removed once #3514 is merged. --- test/known_failures_Linux.json | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/known_failures_Linux.json b/test/known_failures_Linux.json index ccdb5de776d..8f4e0ee2739 100644 --- a/test/known_failures_Linux.json +++ b/test/known_failures_Linux.json @@ -648,6 +648,10 @@ "perl-netstd_multi-binary_buffered-ip-ssl", "perl-netstd_multi-binary_framed-ip", "perl-netstd_multi-binary_framed-ip-ssl", + "php-cpp_accel-binary_http-ip", + "php-cpp_binary_http-ip", + "php-cpp_compact_http-ip", + "php-cpp_json_http-ip", "py-cpp_accel-binary_http-domain", "py-cpp_accel-binary_http-ip", "py-cpp_accel-binary_http-ip-ssl",