From 957b8cd0686e84121665d914fe259394b4e158e6 Mon Sep 17 00:00:00 2001 From: barbosa89 Date: Tue, 9 Dec 2025 07:50:57 -0500 Subject: [PATCH 1/7] refactor: rename IpAddress to Ip --- src/Auth/Middlewares/Authenticated.php | 4 ++-- src/Auth/Middlewares/TokenRateLimit.php | 4 ++-- src/Cache/RateLimit/Middlewares/RateLimiter.php | 4 ++-- src/Http/{IpAddress.php => Ip.php} | 4 ++-- src/Http/Request.php | 2 +- tests/Unit/Http/IpAddressTest.php | 4 ++-- 6 files changed, 11 insertions(+), 11 deletions(-) rename src/Http/{IpAddress.php => Ip.php} (92%) diff --git a/src/Auth/Middlewares/Authenticated.php b/src/Auth/Middlewares/Authenticated.php index c75c82de..badbef59 100644 --- a/src/Auth/Middlewares/Authenticated.php +++ b/src/Auth/Middlewares/Authenticated.php @@ -16,7 +16,7 @@ use Phenix\Facades\Config; use Phenix\Facades\Event; use Phenix\Http\Constants\HttpStatus; -use Phenix\Http\IpAddress; +use Phenix\Http\Ip; use Phenix\Http\Request as HttpRequest; class Authenticated implements Middleware @@ -34,7 +34,7 @@ public function handleRequest(Request $request, RequestHandler $next): Response /** @var AuthenticationManager $auth */ $auth = App::make(AuthenticationManager::class); - $clientIp = IpAddress::hash($request); + $clientIp = Ip::hash($request); if (! $token || ! $auth->validate($token)) { Event::emitAsync(new FailedTokenValidation( diff --git a/src/Auth/Middlewares/TokenRateLimit.php b/src/Auth/Middlewares/TokenRateLimit.php index 1e423277..8190f70c 100644 --- a/src/Auth/Middlewares/TokenRateLimit.php +++ b/src/Auth/Middlewares/TokenRateLimit.php @@ -12,7 +12,7 @@ use Phenix\Auth\AuthenticationManager; use Phenix\Facades\Config; use Phenix\Http\Constants\HttpStatus; -use Phenix\Http\IpAddress; +use Phenix\Http\Ip; use function str_starts_with; @@ -29,7 +29,7 @@ public function handleRequest(Request $request, RequestHandler $next): Response /** @var AuthenticationManager $auth */ $auth = App::make(AuthenticationManager::class); - $clientIp = IpAddress::hash($request); + $clientIp = Ip::hash($request); $attemptLimit = (int) (Config::get('auth.tokens.rate_limit.attempts', 5)); $windowSeconds = (int) (Config::get('auth.tokens.rate_limit.window', 300)); diff --git a/src/Cache/RateLimit/Middlewares/RateLimiter.php b/src/Cache/RateLimit/Middlewares/RateLimiter.php index 3e9a3964..b2c67f07 100644 --- a/src/Cache/RateLimit/Middlewares/RateLimiter.php +++ b/src/Cache/RateLimit/Middlewares/RateLimiter.php @@ -12,7 +12,7 @@ use Phenix\Cache\RateLimit\RateLimitManager; use Phenix\Facades\Config; use Phenix\Http\Constants\HttpStatus; -use Phenix\Http\IpAddress; +use Phenix\Http\Ip; class RateLimiter implements Middleware { @@ -29,7 +29,7 @@ public function handleRequest(Request $request, RequestHandler $next): Response return $next->handleRequest($request); } - $clientIp = IpAddress::hash($request); + $clientIp = Ip::hash($request); $current = $this->rateLimiter->increment($clientIp); $perMinuteLimit = (int) Config::get('cache.rate_limit.per_minute', 60); diff --git a/src/Http/IpAddress.php b/src/Http/Ip.php similarity index 92% rename from src/Http/IpAddress.php rename to src/Http/Ip.php index 5598ca96..68b22768 100644 --- a/src/Http/IpAddress.php +++ b/src/Http/Ip.php @@ -6,7 +6,7 @@ use Amp\Http\Server\Request; -final class IpAddress +final class Ip { private function __construct() { @@ -15,7 +15,7 @@ private function __construct() public static function parse(Request $request): string { - $xff = $request->getHeader('X-Forwarded-For'); + $xff = $request->getHeader('X-Forwarded-For'); // TODO: Review trusted proxies handling if ($xff && $ip = self::getFromHeader($xff)) { return $ip; diff --git a/src/Http/Request.php b/src/Http/Request.php index fcf8be2b..c973df46 100644 --- a/src/Http/Request.php +++ b/src/Http/Request.php @@ -149,7 +149,7 @@ public function session(string|null $key = null, array|string|int|null $default public function ip(): string|null { - return IpAddress::parse($this->request); + return Ip::parse($this->request); } public function toArray(): array diff --git a/tests/Unit/Http/IpAddressTest.php b/tests/Unit/Http/IpAddressTest.php index 22b2a18c..3f373385 100644 --- a/tests/Unit/Http/IpAddressTest.php +++ b/tests/Unit/Http/IpAddressTest.php @@ -6,7 +6,7 @@ use Amp\Http\Server\Request as ServerRequest; use League\Uri\Http; use Phenix\Http\Constants\HttpMethod; -use Phenix\Http\IpAddress; +use Phenix\Http\Ip; use Phenix\Util\URL; it('generate ip hash from request', function (string $ip, $expected): void { @@ -16,7 +16,7 @@ $request->setHeader('X-Forwarded-For', $ip); - expect(IpAddress::hash($request))->toBe($expected); + expect(Ip::hash($request))->toBe($expected); })->with([ ['192.168.1.1', hash('sha256', '192.168.1.1')], ['192.168.1.1:8080', hash('sha256', '192.168.1.1')], From b87b465c848867ce5b64c46c6120fb43a490fcba Mon Sep 17 00:00:00 2001 From: barbosa89 Date: Tue, 9 Dec 2025 12:18:34 -0500 Subject: [PATCH 2/7] refactor: update ip method to return Ip instance instead of nullable string --- src/Http/Request.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Http/Request.php b/src/Http/Request.php index c973df46..82042965 100644 --- a/src/Http/Request.php +++ b/src/Http/Request.php @@ -147,9 +147,9 @@ public function session(string|null $key = null, array|string|int|null $default return $this->session; } - public function ip(): string|null + public function ip(): Ip { - return Ip::parse($this->request); + return Ip::make($this->request); } public function toArray(): array From fe4e4db9a5077af2c7dbd6d142892acc2177b37e Mon Sep 17 00:00:00 2001 From: barbosa89 Date: Tue, 9 Dec 2025 13:16:29 -0500 Subject: [PATCH 3/7] refactor: update client IP retrieval to use Ip::make() method --- src/Auth/Middlewares/Authenticated.php | 2 +- src/Auth/Middlewares/TokenRateLimit.php | 2 +- src/Cache/RateLimit/Middlewares/RateLimiter.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Auth/Middlewares/Authenticated.php b/src/Auth/Middlewares/Authenticated.php index badbef59..8fec53be 100644 --- a/src/Auth/Middlewares/Authenticated.php +++ b/src/Auth/Middlewares/Authenticated.php @@ -34,7 +34,7 @@ public function handleRequest(Request $request, RequestHandler $next): Response /** @var AuthenticationManager $auth */ $auth = App::make(AuthenticationManager::class); - $clientIp = Ip::hash($request); + $clientIp = Ip::make($request)->hash(); if (! $token || ! $auth->validate($token)) { Event::emitAsync(new FailedTokenValidation( diff --git a/src/Auth/Middlewares/TokenRateLimit.php b/src/Auth/Middlewares/TokenRateLimit.php index 8190f70c..86e59a75 100644 --- a/src/Auth/Middlewares/TokenRateLimit.php +++ b/src/Auth/Middlewares/TokenRateLimit.php @@ -29,7 +29,7 @@ public function handleRequest(Request $request, RequestHandler $next): Response /** @var AuthenticationManager $auth */ $auth = App::make(AuthenticationManager::class); - $clientIp = Ip::hash($request); + $clientIp = Ip::make($request)->hash(); $attemptLimit = (int) (Config::get('auth.tokens.rate_limit.attempts', 5)); $windowSeconds = (int) (Config::get('auth.tokens.rate_limit.window', 300)); diff --git a/src/Cache/RateLimit/Middlewares/RateLimiter.php b/src/Cache/RateLimit/Middlewares/RateLimiter.php index b2c67f07..d39433a1 100644 --- a/src/Cache/RateLimit/Middlewares/RateLimiter.php +++ b/src/Cache/RateLimit/Middlewares/RateLimiter.php @@ -29,7 +29,7 @@ public function handleRequest(Request $request, RequestHandler $next): Response return $next->handleRequest($request); } - $clientIp = Ip::hash($request); + $clientIp = Ip::make($request)->hash(); $current = $this->rateLimiter->increment($clientIp); $perMinuteLimit = (int) Config::get('cache.rate_limit.per_minute', 60); From ac3bf056eda55cd547d3d60c9d81726074451d0f Mon Sep 17 00:00:00 2001 From: barbosa89 Date: Tue, 9 Dec 2025 13:25:05 -0500 Subject: [PATCH 4/7] refactor: enhance Ip class structure and improve address parsing logic --- src/Http/Ip.php | 92 +++++++++++++++++++++---------- tests/Unit/Http/IpAddressTest.php | 35 ++++++++++-- 2 files changed, 95 insertions(+), 32 deletions(-) diff --git a/src/Http/Ip.php b/src/Http/Ip.php index 68b22768..22a031cf 100644 --- a/src/Http/Ip.php +++ b/src/Http/Ip.php @@ -6,61 +6,97 @@ use Amp\Http\Server\Request; -final class Ip +class Ip { - private function __construct() + protected string $address; + + protected string $host; + + protected int|null $port = null; + + protected array $forwardingAddresses = []; + + public function __construct(Request $request) { - // Prevent instantiation + $this->address = $request->getClient()->getRemoteAddress()->toString(); + + if ($forwardingHeader = $request->getHeader('X-Forwarded-For')) { + $parts = array_map(static fn ($v) => trim($v), explode(',', $forwardingHeader)); + $this->forwardingAddresses = $parts; + } } - public static function parse(Request $request): string + public static function make(Request $request): self { - $xff = $request->getHeader('X-Forwarded-For'); // TODO: Review trusted proxies handling + $ip = new self($request); + $ip->parse(); - if ($xff && $ip = self::getFromHeader($xff)) { - return $ip; - } + return $ip; + } - return (string) $request->getClient()->getRemoteAddress(); + public function address(): string + { + return $this->address; } - public static function hash(Request $request): string + public function host(): string { - $ip = self::parse($request); + return $this->host; + } - $normalized = self::normalize($ip); + public function port(): int|null + { + return $this->port; + } - return hash('sha256', $normalized); + public function isForwarded(): bool + { + return ! empty($this->forwardingAddresses); } - private static function getFromHeader(string $header): string + public function forwardingAddresses(): array { - $parts = explode(',', $header)[0] ?? ''; + return $this->forwardingAddresses; + } - return trim($parts); + public function hash(): string + { + return hash('sha256', $this->host); } - private static function normalize(string $ip): string + protected function parse(): void { - if (preg_match('/^\[(?[^\]]+)\](?::\d+)?$/', $ip, $m) === 1) { - return $m['addr']; + $address = trim($this->address); + + if (preg_match('/^\[(?[^\]]+)\](?::(?\d+))?$/', $address, $m) === 1) { + $this->host = $m['addr']; + $this->port = isset($m['port']) ? (int) $m['port'] : null; + + return; } - $normalized = $ip; + if (filter_var($address, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6)) { + $this->host = $address; + $this->port = null; - if (filter_var($normalized, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6)) { - return $normalized; + return; } - if (str_contains($normalized, ':')) { - $parts = explode(':', $normalized); - $maybeIpv4 = $parts[0]; + if (str_contains($address, ':')) { + [$maybeHost, $maybePort] = explode(':', $address, 2); + + if ( + filter_var($maybeHost, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4) || + filter_var($maybeHost, FILTER_VALIDATE_DOMAIN, FILTER_FLAG_HOSTNAME) + ) { + $this->host = $maybeHost; + $this->port = is_numeric($maybePort) ? (int) $maybePort : null; - if (filter_var($maybeIpv4, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4)) { - $normalized = $maybeIpv4; + return; } } - return $normalized; + $this->host = $address; + $this->port = null; } } diff --git a/tests/Unit/Http/IpAddressTest.php b/tests/Unit/Http/IpAddressTest.php index 3f373385..f239198b 100644 --- a/tests/Unit/Http/IpAddressTest.php +++ b/tests/Unit/Http/IpAddressTest.php @@ -4,6 +4,8 @@ use Amp\Http\Server\Driver\Client; use Amp\Http\Server\Request as ServerRequest; +use Amp\Socket\SocketAddress; +use Amp\Socket\SocketAddressType; use League\Uri\Http; use Phenix\Http\Constants\HttpMethod; use Phenix\Http\Ip; @@ -11,12 +13,37 @@ it('generate ip hash from request', function (string $ip, $expected): void { $client = $this->createMock(Client::class); + $client->method('getRemoteAddress')->willReturn( + new class ($ip) implements SocketAddress { + public function __construct(private string $address) + { + } + + public function toString(): string + { + return $this->address; + } + + public function getType(): SocketAddressType + { + return SocketAddressType::Internet; + } + + public function __toString(): string + { + return $this->address; + } + } + ); + $uri = Http::new(URL::build('posts/7/comments/22')); $request = new ServerRequest($client, HttpMethod::GET->value, $uri); - $request->setHeader('X-Forwarded-For', $ip); + $ip = Ip::make($request); - expect(Ip::hash($request))->toBe($expected); + expect($ip->hash())->toBe($expected); + expect($ip->isForwarded())->toBeFalse(); + expect($ip->forwardingAddresses())->toBe([]); })->with([ ['192.168.1.1', hash('sha256', '192.168.1.1')], ['192.168.1.1:8080', hash('sha256', '192.168.1.1')], @@ -25,7 +52,7 @@ ['[2001:db8::1]:443', hash('sha256', '2001:db8::1')], ['::1', hash('sha256', '::1')], ['2001:db8::7334', hash('sha256', '2001:db8::7334')], - ['203.0.113.1, 198.51.100.2', hash('sha256', '203.0.113.1')], - [' 192.168.0.1:8080 , 10.0.0.2', hash('sha256', '192.168.0.1')], + ['203.0.113.1', hash('sha256', '203.0.113.1')], + [' 192.168.0.1:8080', hash('sha256', '192.168.0.1')], ['::ffff:192.168.0.1', hash('sha256', '::ffff:192.168.0.1')], ]); From 417918f75e7bd2d13ec1df0922b5d9d9926852c4 Mon Sep 17 00:00:00 2001 From: barbosa89 Date: Tue, 9 Dec 2025 14:01:11 -0500 Subject: [PATCH 5/7] test: add comprehensive tests for IP address parsing and forwarding logic --- tests/Unit/Http/IpAddressTest.php | 196 ++++++++++++++++++++++++++++++ 1 file changed, 196 insertions(+) diff --git a/tests/Unit/Http/IpAddressTest.php b/tests/Unit/Http/IpAddressTest.php index f239198b..7009f157 100644 --- a/tests/Unit/Http/IpAddressTest.php +++ b/tests/Unit/Http/IpAddressTest.php @@ -56,3 +56,199 @@ public function __toString(): string [' 192.168.0.1:8080', hash('sha256', '192.168.0.1')], ['::ffff:192.168.0.1', hash('sha256', '::ffff:192.168.0.1')], ]); + +it('parses host and port from remote address (IPv6 bracket + port)', function (): void { + $client = $this->createMock(Client::class); + $client->method('getRemoteAddress')->willReturn( + new class ('[2001:db8::1]:443') implements SocketAddress { + public function __construct(private string $address) + { + } + + public function toString(): string + { + return $this->address; + } + + public function getType(): SocketAddressType + { + return SocketAddressType::Internet; + } + + public function __toString(): string + { + return $this->address; + } + } + ); + + $request = new ServerRequest($client, HttpMethod::GET->value, Http::new(URL::build('/'))); + $ip = Ip::make($request); + + expect($ip->host())->toBe('2001:db8::1'); + expect($ip->port())->toBe(443); + expect($ip->isForwarded())->toBeFalse(); + expect($ip->forwardingAddresses())->toBe([]); +}); + +it('parses host only from raw IPv6 without port', function (): void { + $client = $this->createMock(Client::class); + $client->method('getRemoteAddress')->willReturn( + new class ('2001:db8::2') implements SocketAddress { + public function __construct(private string $address) + { + } + + public function toString(): string + { + return $this->address; + } + + public function getType(): SocketAddressType + { + return SocketAddressType::Internet; + } + + public function __toString(): string + { + return $this->address; + } + } + ); + + $request = new ServerRequest($client, HttpMethod::GET->value, Http::new(URL::build('/'))); + $ip = Ip::make($request); + + expect($ip->host())->toBe('2001:db8::2'); + expect($ip->port())->toBeNull(); +}); + +it('parses host and port from IPv4 with port', function (): void { + $client = $this->createMock(Client::class); + $client->method('getRemoteAddress')->willReturn( + new class ('192.168.0.1:8080') implements SocketAddress { + public function __construct(private string $address) + { + } + + public function toString(): string + { + return $this->address; + } + + public function getType(): SocketAddressType + { + return SocketAddressType::Internet; + } + + public function __toString(): string + { + return $this->address; + } + } + ); + + $request = new ServerRequest($client, HttpMethod::GET->value, Http::new(URL::build('/'))); + $ip = Ip::make($request); + + expect($ip->host())->toBe('192.168.0.1'); + expect($ip->port())->toBe(8080); +}); + +it('parses host only from hostname with port', function (): void { + $client = $this->createMock(Client::class); + $client->method('getRemoteAddress')->willReturn( + new class ('localhost:3000') implements SocketAddress { + public function __construct(private string $address) + { + } + + public function toString(): string + { + return $this->address; + } + + public function getType(): SocketAddressType + { + return SocketAddressType::Internet; + } + + public function __toString(): string + { + return $this->address; + } + } + ); + + $request = new ServerRequest($client, HttpMethod::GET->value, Http::new(URL::build('/'))); + $ip = Ip::make($request); + + expect($ip->host())->toBe('localhost'); + expect($ip->port())->toBe(3000); +}); + +it('parses host only from hostname without port', function (): void { + $client = $this->createMock(Client::class); + $client->method('getRemoteAddress')->willReturn( + new class ('example.com') implements SocketAddress { + public function __construct(private string $address) + { + } + + public function toString(): string + { + return $this->address; + } + + public function getType(): SocketAddressType + { + return SocketAddressType::Internet; + } + + public function __toString(): string + { + return $this->address; + } + } + ); + + $request = new ServerRequest($client, HttpMethod::GET->value, Http::new(URL::build('/'))); + $ip = Ip::make($request); + + expect($ip->host())->toBe('example.com'); + expect($ip->port())->toBeNull(); +}); + +it('sets forwarding info from X-Forwarded-For header', function (): void { + $client = $this->createMock(Client::class); + $client->method('getRemoteAddress')->willReturn( + new class ('10.0.0.1:1234') implements SocketAddress { + public function __construct(private string $address) + { + } + + public function toString(): string + { + return $this->address; + } + + public function getType(): SocketAddressType + { + return SocketAddressType::Internet; + } + + public function __toString(): string + { + return $this->address; + } + } + ); + + $request = new ServerRequest($client, HttpMethod::GET->value, Http::new(URL::build('/'))); + $request->setHeader('X-Forwarded-For', '203.0.113.1, 198.51.100.2'); + + $ip = Ip::make($request); + + expect($ip->isForwarded())->toBeTrue(); + expect($ip->forwardingAddresses())->toBe(['203.0.113.1', '198.51.100.2']); +}); From e69262611e3bc29b697e22c9e7638991ceb13974 Mon Sep 17 00:00:00 2001 From: barbosa89 Date: Tue, 9 Dec 2025 14:03:45 -0500 Subject: [PATCH 6/7] refactor: improve test description for IPv6 address parsing and add address expectation --- tests/Unit/Http/IpAddressTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/Unit/Http/IpAddressTest.php b/tests/Unit/Http/IpAddressTest.php index 7009f157..7e35b3d3 100644 --- a/tests/Unit/Http/IpAddressTest.php +++ b/tests/Unit/Http/IpAddressTest.php @@ -57,7 +57,7 @@ public function __toString(): string ['::ffff:192.168.0.1', hash('sha256', '::ffff:192.168.0.1')], ]); -it('parses host and port from remote address (IPv6 bracket + port)', function (): void { +it('parses host and port from remote address IPv6 bracket with port', function (): void { $client = $this->createMock(Client::class); $client->method('getRemoteAddress')->willReturn( new class ('[2001:db8::1]:443') implements SocketAddress { @@ -85,6 +85,7 @@ public function __toString(): string $request = new ServerRequest($client, HttpMethod::GET->value, Http::new(URL::build('/'))); $ip = Ip::make($request); + expect($ip->address())->toBe('[2001:db8::1]:443'); expect($ip->host())->toBe('2001:db8::1'); expect($ip->port())->toBe(443); expect($ip->isForwarded())->toBeFalse(); From 03a917fcb368616ab0ab90c50fe45416d1422d74 Mon Sep 17 00:00:00 2001 From: barbosa89 Date: Tue, 9 Dec 2025 15:24:32 -0500 Subject: [PATCH 7/7] refactor: update IP expectation in RequestTest to ensure it returns an Ip instance --- tests/Unit/Http/RequestTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/Unit/Http/RequestTest.php b/tests/Unit/Http/RequestTest.php index f26f67a2..126e55e5 100644 --- a/tests/Unit/Http/RequestTest.php +++ b/tests/Unit/Http/RequestTest.php @@ -11,6 +11,7 @@ use Amp\Http\Server\Trailers; use League\Uri\Http; use Phenix\Http\Constants\HttpMethod; +use Phenix\Http\Ip; use Phenix\Http\Request; use Phenix\Util\URL; use Psr\Http\Message\UriInterface; @@ -25,7 +26,7 @@ $formRequest = new Request($request); - expect($formRequest->ip())->toBeEmpty(); + expect($formRequest->ip())->toBeInstanceOf(Ip::class); expect($formRequest->route('post'))->toBe('7'); expect($formRequest->route('comment'))->toBe('22'); expect($formRequest->route()->integer('post'))->toBe(7);