Skip to content

fix: stop trusting X-Forwarded-Proto header unconditionally (#152) #176

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
s2x merged 7 commits into from
May 2, 2026
Merged

fix: stop trusting X-Forwarded-Proto header unconditionally (#152) #176

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9a9772f
fix: detect HTTPS from SSL transport instead of X-Forwarded-Proto hea…
May 2, 2026
002766b
test: update RequestConverter tests for SSL transport detection (#152)
May 2, 2026
da1e334
test: update E2E tests for SSL transport HTTPS detection (#152)
May 2, 2026
6b91ef2
test: differentiate testHttpsE2E from testHttpsE2EWithSslTransport
May 2, 2026
b4e1c04
docs: add changelog entry for X-Forwarded-Proto fix (#152)
May 2, 2026
2450e03
chore: remove stale comment in RequestConverter
May 2, 2026
1239589
fix: avoid redundant nullsafe property access in HTTPS detection
May 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 3 additions & 7 deletions src/DTO/RequestConverter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,9 @@ 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 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 = isset($rawRequest->connection) && $rawRequest->connection->transport === 'ssl';

$requestTimeFloat = microtime(true);
$server = [
Expand All @@ -48,7 +44,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,
Expand Down
48 changes: 38 additions & 10 deletions tests/RequestConverterTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand All @@ -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";
Expand Down Expand Up @@ -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
Expand Down
64 changes: 18 additions & 46 deletions tests/SymfonyControllerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1078,15 +1078,15 @@ 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);

Expand All @@ -1095,38 +1095,11 @@ public function testHttpsE2E(): void
$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
$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
Expand Down Expand Up @@ -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
Expand Down