Skip to content

[P2] Price::fromArray still blind-casts currency/unit/name/source/type/formatted: currency ['EUR'] becomes 'Array' #21

Description

@karlwaldman

Confirmed live on main (3feb505) and still present on #18 as proposed. Verified by execution on PHP 8.5.8, 2026-09-13, and independently by PHPStan level 9.

#18 correctly stops price and code being coerced. The other six fields are still blind (string) casts on untrusted JSON, so a malformed row produces a mislabelled price instead of a fabricated one. For energy data a wrong currency or a wrong unit is the same customer harm as a wrong number: a barrel price read as a tonne price, or a EUR carbon price labelled USD, is silently ~8-700% wrong and looks completely normal.

Repro

<?php
require 'vendor/autoload.php';
use OilPriceAPI\Price;

foreach ([
    'currency array' => ['code' => 'B', 'price' => 1.0, 'currency' => ['EUR']],
    'currency bool'  => ['code' => 'B', 'price' => 1.0, 'currency' => true],
    'currency int'   => ['code' => 'B', 'price' => 1.0, 'currency' => 978],
    'currency ""'    => ['code' => 'B', 'price' => 1.0, 'currency' => ''],
    'unit array'     => ['code' => 'B', 'price' => 1.0, 'unit' => ['bbl']],
] as $label => $row) {
    $p = Price::fromArray($row);
    printf("%-16s currency=%-8s unit=%s\n", $label, var_export($p->currency, true), var_export($p->unit, true));
}

Output on main and on fix/15-reject-malformed-price-rows (cc74b0e):

PHP Warning:  Array to string conversion
currency array   currency='Array'  unit=NULL
currency bool    currency='1'      unit=NULL
currency int     currency='978'    unit=NULL
currency ""      currency=''       unit=NULL
PHP Warning:  Array to string conversion
unit array       currency='USD'    unit='Array'

src/Price.php:65-75 on main, :96-103 on #18 — currency, name, unit, source, type, formatted.

Note the PHP Warning, not an exception: in production this is a log line and the object is still handed to the caller. Under phpunit.xml.dist's failOnWarning="true" it would fail a test, which is why no existing test covers it — none of them feed a non-string.

Independent confirmation

PHPStan level 9 (not currently configured in this repo — see the packaging issue) reports exactly these six lines:

src/Price.php:96   Cannot cast mixed to string.  (cast.string)
src/Price.php:99   Cannot cast mixed to string.
src/Price.php:100  Cannot cast mixed to string.
src/Price.php:101  Cannot cast mixed to string.
src/Price.php:102  Cannot cast mixed to string.
src/Price.php:103  Cannot cast mixed to string.

Suggested fix

Validate rather than cast: is_string($data['currency']) ? $data['currency'] : <reject or null>, and the same for the five optional strings (where a non-string should become null, not "Array").

Open design question, for Karl — not changed here

currency still defaults to 'USD' when absent (src/Price.php:67 main / :96 #18). This repo's own test fixture at tests/ClientTest.php:404 uses EU_CARBON_EUR, so the catalogue is not USD-only and the default can mislabel a EUR series. My recommendation is to make currency required alongside code and price — #18 already breaks BC on this method, so the extra strictness is free — but this is a product call, not a code call.

Related: #15, PR #18. Found during the post-merge PHP expert review of #17/#18/#19.

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

    P2Priority 2 - next sprintbugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions