Skip to content

[P1] Price::fromArray fabricates timestamps: 2026-13-45T99:99:99Z becomes 2027-02-18 #20

Description

@karlwaldman

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:

  1. DateTimeImmutable::createFromFormat(DateTimeInterface::ATOM, '2026-13-45T99:99:99Z') returns a valid object, not false. It rolls the out-of-range fields over and reports the problem only through DateTimeImmutable::getLastErrors(), which the code never calls:
var_dump(DateTimeImmutable::getLastErrors());
// ['warning_count' => 1, 'warnings' => [20 => 'The parsed date was invalid'], ...]
  1. 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() after createFromFormat() and reject on any warning_count or error_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 updatedAt should fail the whole response the way a bad price does — for pastYear() 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    P1Priority 1 - complete in the active sprintbugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions