From c28605fae35af90cf303d411e1a29722358a3ba5 Mon Sep 17 00:00:00 2001 From: MudassarTariq Date: Sat, 24 Oct 2026 23:05:09 +0500 Subject: [PATCH 1/4] fix: harden PHP transport and add security regression checks --- .github/workflows/php.yml | 51 ++++++- README.md | 70 +++++++++ lib/GuzzleTransport.php | 82 ++++++++++- lib/IpGeolocationClientConfig.php | 5 + lib/RequestNormalizer.php | 9 ++ tests/ip_location/GuzzleTransportTest.php | 56 +++++++ .../IpGeolocationClientConfigTest.php | 11 ++ tests/ip_location/IpGeolocationClientTest.php | 37 +++++ tests/ip_location/LiveIntegrationTest.php | 41 ++++-- tests/ip_location/Support/HttpTestServer.php | 84 +++++++++++ tests/ip_location/Support/http_server.php | 88 +++++++++++ .../ip_location/TransportIntegrationTest.php | 138 ++++++++++++++++++ 12 files changed, 648 insertions(+), 24 deletions(-) create mode 100644 tests/ip_location/Support/HttpTestServer.php create mode 100644 tests/ip_location/Support/http_server.php create mode 100644 tests/ip_location/TransportIntegrationTest.php diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml index 3309405..564bf93 100644 --- a/.github/workflows/php.yml +++ b/.github/workflows/php.yml @@ -8,30 +8,67 @@ on: branches: - master +permissions: + contents: read + jobs: test: + name: PHP ${{ matrix.php-version }} / ${{ matrix.dependencies }} runs-on: ubuntu-latest + timeout-minutes: 15 strategy: fail-fast: false matrix: - php-version: ['8.1', '8.2', '8.3'] + php-version: ['8.1', '8.2', '8.3', '8.4', '8.5'] + dependencies: [latest] + include: + - php-version: '8.1' + dependencies: lowest + - php-version: '8.2' + minimum-patched-version: '8.2.35' + - php-version: '8.3' + minimum-patched-version: '8.3.36' + - php-version: '8.4' + minimum-patched-version: '8.4.27' + - php-version: '8.5' + minimum-patched-version: '8.5.12' steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 + with: + persist-credentials: false - name: Set up PHP - uses: shivammathur/setup-php@v2 + uses: shivammathur/setup-php@eb7c497e18156a6bbabfef1d3a82760b9eda3962 # v2 with: php-version: ${{ matrix.php-version }} - extensions: curl + extensions: curl, openssl, zlib coverage: none + tools: composer:v2 + + - name: Verify supported runtimes include the security patches + if: matrix.minimum-patched-version != '' + env: + MINIMUM_PATCHED_VERSION: ${{ matrix.minimum-patched-version }} + run: php -r 'exit(version_compare(PHP_VERSION, getenv("MINIMUM_PATCHED_VERSION"), ">=") ? 0 : 1);' - name: Validate Composer - run: composer validate + run: composer validate --strict - name: Install dependencies - run: composer install --prefer-dist --no-interaction + run: | + if [ "${{ matrix.dependencies }}" = "lowest" ]; then + composer update --prefer-lowest --prefer-stable --prefer-dist --no-interaction --no-progress + else + composer install --prefer-dist --no-interaction --no-progress + fi + + - name: Audit all resolved dependencies + run: composer audit --locked --format=json + + - name: Check PHP syntax + run: find lib tests -name '*.php' -print0 | xargs -0 -n1 php -l - name: Run unit tests - run: ./vendor/bin/phpunit --exclude-group live + run: composer test:unit diff --git a/README.md b/README.md index 267360a..196c023 100644 --- a/README.md +++ b/README.md @@ -11,6 +11,7 @@ Look up IPv4, IPv6, and domains with `/v3/ipgeo` and `/v3/ipgeo-bulk`. Get geolo ## Table of Contents - [Installation](#installation) +- [Runtime Security](#runtime-security) - [Quick Start](#quick-start) - [At a Glance](#at-a-glance) - [Get Your API Key](#get-your-api-key) @@ -25,6 +26,7 @@ Look up IPv4, IPv6, and domains with `/v3/ipgeo` and `/v3/ipgeo-bulk`. Get geolo - [Errors](#errors) - [Troubleshooting](#troubleshooting) - [Frequently Asked Questions](#frequently-asked-questions) +- [Development and Testing](#development-and-testing) - [Links](#links) ## Installation @@ -45,6 +47,51 @@ use Ipgeolocation\Sdk\IpGeolocationClient; - Package page: - GitHub repository: +## Runtime Security + +PHP 8.1 is the SDK's language compatibility minimum. Production applications +must use a security-patched PHP runtime. The SDK cannot repair PHP engine +vulnerabilities through a Composer update. + +The [21 October 2026 PHP advisory](https://www.php.net/archive/2026.php#2026-10-21-5) +lists these minimum upstream fixes for CVE-2026-109618 and CVE-2026-109619: + +| PHP branch | Minimum patched version | +| --- | --- | +| 8.2 | 8.2.35 | +| 8.3 | 8.3.36 | +| 8.4 | 8.4.27 | +| 8.5 | 8.5.12 | + +Install the latest security release in a supported branch, or verify that your +distribution/vendor package includes the fixes. PHP 8.1 is upstream end of life; +its CI jobs check SDK compatibility and do not certify the runtime as secure. +Legacy deployments need verified vendor backports or an upgrade to supported PHP. + +Update the packages used by the web server and restart the PHP workers. Verify +the package revision used by PHP-FPM, Apache or your other web SAPI; the CLI can +use a different PHP installation. + +Also review the [23 October session-upload advisory](https://www.php.net/archive/2026.php#2026-10-23-1) +for CVE-2026-115560. It concerns applications that configure the session serializer +as `php_binary` or `php_serialize`, switch it to `php` at runtime before +`session_start()`, and leave session upload progress enabled. Follow PHP's +`session.upload_progress.enabled=0` guidance for that configuration until the +upstream fix is available. The SDK does not configure PHP sessions. + +Keep the application's own dependency lock updated and audited: + +```bash +composer update guzzlehttp/guzzle guzzlehttp/psr7 guzzlehttp/promises --with-all-dependencies +composer audit --locked +``` + +Keep TLS certificate verification enabled. The default transport verifies the +server certificate and hostname, bounds decoded response bytes and closes +response streams after use. Custom Guzzle clients should retain these TLS checks. +Configuration serialization/debug output and transport exception diagnostics redact +API keys in request URLs. Treat credentials and raw request objects as sensitive. + ## Quick Start ```php @@ -401,6 +448,29 @@ Use the raw methods with output => "xml". Check $result->success. Success items use $result->data. Error items use $result->error->message. +## Development and Testing + +```bash +composer install +composer validate --strict +composer audit --locked +composer test:unit +``` + +The unit suite includes real local HTTP and TLS servers. It checks single and +bulk requests, certificate trust and hostname verification, stalled responses, +truncated responses, redirects and decoded response-size limits, including gzip. +These tests need the OpenSSL extension and `proc_open`, and require no API key. +CI runs the suite on PHP 8.1 through 8.5 and checks the lowest allowed dependency +set on PHP 8.1. Supported runtime jobs enforce the security versions listed above. + +Live tests are opt-in. Supply credentials through the process environment rather +than saved files or command arguments. Set `IPGEO_RUN_LIVE_TESTS=true` and +`IPGEO_PAID_KEY` for paid-plan tests, or `IPGEO_FREE_KEY` for free-plan tests. +`IPGEO_RUN_LIVE_HARDENING=true` enables additional paid response comparisons. +Run them with `composer test:live`. Live requests consume account credits. +Never commit credentials or upload unredacted HTTP traces or exception dumps. + ## Links - API docs: diff --git a/lib/GuzzleTransport.php b/lib/GuzzleTransport.php index 9a5dff7..3905390 100644 --- a/lib/GuzzleTransport.php +++ b/lib/GuzzleTransport.php @@ -9,7 +9,11 @@ use GuzzleHttp\Exception\ConnectException; use GuzzleHttp\Exception\GuzzleException; use GuzzleHttp\Exception\RequestException; +use GuzzleHttp\Exception\TransferException; +use GuzzleHttp\Psr7\FnStream; use GuzzleHttp\Psr7\Request; +use GuzzleHttp\Psr7\Utils; +use Psr\Http\Message\ResponseInterface; final class GuzzleTransport implements HttpTransport { @@ -30,12 +34,35 @@ public function __construct( public function sendRequest(HttpRequestData $request): HttpResponseData { + $resource = Utils::tryFopen('php://temp', 'w+'); + $buffer = Utils::streamFor($resource); + $sink = FnStream::decorate($buffer, [ + 'write' => function (string $chunk) use ($buffer): int { + if ($buffer->tell() + strlen($chunk) > $this->max_response_body_bytes) { + throw new TransportException( + 'Response body exceeded max size of ' . $this->max_response_body_bytes . ' bytes' + ); + } + return $buffer->write($chunk); + }, + ]); + $onHeaders = $this->client->getConfig('on_headers'); try { $response = $this->client->send( new Request($request->method, $request->url, $request->headers, $request->body), [ 'http_errors' => false, - 'stream' => true, + // cURL enforces connection and low-speed timeouts; the sink bounds decoded bytes. + 'stream' => false, + 'sink' => $sink, + 'read_timeout' => $request->read_timeout, + 'on_headers' => static function ($response) use ($resource, $buffer, $onHeaders): void { + ftruncate($resource, 0); + $buffer->rewind(); + if ($onHeaders !== null && $response instanceof ResponseInterface) { + $onHeaders($response); + } + }, 'connect_timeout' => $request->connect_timeout, 'curl' => [ \CURLOPT_LOW_SPEED_LIMIT => 1, @@ -45,6 +72,12 @@ public function sendRequest(HttpRequestData $request): HttpResponseData ); $stream = $response->getBody(); + if ($stream->isSeekable()) { + $stream->rewind(); + } elseif ($stream->eof() && $buffer->tell() > 0) { + $stream = $sink; + $stream->rewind(); + } $chunks = []; $bodyBytes = 0; while (!$stream->eof()) { @@ -76,22 +109,27 @@ public function sendRequest(HttpRequestData $request): HttpResponseData if ($this->isTimeoutException($error)) { throw new RequestTimeoutException( 'HTTP request timed out after ' . (int) ($request->connect_timeout * 1000) . 'ms while opening the connection', - $error + $this->redactError($error) ); } - throw new TransportException('HTTP transport error', $error); + throw new TransportException('HTTP transport error', $this->redactError($error)); } catch (RequestException $error) { if ($this->isTimeoutException($error)) { throw new RequestTimeoutException( 'HTTP request timed out while waiting for response body data', - $error + $this->redactError($error) ); } - throw new TransportException('HTTP transport error', $error); + throw new TransportException('HTTP transport error', $this->redactError($error)); } catch (GuzzleException $error) { - throw new TransportException('HTTP transport error', $error); + throw new TransportException('HTTP transport error', $this->redactError($error)); + } finally { + if (isset($response)) { + $response->getBody()->close(); + } + $sink->close(); } } @@ -99,6 +137,38 @@ public function close(): void { } + private function redactError(GuzzleException $error): \Throwable + { + $message = self::redactApiKey($error->getMessage()); + $previous = $error->getPrevious(); + $safePrevious = $previous === null ? null : new \RuntimeException( + self::redactApiKey($previous->getMessage()), + (int) $previous->getCode() + ); + if ($error instanceof ConnectException || $error instanceof RequestException) { + $request = $error->getRequest(); + $uri = $request->getUri(); + $request = $request->withUri($uri->withQuery(substr(self::redactApiKey('?' . $uri->getQuery()), 1))); + $context = $error->getHandlerContext(); + array_walk_recursive($context, static function (&$value): void { + if (is_string($value)) { + $value = self::redactApiKey($value); + } + }); + $class = $error::class; + if ($error instanceof ConnectException) { + return new $class($message, $request, $safePrevious, $context); + } + return new $class($message, $request, $error->getResponse(), $safePrevious, $context); + } + return new TransferException($message, (int) $error->getCode(), $safePrevious); + } + + private static function redactApiKey(string $text): string + { + return preg_replace('/([?&]apiKey=)[^&#\s]+/i', '$1[REDACTED]', $text) ?? $text; + } + private function isTimeoutException(\Throwable $error): bool { if (method_exists($error, 'getHandlerContext')) { diff --git a/lib/IpGeolocationClientConfig.php b/lib/IpGeolocationClientConfig.php index 2b31ddb..3697f49 100644 --- a/lib/IpGeolocationClientConfig.php +++ b/lib/IpGeolocationClientConfig.php @@ -44,6 +44,11 @@ public function toArray(bool $keepNulls = false): array return $data; } + public function __debugInfo(): array + { + return $this->toArray(true); + } + private static function normalizeApiKey(?string $value): ?string { if ($value === null) { diff --git a/lib/RequestNormalizer.php b/lib/RequestNormalizer.php index b45b0ad..411c7f7 100644 --- a/lib/RequestNormalizer.php +++ b/lib/RequestNormalizer.php @@ -50,6 +50,9 @@ public static function headers(array $headers): array if (str_contains($name, "\r") || str_contains($name, "\n")) { throw new ValidationException('header names must not contain CR or LF'); } + if (preg_match('/^[!#$%&\'*+.^_`|~0-9A-Za-z-]+$/D', $name) !== 1) { + throw new ValidationException('header names must be valid HTTP tokens'); + } $values = is_array($rawValue) ? $rawValue : [$rawValue]; if ($values === []) { @@ -70,6 +73,9 @@ public static function headers(array $headers): array if (str_contains($text, "\r") || str_contains($text, "\n")) { throw new ValidationException('header values must not contain CR or LF'); } + if (preg_match('/[\x00-\x08\x0a-\x1f\x7f]/', $text) === 1) { + throw new ValidationException('header values must not contain control characters'); + } $normalizedValues[] = $text; } @@ -90,6 +96,9 @@ public static function optionalString(?string $value, string $field): ?string if ($normalized === '') { throw new ValidationException($field . ' must not be blank'); } + if (preg_match('/[\x00-\x08\x0a-\x1f\x7f]/', $normalized) === 1) { + throw new ValidationException($field . ' must not contain control characters'); + } return $normalized; } diff --git a/tests/ip_location/GuzzleTransportTest.php b/tests/ip_location/GuzzleTransportTest.php index 3efac8e..ea28e94 100644 --- a/tests/ip_location/GuzzleTransportTest.php +++ b/tests/ip_location/GuzzleTransportTest.php @@ -6,6 +6,7 @@ use GuzzleHttp\Client; use GuzzleHttp\Exception\ConnectException; +use GuzzleHttp\Exception\RequestException; use GuzzleHttp\Handler\MockHandler; use GuzzleHttp\HandlerStack; use GuzzleHttp\Middleware; @@ -16,9 +17,64 @@ use Ipgeolocation\Sdk\RequestTimeoutException; use Ipgeolocation\Sdk\TransportException; use PHPUnit\Framework\TestCase; +use PHPUnit\Framework\Attributes\DataProvider; final class GuzzleTransportTest extends TestCase { + public static function transportErrors(): array + { + return [['connect'], ['request']]; + } + + #[DataProvider('transportErrors')] + public function testTransportDiagnosticsRedactApiKeys(string $kind): void + { + $credential = 'unit-test-credential'; + $url = 'https://api.ipgeolocation.io/v3/ipgeo?apiKey=' . $credential . '&ip=8.8.8.8'; + $request = new Request('GET', $url); + $previous = new \RuntimeException('Request failed for ' . $url); + $context = ['errno' => 60, 'url' => $url]; + $cause = $kind === 'connect' + ? new ConnectException('TLS failure for ' . $url, $request, $previous, $context) + : new RequestException('Transfer failure for ' . $url, $request, null, $previous, $context); + $transport = new GuzzleTransport(new Client(['handler' => HandlerStack::create(new MockHandler([$cause]))])); + + try { + $transport->sendRequest(new HttpRequestData($url, 'GET', [], null, 1.0, 1.0)); + self::fail('Expected a transport error'); + } catch (TransportException $error) { + self::assertInstanceOf($cause::class, $error->cause); + self::assertStringNotContainsString($credential, (string) $error); + self::assertStringNotContainsString($credential, (string) $error->cause->getRequest()->getUri()); + self::assertStringNotContainsString($credential, json_encode($error->cause->getHandlerContext(), JSON_THROW_ON_ERROR)); + self::assertSame(60, $error->cause->getHandlerContext()['errno']); + } + } + + public function testClosesResponseStreamsAfterReading(): void + { + $stream = \GuzzleHttp\Psr7\Utils::streamFor('{}'); + $transport = new GuzzleTransport(new Client([ + 'handler' => HandlerStack::create(new MockHandler([new Response(200, [], $stream)])), + ])); + $response = $transport->sendRequest(new HttpRequestData('https://example.com', 'GET', [], null, 1.0, 1.0)); + self::assertSame('{}', $response->body); + self::assertFalse($stream->isReadable()); + } + + public function testReadsANonSeekableResponse(): void + { + $stream = \GuzzleHttp\Psr7\FnStream::decorate(\GuzzleHttp\Psr7\Utils::streamFor('{}'), [ + 'isSeekable' => static fn (): bool => false, + ]); + $transport = new GuzzleTransport(new Client([ + 'handler' => HandlerStack::create(new MockHandler([new Response(200, [], $stream)])), + ])); + $response = $transport->sendRequest(new HttpRequestData('https://example.com', 'GET', [], null, 1.0, 1.0)); + self::assertSame('{}', $response->body); + self::assertFalse($stream->isReadable()); + } + public function testPassesBodyProgressTimeoutToCurlOptions(): void { $history = []; diff --git a/tests/ip_location/IpGeolocationClientConfigTest.php b/tests/ip_location/IpGeolocationClientConfigTest.php index 70d6ccd..b879d55 100644 --- a/tests/ip_location/IpGeolocationClientConfigTest.php +++ b/tests/ip_location/IpGeolocationClientConfigTest.php @@ -62,4 +62,15 @@ public function testSerializationRedactsApiKey(): void self::assertSame(10, $decoded['connect_timeout']); self::assertSame(30, $decoded['read_timeout']); } + + public function testDebugOutputRedactsApiKey(): void + { + $config = new IpGeolocationClientConfig(api_key: 'unit-test-credential'); + ob_start(); + var_dump($config); + $debug = (string) ob_get_clean(); + self::assertStringNotContainsString('unit-test-credential', $debug); + self::assertStringContainsString('[REDACTED]', $debug); + self::assertSame('unit-test-credential', $config->api_key); + } } diff --git a/tests/ip_location/IpGeolocationClientTest.php b/tests/ip_location/IpGeolocationClientTest.php index 84b315d..06ce738 100644 --- a/tests/ip_location/IpGeolocationClientTest.php +++ b/tests/ip_location/IpGeolocationClientTest.php @@ -11,9 +11,46 @@ use Ipgeolocation\Sdk\ValidationException; use Ipgeolocation\Sdk\Test\Support\FakeTransport; use PHPUnit\Framework\TestCase; +use PHPUnit\Framework\Attributes\DataProvider; final class IpGeolocationClientTest extends TestCase { + public static function malformedHeaders(): iterable + { + $cases = [ + 'name_space' => ['headers' => ['Bad Name' => 'value']], + 'name_colon' => ['headers' => ['Bad:Name' => 'value']], + 'name_crlf' => ['headers' => ["Bad\r\nInjected" => 'value']], + 'value_crlf' => ['headers' => ['X-Test' => "value\r\nInjected: yes"]], + 'value_nul' => ['headers' => ['X-Test' => "before\0after"]], + 'value_del' => ['headers' => ['X-Test' => "before\x7fafter"]], + 'user_agent_crlf' => ['user_agent' => "agent\r\nInjected: yes"], + 'user_agent_nul' => ['user_agent' => "agent\0suffix"], + ]; + foreach ($cases as $name => $request) { + yield $name . '_single' => [false, $request]; + yield $name . '_bulk' => [true, $request]; + } + } + + #[DataProvider('malformedHeaders')] + public function testMalformedHeadersAreRejectedBeforeAnyTransportCall(bool $bulk, array $request): void + { + $transport = new FakeTransport(); + $transport->push(new HttpResponseData(200, $bulk ? '[{"ip":"8.8.8.8"}]' : '{"ip":"8.8.8.8"}', [])); + $client = new IpGeolocationClient(['api_key' => 'test-key'], $transport); + try { + if ($bulk) { + $client->bulkLookupIpGeolocation(['ips' => ['8.8.8.8']] + $request); + } else { + $client->lookupIpGeolocation(['ip' => '8.8.8.8'] + $request); + } + self::fail('Expected invalid HTTP headers to be rejected'); + } catch (ValidationException) { + self::assertSame([], $transport->requests); + } + } + public function testLookupBuildsExpectedGetRequest(): void { $transport = new FakeTransport(); diff --git a/tests/ip_location/LiveIntegrationTest.php b/tests/ip_location/LiveIntegrationTest.php index c3c0b9b..cf25af3 100644 --- a/tests/ip_location/LiveIntegrationTest.php +++ b/tests/ip_location/LiveIntegrationTest.php @@ -13,11 +13,11 @@ #[Group('live')] final class LiveIntegrationTest extends TestCase { - private static function shouldRunLiveTests(): bool + private static function shouldRunLiveTests(string $key): bool { return getenv('IPGEO_RUN_LIVE_TESTS') === 'true' - && getenv('IPGEO_FREE_KEY') !== false - && getenv('IPGEO_PAID_KEY') !== false; + && getenv($key) !== false + && trim((string) getenv($key)) !== ''; } private function freeClient(): IpGeolocationClient @@ -32,8 +32,8 @@ private function paidClient(): IpGeolocationClient public function testFreePlanBaseLookupWorks(): void { - if (!self::shouldRunLiveTests()) { - self::markTestSkipped('Set IPGEO_RUN_LIVE_TESTS=true, IPGEO_FREE_KEY, and IPGEO_PAID_KEY to enable live tests'); + if (!self::shouldRunLiveTests('IPGEO_FREE_KEY')) { + self::markTestSkipped('Set IPGEO_RUN_LIVE_TESTS=true and IPGEO_FREE_KEY to enable free-plan tests'); } $response = $this->freeClient()->lookupIpGeolocation(['ip' => '8.8.8.8']); @@ -42,8 +42,8 @@ public function testFreePlanBaseLookupWorks(): void public function testFreePlanDomainLookupIsRejected(): void { - if (!self::shouldRunLiveTests()) { - self::markTestSkipped('Set IPGEO_RUN_LIVE_TESTS=true, IPGEO_FREE_KEY, and IPGEO_PAID_KEY to enable live tests'); + if (!self::shouldRunLiveTests('IPGEO_FREE_KEY')) { + self::markTestSkipped('Set IPGEO_RUN_LIVE_TESTS=true and IPGEO_FREE_KEY to enable free-plan tests'); } $this->expectException(ApiException::class); @@ -52,8 +52,8 @@ public function testFreePlanDomainLookupIsRejected(): void public function testPaidPlanBulkMixedLookupReturnsSuccessAndErrorItems(): void { - if (!self::shouldRunLiveTests()) { - self::markTestSkipped('Set IPGEO_RUN_LIVE_TESTS=true, IPGEO_FREE_KEY, and IPGEO_PAID_KEY to enable live tests'); + if (!self::shouldRunLiveTests('IPGEO_PAID_KEY')) { + self::markTestSkipped('Set IPGEO_RUN_LIVE_TESTS=true and IPGEO_PAID_KEY to enable paid-plan tests'); } $response = $this->paidClient()->bulkLookupIpGeolocation([ @@ -69,8 +69,8 @@ public function testPaidPlanBulkMixedLookupReturnsSuccessAndErrorItems(): void public function testPaidPlanRawXmlWorks(): void { - if (!self::shouldRunLiveTests()) { - self::markTestSkipped('Set IPGEO_RUN_LIVE_TESTS=true, IPGEO_FREE_KEY, and IPGEO_PAID_KEY to enable live tests'); + if (!self::shouldRunLiveTests('IPGEO_PAID_KEY')) { + self::markTestSkipped('Set IPGEO_RUN_LIVE_TESTS=true and IPGEO_PAID_KEY to enable paid-plan tests'); } $response = $this->paidClient()->lookupIpGeolocationRaw([ @@ -80,4 +80,23 @@ public function testPaidPlanRawXmlWorks(): void self::assertStringContainsString('<', $response->data); } + + public function testPaidPlanDomainAndIpv6LookupsWork(): void + { + if (!self::shouldRunLiveTests('IPGEO_PAID_KEY')) { + self::markTestSkipped('Set IPGEO_RUN_LIVE_TESTS=true and IPGEO_PAID_KEY to enable paid-plan tests'); + } + + $client = $this->paidClient(); + try { + $ipv6 = $client->lookupIpGeolocation(['ip' => '2001:4860:4860::8888']); + self::assertSame(inet_pton('2001:4860:4860::8888'), inet_pton((string) $ipv6->data->ip)); + self::assertSame(200, $ipv6->metadata->status_code); + $domain = $client->lookupIpGeolocation(['ip' => 'ipgeolocation.io']); + self::assertNotEmpty($domain->data->ip); + self::assertSame(200, $domain->metadata->status_code); + } finally { + $client->close(); + } + } } diff --git a/tests/ip_location/Support/HttpTestServer.php b/tests/ip_location/Support/HttpTestServer.php new file mode 100644 index 0000000..3f8d336 --- /dev/null +++ b/tests/ip_location/Support/HttpTestServer.php @@ -0,0 +1,84 @@ +directory = sys_get_temp_dir() . '/ipgeo-tls-' . bin2hex(random_bytes(8)); + if (!mkdir($this->directory, 0700)) { + throw new \RuntimeException('Cannot create test certificate directory'); + } + $config = $this->directory . '/openssl.cnf'; + file_put_contents($config, "[req]\ndistinguished_name=dn\n[dn]\n[cert]\nsubjectAltName=DNS:localhost\nbasicConstraints=critical,CA:TRUE\nkeyUsage=critical,digitalSignature,keyEncipherment,keyCertSign\nextendedKeyUsage=serverAuth\n"); + $key = openssl_pkey_new(['private_key_bits' => 2048]); + if ($key === false) { + throw new \RuntimeException('Cannot create test private key'); + } + $csr = openssl_csr_new(['commonName' => 'localhost'], $key, ['config' => $config, 'digest_alg' => 'sha256']); + $cert = $csr === false ? false : openssl_csr_sign($csr, null, $key, 1, ['config' => $config, 'x509_extensions' => 'cert', 'digest_alg' => 'sha256']); + if ($cert === false || !openssl_x509_export($cert, $certificate) || !openssl_pkey_export($key, $privateKey)) { + throw new \RuntimeException('Cannot create test certificate'); + } + $caFile = $this->directory . '/ca.pem'; + $keyFile = $this->directory . '/key.pem'; + file_put_contents($caFile, $certificate); + file_put_contents($keyFile, $privateKey); + chmod($keyFile, 0600); + } + + $this->caFile = $caFile; + $this->process = proc_open( + [PHP_BINARY, __DIR__ . '/http_server.php', $this->caFile ?? '', $keyFile], + [['pipe', 'r'], ['pipe', 'w'], ['pipe', 'w']], + $this->pipes + ); + if (!is_resource($this->process)) { + throw new \RuntimeException('Cannot start test HTTP server'); + } + fclose($this->pipes[0]); + unset($this->pipes[0]); + stream_set_timeout($this->pipes[1], 5); + $port = trim((string) fgets($this->pipes[1])); + if (!ctype_digit($port)) { + throw new \RuntimeException('Test HTTP server did not start: ' . stream_get_contents($this->pipes[2])); + } + $this->url = ($tls ? 'https://localhost:' : 'http://127.0.0.1:') . $port; + } + + public function close(): void + { + if (is_resource($this->process)) { + proc_terminate($this->process); + foreach ($this->pipes as $pipe) { + fclose($pipe); + } + proc_close($this->process); + $this->process = null; + } + if ($this->directory !== null) { + foreach (glob($this->directory . '/*') ?: [] as $file) { + unlink($file); + } + rmdir($this->directory); + $this->directory = null; + } + } + + public function __destruct() + { + $this->close(); + } +} diff --git a/tests/ip_location/Support/http_server.php b/tests/ip_location/Support/http_server.php new file mode 100644 index 0000000..192d27d --- /dev/null +++ b/tests/ip_location/Support/http_server.php @@ -0,0 +1,88 @@ + ['local_cert' => $argv[1], 'local_pk' => $argv[2]]] : []); +$server = stream_socket_server('tcp://127.0.0.1:0', $errno, $error, STREAM_SERVER_BIND | STREAM_SERVER_LISTEN, $context); +if ($server === false) { + throw new RuntimeException('Cannot bind test HTTP server'); +} +echo substr(strrchr(stream_socket_get_name($server, false), ':'), 1) . "\n"; +flush(); + +while ($connection = @stream_socket_accept($server, 10)) { + stream_set_timeout($connection, 5); + if ($tls && !@stream_socket_enable_crypto($connection, true, STREAM_CRYPTO_METHOD_TLS_SERVER)) { + fclose($connection); + continue; + } + $line = fgets($connection, 8192); + if ($line === false) { + fclose($connection); + continue; + } + [$method, $target] = explode(' ', $line, 3); + $headers = []; + while (($line = fgets($connection, 8192)) !== false && trim($line) !== '') { + [$name, $value] = explode(':', $line, 2); + $headers[strtolower($name)] = trim($value); + } + $body = ''; + $length = min(65536, (int) ($headers['content-length'] ?? 0)); + while (strlen($body) < $length && !feof($connection)) { + $chunk = fread($connection, $length - strlen($body)); + if ($chunk === false || $chunk === '') { + break; + } + $body .= $chunk; + } + $path = parse_url($target, PHP_URL_PATH); + parse_str((string) parse_url($target, PHP_URL_QUERY), $query); + $status = 200; + $payload = '{}'; + $extraHeaders = ''; + if ($path === '/slow-headers') { + usleep(3000000); + } elseif ($path === '/redirect') { + $status = 302; + $payload = 'redirect-body'; + $extraHeaders = "Location: /ok\r\n"; + } elseif ($path === '/oversized' || $path === '/compressed') { + $payload = str_repeat('x', 8192); + if ($path === '/compressed') { + $payload = gzencode($payload); + $extraHeaders = "Content-Encoding: gzip\r\n"; + } + } elseif ($path === '/v3/ipgeo' || $path === '/v3/ipgeo-bulk') { + $valid = ($query['apiKey'] ?? '') === 'test-key' + && ($headers['user-agent'] ?? '') === 'php-security-test' + && ($headers['accept'] ?? '') === 'application/json'; + if ($path === '/v3/ipgeo') { + $valid = $valid && $method === 'GET' && ($query['ip'] ?? '') === '8.8.8.8' + && ($query['include'] ?? '') === 'security,abuse' && ($headers['x-test'] ?? '') === 'yes'; + $payload = '{"ip":"8.8.8.8","location":{"country_name":"United States"},"security":{"is_proxy":false,"threat_score":0},"abuse":{"emails":["abuse@example.com"]}}'; + $extraHeaders = "X-Credits-Charged: 1\r\n"; + } else { + $valid = $valid && $method === 'POST' && ($headers['content-type'] ?? '') === 'application/json' + && json_decode($body, true) === ['ips' => ['8.8.8.8', 'invalid-ip', '1.1.1.1']]; + $payload = '[{"ip":"8.8.8.8"},{"message":"Invalid IP address."},{"ip":"1.1.1.1"}]'; + $extraHeaders = "X-Credits-Charged: 2\r\nX-Successful-Record: 2\r\n"; + } + if (!$valid) { + $status = 400; + $payload = '{"message":"Unexpected test request"}'; + } + } elseif (!in_array($path, ['/ok', '/slow-body', '/truncated'], true)) { + $status = 501; + } + $declaredLength = $path === '/truncated' ? 10 : strlen($payload); + @fwrite($connection, "HTTP/1.1 $status Test\r\nContent-Type: application/json\r\nContent-Length: $declaredLength\r\n$extraHeaders" . "Connection: close\r\n\r\n"); + fflush($connection); + if ($path === '/slow-body') { + usleep(3000000); + } + @fwrite($connection, $payload); + fclose($connection); +} +fclose($server); diff --git a/tests/ip_location/TransportIntegrationTest.php b/tests/ip_location/TransportIntegrationTest.php new file mode 100644 index 0000000..0af505d --- /dev/null +++ b/tests/ip_location/TransportIntegrationTest.php @@ -0,0 +1,138 @@ +servers as $server) { + $server->close(); + } + } + + private function server(bool $tls = false): HttpTestServer + { + return $this->servers[] = new HttpTestServer($tls); + } + + public function testSingleAndBulkRequestsUseTheRealTransport(): void + { + $server = $this->server(); + $client = new IpGeolocationClient(['api_key' => 'test-key', 'base_url' => $server->url]); + $response = $client->lookupIpGeolocation([ + 'ip' => '8.8.8.8', 'include' => ['security', 'abuse'], + 'user_agent' => 'php-security-test', 'headers' => ['X-Test' => 'yes'], + ]); + self::assertSame('8.8.8.8', $response->data->ip); + self::assertSame('United States', $response->data->location?->country_name); + self::assertFalse($response->data->security?->is_proxy); + self::assertSame(['abuse@example.com'], $response->data->abuse?->emails); + self::assertSame(1, $response->metadata->credits_charged); + $bulk = $client->bulkLookupIpGeolocation([ + 'ips' => ['8.8.8.8', 'invalid-ip', '1.1.1.1'], 'user_agent' => 'php-security-test', + ]); + self::assertCount(3, $bulk->data); + self::assertSame('8.8.8.8', $bulk->data[0]->data->ip); + self::assertFalse($bulk->data[1]->success); + self::assertSame('Invalid IP address.', $bulk->data[1]->error->message); + self::assertSame('1.1.1.1', $bulk->data[2]->data->ip); + self::assertSame(2, $bulk->metadata->successful_records); + $client->close(); + } + + public function testAcceptsATrustedCertificate(): void + { + $server = $this->server(true); + $transport = new GuzzleTransport(new Client(['verify' => $server->caFile])); + $response = $transport->sendRequest($this->request($server->url . '/ok')); + self::assertSame(200, $response->status_code); + self::assertSame('{}', $response->body); + } + + public function testRejectsAnUntrustedCertificateByDefault(): void + { + $server = $this->server(true); + $this->expectException(TransportException::class); + (new GuzzleTransport())->sendRequest($this->request($server->url . '/ok')); + } + + public function testRejectsACertificateForAnotherHostname(): void + { + $server = $this->server(true); + $transport = new GuzzleTransport(new Client(['verify' => $server->caFile])); + $this->expectException(TransportException::class); + $transport->sendRequest($this->request(str_replace('localhost', '127.0.0.1', $server->url) . '/ok')); + } + + public static function stalledResponses(): array + { + return [['/slow-body'], ['/slow-headers']]; + } + + #[DataProvider('stalledResponses')] + public function testStalledResponsesHonorTheReadTimeout(string $path): void + { + $server = $this->server(); + $started = microtime(true); + $this->expectException(RequestTimeoutException::class); + try { + (new GuzzleTransport())->sendRequest($this->request($server->url . $path)); + } finally { + self::assertLessThan(2.5, microtime(true) - $started, 'The transport must not wait for the 3-second response'); + } + } + + public static function oversizedResponses(): array + { + return [['/oversized'], ['/compressed']]; + } + + #[DataProvider('oversizedResponses')] + public function testBoundsDecodedResponseBytes(string $path): void + { + $server = $this->server(); + $this->expectException(TransportException::class); + $this->expectExceptionMessage('Response body exceeded max size of 1024 bytes'); + (new GuzzleTransport(null, 1024))->sendRequest($this->request($server->url . $path)); + } + + public function testRejectsATruncatedResponse(): void + { + $server = $this->server(); + $this->expectException(TransportException::class); + (new GuzzleTransport())->sendRequest($this->request($server->url . '/truncated')); + } + + public function testRedirectsKeepOnlyTheFinalResponseBody(): void + { + $server = $this->server(); + $headers = 0; + $transport = new GuzzleTransport(new Client(['on_headers' => static function () use (&$headers): void { + $headers++; + }])); + $response = $transport->sendRequest($this->request($server->url . '/redirect')); + self::assertSame(200, $response->status_code); + self::assertSame('{}', $response->body); + self::assertSame(2, $headers, 'Preserve custom client callbacks across redirects'); + } + + private function request(string $url): HttpRequestData + { + return new HttpRequestData($url, 'GET', ['Accept' => ['application/json']], null, 1.0, 1.0); + } +} From c694d128677cf725dd0a35454b54955fc622a71f Mon Sep 17 00:00:00 2001 From: MudassarTariq Date: Sat, 24 Oct 2026 23:07:19 +0500 Subject: [PATCH 2/4] ci: quote the PHP security runtime check correctly --- .github/workflows/php.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml index 564bf93..9b4b766 100644 --- a/.github/workflows/php.yml +++ b/.github/workflows/php.yml @@ -51,7 +51,8 @@ jobs: if: matrix.minimum-patched-version != '' env: MINIMUM_PATCHED_VERSION: ${{ matrix.minimum-patched-version }} - run: php -r 'exit(version_compare(PHP_VERSION, getenv("MINIMUM_PATCHED_VERSION"), ">=") ? 0 : 1);' + run: | + php -r 'exit(version_compare(PHP_VERSION, getenv("MINIMUM_PATCHED_VERSION"), ">=") ? 0 : 1);' - name: Validate Composer run: composer validate --strict From 185d21522b5a5fe602be57d6ef28004decc54dec Mon Sep 17 00:00:00 2001 From: MudassarTariq Date: Sat, 24 Oct 2026 23:11:43 +0500 Subject: [PATCH 3/4] fix: preserve error handling for custom Guzzle exceptions --- lib/GuzzleTransport.php | 5 +++++ tests/ip_location/GuzzleTransportTest.php | 24 +++++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/lib/GuzzleTransport.php b/lib/GuzzleTransport.php index 3905390..ae3c115 100644 --- a/lib/GuzzleTransport.php +++ b/lib/GuzzleTransport.php @@ -156,6 +156,11 @@ private function redactError(GuzzleException $error): \Throwable } }); $class = $error::class; + $base = $error instanceof ConnectException ? ConnectException::class : RequestException::class; + // Custom exception subclasses may require a different constructor signature. + if ((new \ReflectionMethod($class, '__construct'))->getDeclaringClass()->getName() !== $base) { + $class = $base; + } if ($error instanceof ConnectException) { return new $class($message, $request, $safePrevious, $context); } diff --git a/tests/ip_location/GuzzleTransportTest.php b/tests/ip_location/GuzzleTransportTest.php index ea28e94..156a793 100644 --- a/tests/ip_location/GuzzleTransportTest.php +++ b/tests/ip_location/GuzzleTransportTest.php @@ -75,6 +75,22 @@ public function testReadsANonSeekableResponse(): void self::assertFalse($stream->isReadable()); } + public function testCustomExceptionConstructorsKeepSdkErrorHandling(): void + { + $url = 'https://example.com?apiKey=unit-test-credential'; + $cause = new CustomConnectException(new Request('GET', $url)); + $transport = new GuzzleTransport(new Client([ + 'handler' => HandlerStack::create(new MockHandler([$cause])), + ])); + try { + $transport->sendRequest(new HttpRequestData($url, 'GET', [], null, 1.0, 1.0)); + self::fail('Expected a transport error'); + } catch (TransportException $error) { + self::assertInstanceOf(ConnectException::class, $error->cause); + self::assertStringNotContainsString('unit-test-credential', (string) $error); + } + } + public function testPassesBodyProgressTimeoutToCurlOptions(): void { $history = []; @@ -168,3 +184,11 @@ public function testRejectsResponseBodyThatExceedsConfiguredLimit(): void )); } } + +final class CustomConnectException extends ConnectException +{ + public function __construct(Request $request) + { + parent::__construct('TLS failure for ' . $request->getUri(), $request); + } +} From 24789fc60c7a1780d964b684160b135a68931c0c Mon Sep 17 00:00:00 2001 From: MudassarTariq Date: Mon, 26 Oct 2026 17:16:36 +0500 Subject: [PATCH 4/4] fix: correct timeout diagnostics and prepare 3.0.2 --- CHANGELOG.md | 9 +++++++++ lib/GuzzleTransport.php | 4 ++-- lib/Version.php | 2 +- tests/ip_location/GuzzleTransportTest.php | 7 +++++-- tests/ip_location/TransportIntegrationTest.php | 10 +++++++++- 5 files changed, 26 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ec617e2..0c83e49 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,14 @@ # Changelog +## 3.0.2 + +- Enforce connection and read limits with cURL, reject truncated responses, and bound decoded response bodies, including gzip. +- Validate HTTP headers and user-agent values before sending requests, including through custom transports. +- Redact API keys from configuration debug output and transport diagnostics, and close response streams after use. +- Report timeouts without incorrectly identifying the connection phase or duration. +- Add local HTTP/TLS regression coverage and CI for PHP 8.1 through 8.5, including the lowest allowed dependencies on PHP 8.1. +- Keep public methods, response models, JSON/XML support and PHP 8.1 syntax compatibility. Custom Guzzle exception subclasses with incompatible constructors are normalized to the corresponding base exception, and nested diagnostic causes are redacted copies. + ## 3.0.1 - Raised the minimum `guzzlehttp/guzzle` version to 7.15.2 and the dev `phpunit/phpunit` version to 10.5.62, so installs can no longer resolve releases with known security vulnerabilities. No API or behavior changes. diff --git a/lib/GuzzleTransport.php b/lib/GuzzleTransport.php index ae3c115..3a96223 100644 --- a/lib/GuzzleTransport.php +++ b/lib/GuzzleTransport.php @@ -108,7 +108,7 @@ public function sendRequest(HttpRequestData $request): HttpResponseData } catch (ConnectException $error) { if ($this->isTimeoutException($error)) { throw new RequestTimeoutException( - 'HTTP request timed out after ' . (int) ($request->connect_timeout * 1000) . 'ms while opening the connection', + 'HTTP request timed out', $this->redactError($error) ); } @@ -117,7 +117,7 @@ public function sendRequest(HttpRequestData $request): HttpResponseData } catch (RequestException $error) { if ($this->isTimeoutException($error)) { throw new RequestTimeoutException( - 'HTTP request timed out while waiting for response body data', + 'HTTP request timed out', $this->redactError($error) ); } diff --git a/lib/Version.php b/lib/Version.php index eb75b4e..2381960 100644 --- a/lib/Version.php +++ b/lib/Version.php @@ -6,7 +6,7 @@ final class Version { - public const VERSION = '3.0.1'; + public const VERSION = '3.0.2'; private function __construct() { diff --git a/tests/ip_location/GuzzleTransportTest.php b/tests/ip_location/GuzzleTransportTest.php index 156a793..8ee11f1 100644 --- a/tests/ip_location/GuzzleTransportTest.php +++ b/tests/ip_location/GuzzleTransportTest.php @@ -140,11 +140,13 @@ public function testReturnsResponseData(): void self::assertSame(['1'], $response->headers['X-Credits-Charged']); } - public function testMapsTimeoutsToRequestTimeoutException(): void + #[DataProvider('transportErrors')] + public function testMapsTimeoutsToRequestTimeoutException(string $kind): void { + $exception = $kind === 'connect' ? ConnectException::class : RequestException::class; $transport = new GuzzleTransport(new Client([ 'handler' => HandlerStack::create(new MockHandler([ - new ConnectException( + new $exception( 'cURL error 28: Operation timed out', new Request('GET', 'https://api.ipgeolocation.io/v3/ipgeo') ), @@ -152,6 +154,7 @@ public function testMapsTimeoutsToRequestTimeoutException(): void ])); $this->expectException(RequestTimeoutException::class); + $this->expectExceptionMessageMatches('/^HTTP request timed out$/'); $transport->sendRequest(new HttpRequestData( url: 'https://api.ipgeolocation.io/v3/ipgeo', diff --git a/tests/ip_location/TransportIntegrationTest.php b/tests/ip_location/TransportIntegrationTest.php index 0af505d..2eab960 100644 --- a/tests/ip_location/TransportIntegrationTest.php +++ b/tests/ip_location/TransportIntegrationTest.php @@ -90,8 +90,16 @@ public function testStalledResponsesHonorTheReadTimeout(string $path): void $server = $this->server(); $started = microtime(true); $this->expectException(RequestTimeoutException::class); + $this->expectExceptionMessageMatches('/^HTTP request timed out$/'); try { - (new GuzzleTransport())->sendRequest($this->request($server->url . $path)); + (new GuzzleTransport())->sendRequest(new HttpRequestData( + $server->url . $path, + 'GET', + ['Accept' => ['application/json']], + null, + 4.0, + 1.0 + )); } finally { self::assertLessThan(2.5, microtime(true) - $started, 'The transport must not wait for the 3-second response'); }