Repository navigation
[P1] Price::fromArray fabricates timestamps: 2026-13-45T99:99:99Z becomes 2027-02-18 #20
Description
Confirmed live on main (3feb505) and NOT fixed by #18 as proposed. Verified by execution on PHP 8.5.8 / PHPUnit 13.3.3, 2026-09-13.
Price::fromArray() turns unparseable and out-of-range timestamps into confident, plausible-looking dates. For source-timestamped market data a manufactured timestamp is the same defect class as the manufactured $0.00 that #15/#18 removes — arguably worse, because a wrong date on a correct price is invisible downstream.
Repro
<?php
require 'vendor/autoload.php';
use OilPriceAPI\Price;
foreach ([
'2026-13-45T99:99:99Z', // month 13, day 45, 99:99:99
'next friday',
'now',
'0000-00-00',
] as $ts) {
$p = Price::fromArray(['code' => 'BRENT_CRUDE_USD', 'price' => 71.8, 'created_at' => $ts]);
printf("%-24s => %s\n", $ts, $p->updatedAt?->format(DATE_ATOM) ?? 'null');
}Output on main and on fix/15-reject-malformed-price-rows (cc74b0e):
2026-13-45T99:99:99Z => 2027-02-18T04:40:39+00:00
next friday => 2026-09-18T00:00:00+00:00
now => 2026-09-13T16:39:39+00:00
0000-00-00 => -0001-11-30T00:00:00+00:00
Why it happens (src/Price.php:51-60 on main, :63-88 on #18)
Two separate PHP behaviours, both silent:
DateTimeImmutable::createFromFormat(DateTimeInterface::ATOM, '2026-13-45T99:99:99Z')returns a valid object, notfalse. It rolls the out-of-range fields over and reports the problem only throughDateTimeImmutable::getLastErrors(), which the code never calls:
var_dump(DateTimeImmutable::getLastErrors());
// ['warning_count' => 1, 'warnings' => [20 => 'The parsed date was invalid'], ...]- The fallback
new DateTimeImmutable($timestamp)accepts PHP's entire relative-date grammar, so"now","next friday","+1 day"and"0000-00-00"all produce a date rather than throwing.
#18 added if (!$parsed instanceof DateTimeImmutable) { throw ... }, which only catches the case where parsing fails outright. Its test covers one input, 'not-a-date'. Every case above returns an object and sails through.
Suggested fix
- Call
DateTimeImmutable::getLastErrors()aftercreateFromFormat()and reject on anywarning_countorerror_count> 0. - Drop the
new DateTimeImmutable($ts)catch-all, or restrict it to an explicit allowlist of formats (ATOM, RFC3339 with fractional seconds,Y-m-d H:i:s). Anything outside the list is a malformed row. - Decide deliberately whether a bad
updatedAtshould fail the whole response the way a badpricedoes — forpastYear()that discards thousands of good rows for one bad one.
Related: #15, PR #18 (this is a gap that PR leaves open, not a regression it introduces).
Found during the post-merge PHP expert review of #17/#18/#19. Full review: review-php.md.
Activity
fix: reject fabricated observation timestamps (#20) (#25)
docs: list every 3.0.0 break under Breaking changes, not just the first
chore: release as 3.0.0 and write down the BC break #18 shipped (#28)