diff --git a/.changeset/fix-dot-segment-path-parameter-encoding.md b/.changeset/fix-dot-segment-path-parameter-encoding.md new file mode 100644 index 000000000..df7f25ef0 --- /dev/null +++ b/.changeset/fix-dot-segment-path-parameter-encoding.md @@ -0,0 +1,5 @@ +--- +"@fingerprint/php-sdk": patch +--- + +Fixed `event_id`/`visitor_id` values of exactly `.` or `..` being collapsed by curl's URL normalization before the request is sent, causing `getEvent`, `updateEvent`, and `deleteVisitorData` to hit the wrong endpoint instead of the requested resource. diff --git a/src/Api/FingerprintApi.php b/src/Api/FingerprintApi.php index 45c127c04..ef5f01727 100644 --- a/src/Api/FingerprintApi.php +++ b/src/Api/FingerprintApi.php @@ -1690,7 +1690,17 @@ public function updateEventRequest(string $event_id, EventUpdate $event_update): */ protected function createHttpClientOption(): array { - $options = []; + // Path parameters are percent-encoded (see ObjectSerializer::toPathValue()), + // but curl still decodes and collapses RFC 3986 dot-segments (e.g. a + // literal '.' or '..' path parameter) before the request is sent unless + // explicitly told not to. CURLOPT_PATH_AS_IS makes curl transmit the URL + // exactly as built, so an event/visitor ID can never be misrouted to a + // different endpoint via path normalization. + $options = [ + 'curl' => [ + \CURLOPT_PATH_AS_IS => true, + ], + ]; if ($this->config->getDebug()) { $options[RequestOptions::DEBUG] = fopen($this->config->getDebugFile(), 'a'); if (!$options[RequestOptions::DEBUG]) { diff --git a/src/ObjectSerializer.php b/src/ObjectSerializer.php index c3a5a822c..0655275fa 100644 --- a/src/ObjectSerializer.php +++ b/src/ObjectSerializer.php @@ -145,7 +145,18 @@ public static function sanitizeTimestamp(string $timestamp): string */ public static function toPathValue(string $value): string { - return rawurlencode(self::toString($value)); + $encoded = rawurlencode(self::toString($value)); + + // '.' and '..' are RFC 3986 dot-segments: rawurlencode() leaves the + // literal dots untouched, but URL normalizers (e.g. curl, before the + // request ever hits the wire) collapse a path segment consisting + // solely of dots into '' or 'up one level'. Escaping the dots here + // keeps the segment inert without changing any other encoded value. + if ('.' === $encoded || '..' === $encoded) { + $encoded = str_replace('.', '%2E', $encoded); + } + + return $encoded; } /** diff --git a/template/ObjectSerializer.mustache b/template/ObjectSerializer.mustache index 2f5207b3b..b16d842f3 100644 --- a/template/ObjectSerializer.mustache +++ b/template/ObjectSerializer.mustache @@ -132,7 +132,18 @@ class ObjectSerializer */ public static function toPathValue(string $value): string { - return rawurlencode(self::toString($value)); + $encoded = rawurlencode(self::toString($value)); + + // '.' and '..' are RFC 3986 dot-segments: rawurlencode() leaves the + // literal dots untouched, but URL normalizers (e.g. curl, before the + // request ever hits the wire) collapse a path segment consisting + // solely of dots into '' or 'up one level'. Escaping the dots here + // keeps the segment inert without changing any other encoded value. + if ('.' === $encoded || '..' === $encoded) { + $encoded = str_replace('.', '%2E', $encoded); + } + + return $encoded; } /** diff --git a/template/api.mustache b/template/api.mustache index 7c2f20259..ab6b7296a 100644 --- a/template/api.mustache +++ b/template/api.mustache @@ -474,7 +474,17 @@ use {{invokerPackage}}\ObjectSerializer; */ protected function createHttpClientOption(): array { - $options = []; + // Path parameters are percent-encoded (see ObjectSerializer::toPathValue()), + // but curl still decodes and collapses RFC 3986 dot-segments (e.g. a + // literal '.' or '..' path parameter) before the request is sent unless + // explicitly told not to. CURLOPT_PATH_AS_IS makes curl transmit the URL + // exactly as built, so an event/visitor ID can never be misrouted to a + // different endpoint via path normalization. + $options = [ + 'curl' => [ + \CURLOPT_PATH_AS_IS => true, + ], + ]; if ($this->config->getDebug()) { $options[RequestOptions::DEBUG] = fopen($this->config->getDebugFile(), 'a'); if (!$options[RequestOptions::DEBUG]) { diff --git a/test/Api/FingerprintApiTest.php b/test/Api/FingerprintApiTest.php index d08c461ec..148671a19 100644 --- a/test/Api/FingerprintApiTest.php +++ b/test/Api/FingerprintApiTest.php @@ -26,6 +26,7 @@ use Fingerprint\ServerSdk\Model\SearchEventsVpnConfidence; use Fingerprint\ServerSdk\Model\SupplementaryIDHighRecall; use Fingerprint\ServerSdk\Test\MockHelper; +use Fingerprint\ServerSdk\Test\Support\RawRequestCapture; use GuzzleHttp\Client; use GuzzleHttp\Exception\ConnectException; use GuzzleHttp\Exception\GuzzleException; @@ -35,6 +36,8 @@ use GuzzleHttp\Psr7\Response; use GuzzleHttp\Utils; use PHPUnit\Framework\Attributes\CoversClass; +use PHPUnit\Framework\Attributes\DataProvider; +use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\TestCase; use Psr\Http\Message\RequestInterface; @@ -1514,6 +1517,90 @@ public function testUpdateEventNon2xxStatusCode(): void $this->api->updateEvent('test', new EventUpdate()); } + /** + * Verifies a malformed path parameter (path traversal, an absolute URL, + * a bare dot-segment, or an empty string) is always percent-encoded into + * a single opaque path segment rather than being interpreted as part of + * the path structure, across every operation that takes an ID in the path. + */ + #[DataProvider('pathParameterEncodingProvider')] + public function testPathParameterIsEncodedAsSingleOpaqueSegment(\Closure $buildRequest, string $value, string $expectedPath): void + { + $request = $buildRequest($this->api, $value); + + $this->assertSame('api.fpjs.io', $request->getUri()->getHost()); + $this->assertSame($expectedPath, $request->getUri()->getPath()); + } + + public static function pathParameterEncodingProvider(): iterable + { + $endpoints = [ + 'getEventRequest' => ['/v4/events/', static fn (FingerprintApi $api, string $id) => $api->getEventRequest($id)], + 'updateEventRequest' => ['/v4/events/', static fn (FingerprintApi $api, string $id) => $api->updateEventRequest($id, new EventUpdate())], + 'deleteVisitorDataRequest' => ['/v4/visitors/', static fn (FingerprintApi $api, string $id) => $api->deleteVisitorDataRequest($id)], + ]; + + $values = [ + 'path traversal' => ['../events', '..%2Fevents'], + 'nested path traversal' => ['../../events', '..%2F..%2Fevents'], + 'absolute url' => ['https://domain.tld/evil', 'https%3A%2F%2Fdomain.tld%2Fevil'], + 'dot segment' => ['.', '%2E'], + 'parent dot segment' => ['..', '%2E%2E'], + 'empty' => ['', ''], + ]; + + foreach ($endpoints as $endpoint => [$prefix, $call]) { + foreach ($values as $case => [$value, $encoded]) { + yield "{$endpoint}: {$case}" => [$call, $value, $prefix.$encoded]; + } + } + } + + /** + * Regression test for the actual wire-level bug: a PSR-7 Uri never + * normalizes dot-segments (asserting on $request->getUri()->getPath() + * alone would pass even without ObjectSerializer's encoding fix), but + * curl decodes and collapses them just before sending unless + * CURLOPT_PATH_AS_IS is set. This spins up a real local TCP listener and + * checks the literal bytes a real Guzzle+curl request puts on the wire, + * so it fails if either half of the fix (percent-encoding in + * ObjectSerializer::toPathValue, or CURLOPT_PATH_AS_IS in + * createHttpClientOption) is reverted. + */ + #[DataProvider('dotSegmentWireProvider')] + #[Group('wire')] + public function testDotSegmentIsNotCollapsedOnTheWire(\Closure $call, string $expectedPrefix): void + { + $capture = RawRequestCapture::start(); + + try { + $config = new Configuration('test-api-key'); + $config->setHost($capture->baseUri().'/v4'); + $api = new FingerprintApi($config, new Client(['timeout' => 10])); + + try { + $call($api); + } catch (\Throwable $e) { + // Only the request line on the wire matters for this test. + } + + $this->assertStringStartsWith($expectedPrefix, $capture->requestLine()); + } finally { + $capture->stop(); + } + } + + public static function dotSegmentWireProvider(): iterable + { + yield 'getEvent: dot segment' => [static fn (FingerprintApi $api) => $api->getEvent('.'), 'GET /v4/events/%2E?']; + + yield 'getEvent: parent dot segment' => [static fn (FingerprintApi $api) => $api->getEvent('..'), 'GET /v4/events/%2E%2E?']; + + yield 'updateEvent: dot segment' => [static fn (FingerprintApi $api) => $api->updateEvent('.', new EventUpdate()), 'PATCH /v4/events/%2E?']; + + yield 'deleteVisitorData: dot segment' => [static fn (FingerprintApi $api) => $api->deleteVisitorData('.'), 'DELETE /v4/visitors/%2E?']; + } + private function parseQueryString(string $query): array { $queryArray = []; diff --git a/test/ObjectSerializerTest.php b/test/ObjectSerializerTest.php index 4039d9e1f..96f4dc793 100644 --- a/test/ObjectSerializerTest.php +++ b/test/ObjectSerializerTest.php @@ -302,6 +302,69 @@ public function testToPathValue(): void $this->assertSame('simple', ObjectSerializer::toPathValue('simple')); } + /** + * Verifies path traversal sequences are encoded so a single path segment + * cannot escape into a sibling resource (e.g. `../events`). + */ + public function testToPathValueEncodesPathTraversal(): void + { + $this->assertSame('..%2Fevents', ObjectSerializer::toPathValue('../events')); + $this->assertSame('..%2F..%2Fevents', ObjectSerializer::toPathValue('../../events')); + } + + /** + * Verifies slashes are always encoded, since an un-encoded slash would let + * a path parameter inject extra path segments. + */ + public function testToPathValueEncodesSlash(): void + { + $this->assertSame('abc%2Fdef', ObjectSerializer::toPathValue('abc/def')); + } + + /** + * A value that looks like an absolute URL must not be able to redirect + * the request elsewhere; its scheme and slashes are encoded so it stays + * a single, inert path segment. + */ + public function testToPathValueEncodesAbsoluteUrlValue(): void + { + $this->assertSame('https%3A%2F%2Fdomain.tld%2Fevil', ObjectSerializer::toPathValue('https://domain.tld/evil')); + } + + public function testToPathValueWithEmptyString(): void + { + $this->assertSame('', ObjectSerializer::toPathValue('')); + } + + /** + * A path parameter of exactly '.' or '..' is an RFC 3986 dot-segment: + * left as a literal dot, URL normalizers (including curl, before the + * request ever reaches the wire — see RawRequestCapture-based tests in + * FingerprintApiTest) collapse it into the parent/current path instead + * of treating it as an opaque resource identifier. The dots must be + * percent-encoded so no normalizer can mistake the segment for one. + */ + public function testToPathValueEncodesDotSegment(): void + { + $this->assertSame('%2E', ObjectSerializer::toPathValue('.')); + } + + public function testToPathValueEncodesDotDotSegment(): void + { + $this->assertSame('%2E%2E', ObjectSerializer::toPathValue('..')); + } + + /** + * Only a segment consisting solely of dots is special under RFC 3986; + * anything else containing a dot (e.g. a real event ID) must pass + * through unencoded, since '.' is otherwise a safe, unreserved character. + */ + public function testToPathValueDoesNotEncodeDotsInOtherwiseNormalValues(): void + { + $this->assertSame('1708102555327.NLOjmg', ObjectSerializer::toPathValue('1708102555327.NLOjmg')); + $this->assertSame('...', ObjectSerializer::toPathValue('...')); + } + // -- toHeaderValue -- public function testToHeaderValueWithString(): void diff --git a/test/Support/RawRequestCapture.php b/test/Support/RawRequestCapture.php new file mode 100644 index 000000000..d3188487d --- /dev/null +++ b/test/Support/RawRequestCapture.php @@ -0,0 +1,130 @@ +getUri()->getPath() cannot reveal that curl collapses a bare + * '.' or '..' path segment before the bytes leave the process. This spins + * up a plain TCP listener in a child process and hands back the raw + * request line it received, so tests can assert on what the transport + * actually sent rather than what the PHP object model says it sent. + * + * @internal + */ +final class RawRequestCapture +{ + private $process; + + /** @var resource */ + private $stdout; + + /** @var resource */ + private $stderr; + + private string $buffer = ''; + + private int $port; + + /** + * @param resource $process + * @param resource $stdout + * @param resource $stderr + */ + private function __construct($process, $stdout, $stderr) + { + $this->process = $process; + $this->stdout = $stdout; + $this->stderr = $stderr; + } + + public static function start(): self + { + $process = proc_open( + [PHP_BINARY, __DIR__.'/raw_request_listener.php'], + [1 => ['pipe', 'w'], 2 => ['pipe', 'w']], + $pipes + ); + + if (!\is_resource($process)) { + throw new \RuntimeException('Failed to start raw request listener.'); + } + + foreach ($pipes as $pipe) { + stream_set_blocking($pipe, false); + } + + $capture = new self($process, $pipes[1], $pipes[2]); + + try { + $ready = $capture->readLine(10.0); + if (!str_starts_with($ready, 'READY ')) { + throw new \RuntimeException("Unexpected listener handshake: {$ready}"); + } + $capture->port = (int) substr($ready, 6); + } catch (\Throwable $e) { + $capture->stop(); + + throw $e; + } + + return $capture; + } + + public function baseUri(): string + { + return "http://127.0.0.1:{$this->port}"; + } + + /** + * Returns the raw request line (e.g. "GET /v4/events/. HTTP/1.1") + * once the listener has accepted and read one request. + */ + public function requestLine(): string + { + return $this->readLine(10.0); + } + + public function stop(): void + { + if (\is_resource($this->process)) { + proc_terminate($this->process); + proc_close($this->process); + } + } + + private function readLine(float $timeout): string + { + $deadline = microtime(true) + $timeout; + + while (false === ($eol = strpos($this->buffer, "\n"))) { + $chunk = fread($this->stdout, 8192); + if (false !== $chunk && '' !== $chunk) { + $this->buffer .= $chunk; + + continue; + } + if (feof($this->stdout)) { + throw new \RuntimeException('Raw request listener exited early. stderr: '.$this->stderrTail()); + } + if (microtime(true) >= $deadline) { + throw new \RuntimeException('Timed out reading from the raw request listener. stderr: '.$this->stderrTail()); + } + usleep(5000); + } + + $line = substr($this->buffer, 0, $eol); + $this->buffer = substr($this->buffer, $eol + 1); + + return rtrim($line, "\r"); + } + + private function stderrTail(): string + { + $stderr = (string) @stream_get_contents($this->stderr); + + return '' === $stderr ? '(empty)' : $stderr; + } +} diff --git a/test/Support/raw_request_listener.php b/test/Support/raw_request_listener.php new file mode 100644 index 000000000..aa9835357 --- /dev/null +++ b/test/Support/raw_request_listener.php @@ -0,0 +1,45 @@ +