zendframework / zend-diactoros

PSR-7 HTTP Message implementation
BSD 3-Clause "New" or "Revised" License
1.55k stars 152 forks source link

Stream::setStream method causes PHP bug #63206 #298

Closed ste93cry closed 6 years ago

ste93cry commented 6 years ago

While using Zend Diactoros from inside an error handler I found out that the next errors were not logged anymore through my custom error handler. The cause is the PHP bug #63206 raised by the following code:

https://github.com/zendframework/zend-diactoros/blob/bb833f8e92297be103bf6cd3fc25219474bbf0d0/src/Stream.php#L310-L314

There are two workarounds that I can think of: do not use the second argument of the set_error_handler method or suppress the errors reported by the fopen method and check instead if it returns false

weierophinney commented 6 years ago

Error suppression is a no-go; it's against our coding standards.

We could potentially remove the second argument to set_error_handler() and check $errno, comparing it to E_WARNING; I'd consider such a patch if submitted.

Alternately, you could create your stream early, and inject it into your error handler, ensuring the fopen() call within Stream never needs to be triggered.

ste93cry commented 6 years ago

We could potentially remove the second argument to set_error_handler() and check $errno, comparing it to E_WARNING; I'd consider such a patch if submitted.

I've opened a PR to fix the issue, the change is trivial and unit tests should work the same as before.