From 98f3c93c4e5bdcccfb76371c66bc30e27a903fc5 Mon Sep 17 00:00:00 2001 From: Karl Waldman Date: Sun, 13 Sep 2026 12:54:44 -0400 Subject: [PATCH] fix: reject fabricated observation timestamps (#20) `Price::fromArray()` built the observation timestamp with `DateTimeImmutable::createFromFormat()` and never called `getLastErrors()`. PHP reports a repaired parse only through that method, so an impossible value came back as a perfectly usable object: "2026-13-45T99:99:99Z" -> 2027-02-18T04:40:39+00:00 "2026-02-30T00:00:00Z" -> 2026-03-02T00:00:00+00:00 The tolerant `new DateTimeImmutable($timestamp)` fallback was looser still and raised nothing at all for relative expressions: "now" -> "next friday" -> 2026-09-18T00:00:00+00:00 "0000-00-00" -> -0001-11-30T00:00:00+00:00 This is the same fabrication class as the zero price fixed in #18, and #18 did not cover it: its guard only caught an outright parse failure. A fabricated timestamp on a real price is the harder failure to catch, because the number is right and only its position in time is invented - nobody eyeballs that the way they eyeball a $0.00 Brent quote. Timestamps are now matched against an explicit list of absolute formats, and a parse counts only when `getLastErrors()` reports zero warnings and zero errors. Anything else raises `ApiException` naming the offending value. Two deliberate calls beyond the reported inputs: - Naive timestamps ("2026-07-19 12:00:00") are read as UTC, not as the host's default timezone, so the same payload does not describe a different instant on a server in Chicago than on one in London. - Leap seconds ("23:59:60") are rejected. PHP has no representation for one and rolls it into the next minute; accepting that would be a silent one-second shift, the same fabrication in miniature. Genuine spellings keep working: 'Z' and lowercase 'z', +00:00, +0000 and offset forms, fractional seconds, the space separator, and date-only values. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015ao5paex73xXvuM424Libo --- CHANGELOG.md | 11 ++ src/Price.php | 97 +++++++++++--- tests/FabricatedTimestampTest.php | 210 ++++++++++++++++++++++++++++++ 3 files changed, 303 insertions(+), 15 deletions(-) create mode 100644 tests/FabricatedTimestampTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 56147ab..78e701c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,17 @@ ### Fixed +- Reject fabricated observation timestamps. `Price::fromArray()` parsed the + `created_at`/`updated_at` field without ever consulting + `DateTimeImmutable::getLastErrors()`, so PHP silently repaired impossible + values into plausible dates - `2026-13-45T99:99:99Z` became + `2027-02-18T04:40:39Z` - and the tolerant constructor fallback accepted + relative expressions such as `now`, `next friday` and `+1 week`. Timestamps + are now matched against an explicit list of absolute formats and accepted + only on a parse that reports zero warnings and zero errors; anything else + raises `ApiException`. Naive timestamps are read as UTC rather than as the + host's local timezone, and leap seconds are rejected rather than rolled + silently into the next minute. - Reject malformed price rows instead of reporting them as `$0.00`. A row without a usable code or a numeric price, an unparseable timestamp, and a missing or non-array `prices` field now raise `ApiException` across diff --git a/src/Price.php b/src/Price.php index b1b1e79..eaa83be 100644 --- a/src/Price.php +++ b/src/Price.php @@ -6,6 +6,7 @@ use DateTimeImmutable; use DateTimeInterface; +use DateTimeZone; use OilPriceAPI\Exception\ApiException; /** @@ -21,6 +22,30 @@ */ final class Price { + /** + * The absolute timestamp spellings the API is allowed to speak. + * + * Every entry is anchored with '!' so unspecified fields reset to the + * epoch rather than to "now", and each is tried with `getLastErrors()` + * checked afterwards. Relative expressions ('now', 'next friday', + * '+1 week') are deliberately absent: the `DateTimeImmutable` constructor + * accepts them without any error at all, which is how a corrupt field + * became a plausible observation time. + * + * @var list + */ + private const TIMESTAMP_FORMATS = [ + '!Y-m-d\\TH:i:sP', // RFC 3339 / ATOM, including the 'Z' spelling + '!Y-m-d\\TH:i:s.uP', // RFC 3339 with fractional seconds + '!Y-m-d\\TH:i:s', // naive ISO 8601, read as UTC + '!Y-m-d\\TH:i:s.u', + '!Y-m-d H:i:sP', // space separator (Postgres / Rails #to_s) + '!Y-m-d H:i:s.uP', + '!Y-m-d H:i:s', + '!Y-m-d H:i:s.u', + '!Y-m-d', // date only + ]; + public function __construct( public readonly string $code, public readonly float $price, @@ -47,6 +72,11 @@ public function __construct( * real quote once it leaves the SDK. Legitimate zero and negative prices * are preserved. * + * The timestamp is held to the same standard. Only an unambiguous absolute + * value is accepted ({@see self::TIMESTAMP_FORMATS}); anything PHP would + * have to repair or interpret relative to "now" raises rather than + * producing a plausible-looking date. + * * @param array $data * * @throws ApiException when a required field is missing or unparseable @@ -71,21 +101,7 @@ public static function fromArray(array $data): self $timestamp = $data['created_at'] ?? $data['updated_at'] ?? null; $updatedAt = null; if (is_string($timestamp) && $timestamp !== '') { - $parsed = DateTimeImmutable::createFromFormat(DateTimeInterface::ATOM, $timestamp); - if ($parsed === false) { - try { - $parsed = new DateTimeImmutable($timestamp); - } catch (\Exception) { - $parsed = null; - } - } - if (!$parsed instanceof DateTimeImmutable) { - throw new ApiException(sprintf( - 'Price row for %s carries an unparseable timestamp.', - $data['code'], - )); - } - $updatedAt = $parsed; + $updatedAt = self::parseTimestamp($timestamp, $data['code']); } $change = $data['change_24h'] ?? $data['change_percent_24h'] ?? null; @@ -104,6 +120,57 @@ public static function fromArray(array $data): self ); } + /** + * Parse an observation timestamp, or refuse it. + * + * PHP will happily hand back a usable object for input it had to repair: + * `createFromFormat(ATOM, '2026-13-45T99:99:99Z')` returns + * 2027-02-18T04:40:39Z and reports the repair only through the static + * `getLastErrors()`. The constructor is looser still and accepts 'now', + * 'next friday' and '+1 week' with no error at all. Both paths turn a + * corrupt field into a plausible date, which misplaces a correct price in + * time - a failure nobody spots the way they spot a $0.00 Brent quote. + * + * So: an explicit list of absolute formats, and a parse only counts when + * `getLastErrors()` reports zero warnings AND zero errors. + * + * Leap seconds ('23:59:60') are rejected on purpose. PHP has no + * representation for one and rolls it into the next minute, so accepting + * it would be a silent one-second shift - the same fabrication in + * miniature. + * + * @throws ApiException when the value is not an unambiguous absolute timestamp + */ + private static function parseTimestamp(string $value, string $code): DateTimeImmutable + { + $utc = new DateTimeZone('UTC'); + + foreach (self::TIMESTAMP_FORMATS as $format) { + $parsed = DateTimeImmutable::createFromFormat($format, $value, $utc); + if (!$parsed instanceof DateTimeImmutable) { + continue; + } + + // getLastErrors() returns false when the parse was clean, and an + // array of counts when PHP had to warn (rolled-over date, trailing + // data) or error. Anything but a clean parse is a fabrication. + $errors = DateTimeImmutable::getLastErrors(); + if ($errors !== false && ($errors['warning_count'] > 0 || $errors['error_count'] > 0)) { + continue; + } + + return $parsed; + } + + throw new ApiException(sprintf( + 'Price row for %s carries an unparseable timestamp (%s); refusing to ' + . 'report a fabricated observation time. Expected an absolute ' + . 'timestamp such as 2026-07-19T12:00:00Z.', + $code, + var_export($value, true), + )); + } + /** * @return array */ diff --git a/tests/FabricatedTimestampTest.php b/tests/FabricatedTimestampTest.php new file mode 100644 index 0000000..6ff4f82 --- /dev/null +++ b/tests/FabricatedTimestampTest.php @@ -0,0 +1,210 @@ + + */ + public static function fabricatedTimestamps(): array + { + return [ + // Reported by getLastErrors() only: rolls over to 2027-02-18. + 'impossible date and time' => ['2026-13-45T99:99:99Z'], + 'month 13' => ['2026-13-01T00:00:00Z'], + 'day 45' => ['2026-07-45T00:00:00Z'], + 'february 30th' => ['2026-02-30T00:00:00Z'], + 'february 29th in a common year' => ['2026-02-29T00:00:00Z'], + 'hour 99' => ['2026-07-19T99:00:00Z'], + // Accepted silently by the DateTimeImmutable constructor. + 'relative next friday' => ['next friday'], + 'relative now' => ['now'], + 'relative tomorrow' => ['tomorrow'], + 'relative offset' => ['+1 week'], + 'relative yesterday noon' => ['yesterday noon'], + 'all zeroes' => ['0000-00-00'], + 'all zeroes with time' => ['0000-00-00 00:00:00'], + // Leap seconds have no representation in PHP; accepting one is a + // silent one-second shift into the following day. + 'leap second' => ['2026-06-30T23:59:60Z'], + 'leap second end of year' => ['2026-12-31T23:59:60Z'], + // Padding and trailing data are repaired, not rejected, by default. + 'trailing junk' => ['2026-07-19T12:00:00Zjunk'], + 'leading whitespace' => [' 2026-07-19T12:00:00Z'], + 'trailing whitespace' => ['2026-07-19T12:00:00Z '], + // Not a timestamp at all. + 'unix epoch seconds' => ['1690000000'], + 'free text' => ['not-a-date'], + 'null string' => ['null'], + 'iso duration' => ['P1Y2M3D'], + 'timezone name only' => ['UTC'], + ]; + } + + #[DataProvider('fabricatedTimestamps')] + public function testFabricatedTimestampIsRejectedRatherThanInvented(string $timestamp): void + { + try { + $price = Price::fromArray([ + 'code' => 'BRENT_CRUDE_USD', + 'price' => 71.80, + 'currency' => 'USD', + 'created_at' => $timestamp, + ]); + } catch (ApiException $e) { + $this->assertStringContainsString('BRENT_CRUDE_USD', $e->getMessage()); + + return; + } + + $this->fail(sprintf( + 'Timestamp %s was fabricated into %s instead of being rejected.', + var_export($timestamp, true), + var_export($price->updatedAt?->format(DateTimeInterface::ATOM), true), + )); + } + + /** + * Absolute timestamps the production API actually speaks must keep working, + * and must keep their exact instant - including the offset. + * + * @return array + */ + public static function genuineTimestamps(): array + { + return [ + 'atom with Z' => ['2026-07-19T12:00:00Z', '2026-07-19T12:00:00+00:00'], + 'atom lowercase z' => ['2026-07-19T12:00:00z', '2026-07-19T12:00:00+00:00'], + 'atom with numeric offset' => ['2026-07-19T12:00:00+00:00', '2026-07-19T12:00:00+00:00'], + 'atom with compact offset' => ['2026-07-19T12:00:00+0000', '2026-07-19T12:00:00+00:00'], + 'atom with negative offset' => ['2026-07-19T12:00:00-05:00', '2026-07-19T12:00:00-05:00'], + 'atom with positive offset' => ['2026-07-19T12:00:00+05:30', '2026-07-19T12:00:00+05:30'], + 'milliseconds' => ['2026-07-19T12:00:00.123Z', '2026-07-19T12:00:00+00:00'], + 'microseconds' => ['2026-07-19T12:00:00.123456Z', '2026-07-19T12:00:00+00:00'], + 'naive iso read as utc' => ['2026-07-19T12:00:00', '2026-07-19T12:00:00+00:00'], + 'space separator' => ['2026-07-19 12:00:00', '2026-07-19T12:00:00+00:00'], + 'space separator with offset' => ['2026-07-19 12:00:00+02:00', '2026-07-19T12:00:00+02:00'], + 'date only' => ['2026-07-19', '2026-07-19T00:00:00+00:00'], + 'leap day in a leap year' => ['2024-02-29T00:00:00Z', '2024-02-29T00:00:00+00:00'], + 'end of year' => ['2026-12-31T23:59:59Z', '2026-12-31T23:59:59+00:00'], + ]; + } + + #[DataProvider('genuineTimestamps')] + public function testGenuineTimestampsAreParsedExactly(string $timestamp, string $expected): void + { + $price = Price::fromArray([ + 'code' => 'BRENT_CRUDE_USD', + 'price' => 71.80, + 'currency' => 'USD', + 'created_at' => $timestamp, + ]); + + $this->assertNotNull($price->updatedAt); + $this->assertSame($expected, $price->updatedAt->format(DateTimeInterface::ATOM)); + } + + /** + * A naive timestamp is read as UTC, not as the host's local timezone: the + * same payload must not describe a different instant on a server in + * Chicago than on one in London. + */ + public function testNaiveTimestampDoesNotDependOnTheHostTimezone(): void + { + $original = date_default_timezone_get(); + date_default_timezone_set('America/Chicago'); + + try { + $price = Price::fromArray([ + 'code' => 'BRENT_CRUDE_USD', + 'price' => 71.80, + 'currency' => 'USD', + 'created_at' => '2026-07-19 12:00:00', + ]); + + $this->assertNotNull($price->updatedAt); + $this->assertSame( + '2026-07-19T12:00:00+00:00', + $price->updatedAt->format(DateTimeInterface::ATOM), + ); + } finally { + date_default_timezone_set($original); + } + } + + /** + * A row with no timestamp at all is still valid: updatedAt is nullable. + */ + public function testAbsentTimestampStaysNull(): void + { + $price = Price::fromArray([ + 'code' => 'BRENT_CRUDE_USD', + 'price' => 71.80, + 'currency' => 'USD', + ]); + + $this->assertNull($price->updatedAt); + } + + /** + * `updated_at` is the documented fallback when `created_at` is absent, and + * it must be held to the same standard. + */ + public function testUpdatedAtFallbackIsValidatedToo(): void + { + $this->expectException(ApiException::class); + + Price::fromArray([ + 'code' => 'BRENT_CRUDE_USD', + 'price' => 71.80, + 'currency' => 'USD', + 'updated_at' => 'next friday', + ]); + } + + /** + * The whole point: a fabricated timestamp must not reach a caller through + * the client, on a response that is otherwise a perfectly good price. + */ + public function testClientRefusesARowWithAFabricatedTimestamp(): void + { + $transport = new MockTransport(); + $transport->queue(200, [ + 'status' => 'success', + 'data' => [ + 'code' => 'BRENT_CRUDE_USD', + 'price' => 71.80, + 'currency' => 'USD', + 'created_at' => '2026-13-45T99:99:99Z', + ], + ]); + $client = new Client('fixture_key_NOT_REAL_0123456789', Client::DEFAULT_BASE_URL, 10.0, 0, $transport); + + $this->expectException(ApiException::class); + + $client->latest('BRENT_CRUDE_USD'); + } +}