From ed53dc45c6c0b9583570387b6ee931c5717e5a20 Mon Sep 17 00:00:00 2001 From: Karl Waldman Date: Sun, 13 Sep 2026 14:42:17 -0400 Subject: [PATCH] fix: match the demo endpoint on a segment boundary, not a raw prefix (#23) `str_starts_with($path, '/v1/demo')` also matched paths that merely start with those characters but are answered by an authenticated endpoint: /v1/demographics, /v1/demo-prices, and /v1/demo/../prices/latest (which the server resolves to /v1/prices/latest). Those requests went out with no Authorization header, came back 401, and the SDK reported "Invalid API key." to callers whose key was perfectly valid. Demo detection is now a segment-boundary match: exactly /v1/demo, or a path below /v1/demo/. Any path containing a . or .. segment - percent-encoded forms included - is never treated as demo, because the characters we inspect are not the endpoint that answers. Keyless demo mode is unchanged: demoPrices() still sends no Authorization header and still works with no key, and the "No API key configured" guidance still fires for every non-demo path. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015ao5paex73xXvuM424Libo --- src/Client.php | 41 ++++++++- tests/DemoPathDetectionTest.php | 150 ++++++++++++++++++++++++++++++++ 2 files changed, 190 insertions(+), 1 deletion(-) create mode 100644 tests/DemoPathDetectionTest.php diff --git a/src/Client.php b/src/Client.php index f81be4a..0f7997b 100644 --- a/src/Client.php +++ b/src/Client.php @@ -36,6 +36,12 @@ final class Client /** Maximum backoff sleep between retries, in seconds. */ private const MAX_BACKOFF_SECONDS = 30.0; + /** + * The one endpoint family that answers without an API key. Matched on a + * segment boundary - see {@see self::isDemoPath()} - never as a raw prefix. + */ + private const DEMO_PATH_ROOT = '/v1/demo'; + /** * Error codes for limits that do not refill inside any backoff this client * could sleep. Retrying one of these cannot succeed - it only spends more @@ -290,7 +296,7 @@ private function malformedRowMessage(string $path, string $detail): string private function request(string $rawPath, array $params): array { $path = $this->normalizeApiPath($rawPath); - $isDemo = str_starts_with($path, '/v1/demo'); + $isDemo = self::isDemoPath($path); if (!$isDemo && $this->apiKey === null) { throw new AuthenticationException( @@ -352,6 +358,39 @@ private function request(string $rawPath, array $params): array * attaches 'Authorization: Token '). Reject those before the * credential is ever assembled. */ + /** + * Is this path served by keyless demo mode? + * + * Matched on a segment boundary: exactly self::DEMO_PATH_ROOT, or a path + * below it. A raw prefix match also swallowed paths that merely start with + * the same characters but are answered by an authenticated endpoint - + * /v1/demographics, /v1/demo-prices - which then went out with no + * Authorization header, came back 401, and were reported to the caller as + * "Invalid API key." about a key that was perfectly valid (#23). + * + * Dot segments are never demo. The server resolves them, so the path we + * inspect is not the endpoint that answers: /v1/demo/../prices/latest is + * really /v1/prices/latest and needs the key. Percent-encoded forms count, + * because some servers decode before resolving. Erring toward "not demo" + * is the safe direction - the worst case is sending a valid key to our own + * origin (already guarded by assertSameOrigin), rather than withholding it. + */ + private static function isDemoPath(string $path): bool + { + // Only the part before the query/fragment names the endpoint. + $bare = substr($path, 0, strcspn($path, '?#')); + + foreach (explode('/', $bare) as $segment) { + $decoded = rawurldecode($segment); + if ($decoded === '.' || $decoded === '..') { + return false; + } + } + + return $bare === self::DEMO_PATH_ROOT + || str_starts_with($bare, self::DEMO_PATH_ROOT . '/'); + } + private function normalizeApiPath(string $path): string { if ($path === '') { diff --git a/tests/DemoPathDetectionTest.php b/tests/DemoPathDetectionTest.php new file mode 100644 index 0000000..f889a3c --- /dev/null +++ b/tests/DemoPathDetectionTest.php @@ -0,0 +1,150 @@ +` earns a 401, which the + * SDK then reports to the caller as "Invalid API key." - about a key that is + * perfectly valid. The key must travel with every non-demo request. + */ +final class DemoPathDetectionTest extends TestCase +{ + private const KEY = 'live_key_123'; + + private MockTransport $transport; + + private string|false $originalEnvKey = false; + + protected function setUp(): void + { + $this->transport = new MockTransport(); + $this->originalEnvKey = getenv('OILPRICEAPI_KEY'); + putenv('OILPRICEAPI_KEY'); + } + + protected function tearDown(): void + { + if ($this->originalEnvKey === false) { + putenv('OILPRICEAPI_KEY'); + } else { + putenv('OILPRICEAPI_KEY=' . $this->originalEnvKey); + } + } + + private function client(?string $apiKey = self::KEY): Client + { + return new Client( + apiKey: $apiKey, + timeout: 10.0, + maxRetries: 0, + transport: $this->transport, + sleeper: static function (float $seconds): void { + }, + ); + } + + /** + * @return iterable + */ + public static function nonDemoPathProvider(): iterable + { + yield 'sibling endpoint sharing the prefix' => ['/v1/demographics']; + yield 'prefix plus a suffix, no separator' => ['/v1/demo-prices']; + yield 'dot segments that resolve away from demo' => ['/v1/demo/../prices/latest']; + yield 'percent-encoded dot segments' => ['/v1/demo/%2e%2e/prices/latest']; + yield 'single dot segment' => ['/v1/demo/./../prices/latest']; + } + + #[DataProvider('nonDemoPathProvider')] + public function testNonDemoPathsCarryTheApiKey(string $path): void + { + $this->transport->queue(200, ['status' => 'success', 'data' => []]); + + $this->client()->raw()->get($path); + + $request = $this->transport->requests[0]; + $this->assertArrayHasKey( + 'Authorization', + $request['headers'], + $path . ' is not the demo endpoint and must be sent with the API key.', + ); + $this->assertSame('Token ' . self::KEY, $request['headers']['Authorization']); + } + + #[DataProvider('nonDemoPathProvider')] + public function testNonDemoPathsStillDemandAKey(string $path): void + { + $client = $this->client(apiKey: null); + + try { + $client->raw()->get($path); + $this->fail('Expected AuthenticationException for keyless ' . $path); + } catch (AuthenticationException $e) { + $this->assertStringContainsString('No API key configured', $e->getMessage()); + } + + $this->assertSame(0, $this->transport->requestCount(), 'No request should leave without a key.'); + } + + public function testDemoPricesStillWorksWithoutAKey(): void + { + $this->transport->queue(200, [ + 'status' => 'success', + 'data' => [ + 'prices' => [ + ['code' => 'BRENT_CRUDE_USD', 'name' => 'Brent Crude', 'price' => 71.23, 'currency' => 'USD', 'unit' => 'barrel'], + ], + 'meta' => ['demo_mode' => true, 'rate_limit' => '20/hour'], + ], + ]); + + $prices = $this->client(apiKey: null)->demoPrices(); + + $this->assertCount(1, $prices); + $request = $this->transport->requests[0]; + $this->assertSame('https://api.oilpriceapi.com/v1/demo/prices', $request['url']); + $this->assertArrayNotHasKey('Authorization', $request['headers']); + } + + public function testDemoPathStaysKeylessEvenWhenAKeyIsConfigured(): void + { + $this->transport->queue(200, ['status' => 'success', 'data' => []]); + + $this->client()->raw()->get('/v1/demo/prices'); + + $this->assertArrayNotHasKey('Authorization', $this->transport->requests[0]['headers']); + } + + public function testDemoPathWithAQueryStringStaysKeyless(): void + { + $this->transport->queue(200, ['status' => 'success', 'data' => []]); + + $this->client(apiKey: null)->raw()->get('/v1/demo/prices?by_code=BRENT_CRUDE_USD'); + + $this->assertArrayNotHasKey('Authorization', $this->transport->requests[0]['headers']); + } + + public function testBareDemoSegmentStaysKeyless(): void + { + $this->transport->queue(200, ['status' => 'success', 'data' => []]); + + $this->client(apiKey: null)->raw()->get('/v1/demo'); + + $this->assertArrayNotHasKey('Authorization', $this->transport->requests[0]['headers']); + } +}