fix: match the demo endpoint on a segment boundary, not a raw prefix (#23)

[karlwaldman]

Closes #23

The defect, re-verified on current main (0672df0)

src/Client.php:293 decided keyless demo mode with a raw prefix match:

$isDemo = str_starts_with($path, '/v1/demo');

That is true for any path whose characters start that way, not just the demo
endpoint. Those requests ship with no Authorization header, the API
answers 401, and the SDK reports Invalid API key. to a customer whose key is
perfectly valid.

Path	Real endpoint	Pre-fix	Post-fix
/v1/demo/prices	demo	keyless	keyless (unchanged)
/v1/demo	demo root	keyless	keyless (unchanged)
/v1/demographics	authenticated	key withheld → 401	key sent
/v1/demo-prices	authenticated	key withheld → 401	key sent
/v1/demo/../prices/latest	/v1/prices/latest	key withheld → 401	key sent
/v1/demo/%2e%2e/prices/latest	/v1/prices/latest	key withheld → 401	key sent

Status: confirmed live on main today. Nothing in the 3.0.0 release moved
this code; the prefix match is still verbatim at line 293.

The contract chosen: segment boundary, and dot segments are never demo

Per the issue, stating the choice explicitly.

Chosen: segment-boundary match — $bare === '/v1/demo' or
str_starts_with($bare, '/v1/demo/'), where $bare is the path with any
query string or fragment stripped.

Why not an exact match on /v1/demo/prices: the server owns the demo
namespace. If a /v1/demo/latest ships tomorrow, an exact match makes the SDK
send a key to an endpoint that does not want one and silently regress keyless
mode for anyone on an older SDK. Segment boundary tracks the namespace.

One addition the boundary alone does not cover. /v1/demo/../prices/latest
does sit under /v1/demo/ by character, but the server resolves the dot
segments and answers from /v1/prices/latest, which needs the key. So any path
with a . or .. segment is never treated as demo. Percent-encoded forms
(%2e, %2E) are decoded before the check, because some servers decode before
resolving.

The failure directions are not symmetric, and the rule errs toward the cheap
one: treating a demo path as authenticated sends a valid key to our own origin
(already fenced by assertSameOrigin), while treating an authenticated path as
demo withholds the key and produces a wrong error message at the customer.

Keyless demo behaviour is preserved: demoPrices() still works with no key and
still sends no Authorization header even when a key is configured, and the
"No API key configured … keyless demo mode is available via
$client->demoPrices()" exception still fires for every non-demo path.

TDD evidence

New file tests/DemoPathDetectionTest.php — 14 tests. Two data-driven cases
over five non-demo paths (the key must be sent; a keyless client must still get
the helpful exception and send nothing), plus four regression guards on real
demo paths.

RED — test file against unmodified src/Client.php from origin/main

Produced by git checkout origin/main -- src/Client.php after the fix was
written, then ./vendor/bin/phpunit tests/DemoPathDetectionTest.php:

PHPUnit 13.3.3 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.5.8
Configuration: /private/tmp/sdkfix/php/phpunit.xml.dist

FFFFFEEEEE....                                                    14 / 14 (100%)

Time: 00:00.007, Memory: 18.00 MB

There were 5 errors:

1) OilPriceAPI\Tests\DemoPathDetectionTest::testNonDemoPathsStillDemandAKey with data set "sibling endpoint sharing the prefix" ('/v1/demographics')
RuntimeException: MockTransport queue exhausted for https://api.oilpriceapi.com/v1/demographics

--

There were 5 failures:

1) OilPriceAPI\Tests\DemoPathDetectionTest::testNonDemoPathsCarryTheApiKey with data set "sibling endpoint sharing the prefix" ('/v1/demographics')
/v1/demographics is not the demo endpoint and must be sent with the API key.
Failed asserting that an array has the key 'Authorization'.

ERRORS!
Tests: 14, Assertions: 11, Errors: 5, Failures: 5.

The red is exactly the defect: the Authorization header is absent from the
outbound request the fake transport recorded for /v1/demographics, and a
keyless client silently sent a request instead of raising.

GREEN — same test file, with the fix

PHPUnit 13.3.3 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.5.8
Configuration: /private/tmp/sdkfix/php/phpunit.xml.dist

..............                                                    14 / 14 (100%)

Time: 00:00.004, Memory: 18.00 MB

OK (14 tests, 26 assertions)

Full suite

	Result
Baseline, origin/main	OK (229 tests, 747 assertions)
This branch	OK (243 tests, 773 assertions)

No behaviour change outside Client::request()'s demo decision; no version
bump, no CHANGELOG entry (VersioningTest pins the top CHANGELOG heading to
Client::VERSION, so a release note belongs with the next release commit).

🤖 Generated with Claude Code

https://claude.ai/code/session_015ao5paex73xXvuM424Libo

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 58cc0c9e-b89d-4068-9b4f-7d336c0be61a

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}