laminas / laminas-stratigility

PSR-7 middleware foundation for building and dispatching middleware pipelines
https://docs.laminas.dev/laminas-stratigility/
BSD 3-Clause "New" or "Revised" License
55 stars 12 forks source link

Utils::getStatusCode() can throw if the passed $error->getCode() returns certain strings. #14

Closed timdev closed 3 years ago

timdev commented 3 years ago

Bug Report

Q A
Version(s) Any since 3.0alpha1

Summary

Utils::getStatusCode() incorrectly assumes that the code value returned from $error->getCode() will be an integer.

If you only look at Throwable that would seem correct. However, both Error and Exception define getCode(): mixed.

In the wild, PDOException can return a string code. For example, if PDO encounters a missing table, it will throw with a message of "SQLSTATE[42S02]: Base table or view not found: 1146 Table 'db.table_name' doesn't exist" and a code of '42S02'. The string then forces the literal 400 and 600 to be coerced to strings in the expression on line 33, which causes the expression to evaluate to true.

The method then attempts to return $errorCode (which is a string), which violates the return type declaration. Kaboom.

Current behavior

getStatusCode() throws with a message of Laminas\Stratigility\Utils::getStatusCode(): Return value must be of type int, string returned

How to reproduce

Throw an exception where getCode() returns a string starting with '4' or '5' followed by at least one letter.

Expected behavior

Not entirely sure. I certainly don't expect it to throw. I'm not sure how the method is used elsewhere.

Unfortunately, it's part of stratigility's public API. For instance, it's used in three classes in mezzio.

I suspect the intended behavior is that non-integer codes from the $error->getCode() should result in the conditional evaluating false. If that's right, then prepending is_int($errorCode) && to the expression in the if statement on line 33 ought to do it.

timdev commented 3 years ago

And here's a fun thing: While PDOException is allowed to return a string from getCode(), PHP makes it really hard (perhaps impossible) to do something similar in userland. You can't implement Throwable, and getCode() is final on Error and Exception. Joy.