From 9a9772f42891641b01f90ebca96486fcc58d7c73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ha=C5=82as?= Date: Sat, 2 May 2026 22:28:05 +0200 Subject: [PATCH 1/7] fix: detect HTTPS from SSL transport instead of X-Forwarded-Proto header (#152) --- src/DTO/RequestConverter.php | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/DTO/RequestConverter.php b/src/DTO/RequestConverter.php index a71c05b..bd1cb35 100644 --- a/src/DTO/RequestConverter.php +++ b/src/DTO/RequestConverter.php @@ -36,10 +36,8 @@ public static function toSymfonyRequest(\Workerman\Protocols\Http\Request $rawRe // Fallback to 127.0.0.1:0 for unit test scenarios where connection is null. // In production, connection should always be present. - // Detect HTTPS from connection port or X-Forwarded-Proto header - $localPort = $rawRequest->connection?->getLocalPort(); - $forwardedProto = strtolower((string) $rawRequest->header('x-forwarded-proto', '')); - $isHttps = $localPort === 443 || $forwardedProto === 'https'; + // Detect HTTPS from Workerman's SSL transport (configured via https:// listen address) + $isHttps = ($rawRequest->connection?->transport ?? 'tcp') === 'ssl'; $requestTimeFloat = microtime(true); $server = [ @@ -48,7 +46,7 @@ public static function toSymfonyRequest(\Workerman\Protocols\Http\Request $rawRe 'SERVER_PROTOCOL' => 'HTTP/' . $rawRequest->protocolVersion(), 'REMOTE_ADDR' => $rawRequest->connection?->getRemoteIp() ?? '127.0.0.1', 'REMOTE_PORT' => $rawRequest->connection?->getRemotePort() ?? 0, - 'SERVER_PORT' => $localPort ?? ($isHttps ? 443 : 80), + 'SERVER_PORT' => $rawRequest->connection?->getLocalPort() ?? ($isHttps ? 443 : 80), 'SERVER_NAME' => $rawRequest->connection?->getLocalIp() ?? 'localhost', 'QUERY_STRING' => $rawRequest->queryString(), 'REQUEST_TIME' => (int) $requestTimeFloat, From 002766b74cb32eec0444e8d8aaebf10a0bf5acc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ha=C5=82as?= Date: Sat, 2 May 2026 22:31:49 +0200 Subject: [PATCH 2/7] test: update RequestConverter tests for SSL transport detection (#152) --- tests/RequestConverterTest.php | 48 +++++++++++++++++++++++++++------- 1 file changed, 38 insertions(+), 10 deletions(-) diff --git a/tests/RequestConverterTest.php b/tests/RequestConverterTest.php index 0cad1fc..7e44ab8 100644 --- a/tests/RequestConverterTest.php +++ b/tests/RequestConverterTest.php @@ -447,27 +447,41 @@ public function testGetPortReturnsPortFromHostHeaderWhenPresent(): void $this->assertSame(8080, $symfonyRequest->getPort()); } - public function testServerPortDefaultsTo443WhenNoConnectionButHttpsForwarded(): void + public function testHttpsDetectedFromSslTransport(): void { - $buffer = "GET /test HTTP/1.1\r\nHost: localhost\r\nX-Forwarded-Proto: https\r\n\r\n"; + $buffer = "GET /test HTTP/1.1\r\nHost: example.com\r\n\r\n"; $rawRequest = new Request($buffer); - $rawRequest->connection = null; + $rawRequest->connection = $this->createMockConnection(443, 'ssl'); $symfonyRequest = RequestConverter::toSymfonyRequest($rawRequest); $this->assertSame(443, $symfonyRequest->server->get('SERVER_PORT')); $this->assertSame('on', $symfonyRequest->server->get('HTTPS')); + $this->assertSame('https', $symfonyRequest->getScheme()); } - public function testHttpsDetectedFromPort443(): void + public function testHttpDetectedFromTcpTransport(): void { $buffer = "GET /test HTTP/1.1\r\nHost: example.com\r\n\r\n"; $rawRequest = new Request($buffer); - $rawRequest->connection = $this->createMockConnection(443); + $rawRequest->connection = $this->createMockConnection(80, 'tcp'); $symfonyRequest = RequestConverter::toSymfonyRequest($rawRequest); - $this->assertSame(443, $symfonyRequest->server->get('SERVER_PORT')); + $this->assertSame(80, $symfonyRequest->server->get('SERVER_PORT')); + $this->assertNull($symfonyRequest->server->get('HTTPS')); + $this->assertSame('http', $symfonyRequest->getScheme()); + } + + public function testHttpsDetectedFromSslTransportOnAnyPort(): void + { + $buffer = "GET /test HTTP/1.1\r\nHost: example.com\r\n\r\n"; + $rawRequest = new Request($buffer); + $rawRequest->connection = $this->createMockConnection(8443, 'ssl'); + + $symfonyRequest = RequestConverter::toSymfonyRequest($rawRequest); + + $this->assertSame(8443, $symfonyRequest->server->get('SERVER_PORT')); $this->assertSame('on', $symfonyRequest->server->get('HTTPS')); $this->assertSame('https', $symfonyRequest->getScheme()); } @@ -476,13 +490,26 @@ public function testGetSchemeAndHttpHostOmitsPort443ForHttps(): void { $buffer = "GET /test HTTP/1.1\r\nHost: example.com\r\n\r\n"; $rawRequest = new Request($buffer); - $rawRequest->connection = $this->createMockConnection(443); + $rawRequest->connection = $this->createMockConnection(443, 'ssl'); $symfonyRequest = RequestConverter::toSymfonyRequest($rawRequest); $this->assertSame('https://example.com', $symfonyRequest->getSchemeAndHttpHost()); } + public function testXForwardedProtoIgnoredOnTcpTransport(): void + { + $buffer = "GET /test HTTP/1.1\r\nHost: localhost\r\nX-Forwarded-Proto: https\r\n\r\n"; + $rawRequest = new Request($buffer); + $rawRequest->connection = $this->createMockConnection(80, 'tcp'); + + $symfonyRequest = RequestConverter::toSymfonyRequest($rawRequest); + + $this->assertNull($symfonyRequest->server->get('HTTPS')); + $this->assertFalse($symfonyRequest->isSecure()); + $this->assertSame('http', $symfonyRequest->getScheme()); + } + public function testRequestTimeAndRequestTimeFloatAreSet(): void { $buffer = "GET /test HTTP/1.1\r\nHost: localhost\r\n\r\n"; @@ -552,12 +579,13 @@ public function testFormUrlEncodedPreservesContent(): void $this->assertSame('key=value', $symfonyRequest->getContent()); } - private function createMockConnection(int $localPort): \Workerman\Connection\TcpConnection + private function createMockConnection(int $localPort, string $transport = 'tcp'): \Workerman\Connection\TcpConnection { - return new class ($localPort) extends \Workerman\Connection\TcpConnection { - public function __construct(private readonly int $port) + return new class ($localPort, $transport) extends \Workerman\Connection\TcpConnection { + public function __construct(private readonly int $port, string $transport) { $this->remoteAddress = '192.168.1.1:12345'; + $this->transport = $transport; } public function getLocalPort(): int From da1e33441e2167117f14a3c8002204f061f62c33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ha=C5=82as?= Date: Sat, 2 May 2026 22:35:40 +0200 Subject: [PATCH 3/7] test: update E2E tests for SSL transport HTTPS detection (#152) --- tests/SymfonyControllerTest.php | 50 ++++++++------------------------- 1 file changed, 11 insertions(+), 39 deletions(-) diff --git a/tests/SymfonyControllerTest.php b/tests/SymfonyControllerTest.php index d157b69..8559913 100644 --- a/tests/SymfonyControllerTest.php +++ b/tests/SymfonyControllerTest.php @@ -1088,41 +1088,14 @@ public function testHttpsE2E(): void // Create a mock connection with SSL port (443) $buffer = "GET /secure HTTP/1.1\r\nHost: localhost\r\n\r\n"; $request = new Request($buffer); - $request->connection = $this->createMockConnection(443); + $request->connection = $this->createMockConnection(443, 'ssl'); $controller($request, $this->connection); $this->assertNotNull($kernel->receivedRequest); $symfonyRequest = $kernel->receivedRequest; - // HTTPS should be detected from port 443 - $this->assertSame('on', $symfonyRequest->server->get('HTTPS')); - $this->assertTrue($symfonyRequest->isSecure()); - $this->assertSame('https', $symfonyRequest->getScheme()); - $this->assertSame(443, $symfonyRequest->getPort()); - } - - public function testXForwardedProtoHttpsE2E(): void - { - // E2E test: Verify X-Forwarded-Proto header is detected (#64) - $symfonyResponse = new SymfonyResponse('proxied'); - $kernel = new TestRequestTrackingKernel($symfonyResponse); - $responseConverter = $this->createResponseConverter(); - - $controller = new SymfonyController($kernel, $responseConverter); - - // Create request with X-Forwarded-Proto header (connection port is regular 80) - $buffer = "GET /proxied HTTP/1.1\r\nHost: localhost\r\nX-Forwarded-Proto: https\r\n\r\n"; - $request = new Request($buffer); - $request->connection = $this->createMockConnection(80); - - $controller($request, $this->connection); - - $this->assertNotNull($kernel->receivedRequest); - $symfonyRequest = $kernel->receivedRequest; - - // HTTPS should be detected from X-Forwarded-Proto header - // Note: when isHttps is true, SERVER_PORT defaults to 443 + // HTTPS should be detected from SSL transport $this->assertSame('on', $symfonyRequest->server->get('HTTPS')); $this->assertTrue($symfonyRequest->isSecure()); $this->assertSame('https', $symfonyRequest->getScheme()); @@ -1155,37 +1128,36 @@ public function testHttpE2E(): void $this->assertSame(80, $symfonyRequest->getPort()); } - public function testXForwardedProtoCaseInsensitiveE2E(): void + public function testHttpsE2EWithSslTransport(): void { - // E2E test: Verify X-Forwarded-Proto works with uppercase values (#64) - $symfonyResponse = new SymfonyResponse('proxied'); + $symfonyResponse = new SymfonyResponse('secure'); $kernel = new TestRequestTrackingKernel($symfonyResponse); $responseConverter = $this->createResponseConverter(); $controller = new SymfonyController($kernel, $responseConverter); - // Test with uppercase HTTPS value - $buffer = "GET /proxied HTTP/1.1\r\nHost: localhost\r\nX-Forwarded-Proto: HTTPS\r\n\r\n"; + $buffer = "GET /secure HTTP/1.1\r\nHost: localhost\r\n\r\n"; $request = new Request($buffer); - $request->connection = $this->createMockConnection(80); + $request->connection = $this->createMockConnection(443, 'ssl'); $controller($request, $this->connection); $this->assertNotNull($kernel->receivedRequest); $symfonyRequest = $kernel->receivedRequest; - // HTTPS should be detected from uppercase X-Forwarded-Proto header $this->assertSame('on', $symfonyRequest->server->get('HTTPS')); $this->assertTrue($symfonyRequest->isSecure()); $this->assertSame('https', $symfonyRequest->getScheme()); + $this->assertSame(443, $symfonyRequest->getPort()); } - private function createMockConnection(int $port): \Workerman\Connection\TcpConnection + private function createMockConnection(int $port, string $transport = 'tcp'): \Workerman\Connection\TcpConnection { - return new class ($port) extends \Workerman\Connection\TcpConnection { - public function __construct(private readonly int $port) + return new class ($port, $transport) extends \Workerman\Connection\TcpConnection { + public function __construct(private readonly int $port, string $transport) { $this->remoteAddress = '192.168.1.1:12345'; + $this->transport = $transport; } public function getLocalPort(): int From 6b91ef26ae511d3b58703b8c6216d4a1d2c0437f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ha=C5=82as?= Date: Sat, 2 May 2026 22:39:50 +0200 Subject: [PATCH 4/7] test: differentiate testHttpsE2E from testHttpsE2EWithSslTransport --- tests/SymfonyControllerTest.php | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/SymfonyControllerTest.php b/tests/SymfonyControllerTest.php index 8559913..03f0484 100644 --- a/tests/SymfonyControllerTest.php +++ b/tests/SymfonyControllerTest.php @@ -1078,28 +1078,28 @@ public function testStreamedJsonResponseE2E(): void public function testHttpsE2E(): void { - // E2E test: Verify HTTPS detection works through full stack (#64) - $symfonyResponse = new SymfonyResponse('secure'); + // E2E test: Verify port 443 without SSL transport is HTTP (#64) + $symfonyResponse = new SymfonyResponse('plain'); $kernel = new TestRequestTrackingKernel($symfonyResponse); $responseConverter = $this->createResponseConverter(); $controller = new SymfonyController($kernel, $responseConverter); - // Create a mock connection with SSL port (443) - $buffer = "GET /secure HTTP/1.1\r\nHost: localhost\r\n\r\n"; + // Port 443 alone should NOT imply HTTPS — must use SSL transport + $buffer = "GET /plain HTTP/1.1\r\nHost: localhost\r\n\r\n"; $request = new Request($buffer); - $request->connection = $this->createMockConnection(443, 'ssl'); + $request->connection = $this->createMockConnection(443); $controller($request, $this->connection); $this->assertNotNull($kernel->receivedRequest); $symfonyRequest = $kernel->receivedRequest; - // HTTPS should be detected from SSL transport - $this->assertSame('on', $symfonyRequest->server->get('HTTPS')); - $this->assertTrue($symfonyRequest->isSecure()); - $this->assertSame('https', $symfonyRequest->getScheme()); - $this->assertSame(443, $symfonyRequest->getPort()); + // Port 443 without SSL transport = HTTP (Symfony defaults to 80 for HTTP scheme) + $this->assertNull($symfonyRequest->server->get('HTTPS')); + $this->assertFalse($symfonyRequest->isSecure()); + $this->assertSame('http', $symfonyRequest->getScheme()); + $this->assertSame(80, $symfonyRequest->getPort()); } public function testHttpE2E(): void From b4e1c0423a80ba0809a4cf8c70ed6c1cdd0201d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ha=C5=82as?= Date: Sat, 2 May 2026 22:40:30 +0200 Subject: [PATCH 5/7] docs: add changelog entry for X-Forwarded-Proto fix (#152) --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ebfdf5..4bb5bed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,6 +62,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Throws `\RuntimeException` with clear message when directory creation fails - Double `is_dir()` check handles race condition between check and `mkdir()` call +- Security: RequestConverter no longer trusts X-Forwarded-Proto header + unconditionally. HTTPS is now detected only from the actual SSL transport + layer. Users behind reverse proxies must configure Symfony's trusted + proxies. ([#152](https://github.com/crazy-goat/workerman-bundle/issues/152)) + ## [0.14.0] - 2026-04-14 ### Deprecated From 2450e03c69486f9442691821ac754546e9bea923 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ha=C5=82as?= Date: Sat, 2 May 2026 22:44:06 +0200 Subject: [PATCH 6/7] chore: remove stale comment in RequestConverter --- src/DTO/RequestConverter.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/DTO/RequestConverter.php b/src/DTO/RequestConverter.php index bd1cb35..f32ba0e 100644 --- a/src/DTO/RequestConverter.php +++ b/src/DTO/RequestConverter.php @@ -33,8 +33,6 @@ public static function toSymfonyRequest(\Workerman\Protocols\Http\Request $rawRe // Build server bag with HTTP_* headers (CGI convention) $headers = $rawRequest->header() ?? []; - // Fallback to 127.0.0.1:0 for unit test scenarios where connection is null. - // In production, connection should always be present. // Detect HTTPS from Workerman's SSL transport (configured via https:// listen address) $isHttps = ($rawRequest->connection?->transport ?? 'tcp') === 'ssl'; From 12395893ca19cacd4be1bd5d1620e6668a67d37a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ha=C5=82as?= Date: Sat, 2 May 2026 22:54:06 +0200 Subject: [PATCH 7/7] fix: avoid redundant nullsafe property access in HTTPS detection --- src/DTO/RequestConverter.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/DTO/RequestConverter.php b/src/DTO/RequestConverter.php index f32ba0e..d92bc54 100644 --- a/src/DTO/RequestConverter.php +++ b/src/DTO/RequestConverter.php @@ -35,7 +35,7 @@ public static function toSymfonyRequest(\Workerman\Protocols\Http\Request $rawRe $headers = $rawRequest->header() ?? []; // Detect HTTPS from Workerman's SSL transport (configured via https:// listen address) - $isHttps = ($rawRequest->connection?->transport ?? 'tcp') === 'ssl'; + $isHttps = isset($rawRequest->connection) && $rawRequest->connection->transport === 'ssl'; $requestTimeFloat = microtime(true); $server = [