Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

https streams on Ubuntu 22.04 might emit PHP warnings during fread() #504

Closed
TimWolla opened this issue May 2, 2022 · 5 comments
Closed

Comments

@TimWolla
Copy link

TimWolla commented May 2, 2022

PHP version: 8.1.2

Description

Because of php/php-src#8369 a https stream might emit a Warning when calling fread() on the underlying stream. If a user of PSR-7 uses an error handler that converts warnings into Exceptions then the call to ->read() might violate read()'s contract to throw a RuntimeException is cases of failure.

How to reproduce

<?php

use GuzzleHttp\Client;
use GuzzleHttp\Handler\CurlMultiHandler;
use GuzzleHttp\HandlerStack;
use GuzzleHttp\Psr7\Request;

error_reporting(E_ALL);
function exception_error_handler($severity, $message, $file, $line) {
    if (!(error_reporting() & $severity)) {
        return;
    }
    throw new ErrorException($message, 0, $severity, $file, $line);
}
set_error_handler("exception_error_handler");

require('vendor/autoload.php');

$client = new Client([
	'stream' => true,
]);

$request = new Request(
	'GET',
	'https://www.umwelt.niedersachsen.de/assets/image/1200/216904',
);

$response = $client->send($request, [
	'timeout' => 1,
]);

$body = $response->getBody();

while (!$body->eof()) {
	$v = $body->read(2048);
	var_dump(gettype($v));
}

# var_dump($response);

Possible Solution

The call to fread():

$string = fread($this->stream, $length);

can be prefixed with @ to fix this issue. Note that fread() won't return false for this warning.. Alternatively a custom error handler might be attached before the call and detached afterwards.

Additional context

root@ubuntu-22:~/guzzle# php test.php 
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
PHP Fatal error:  Uncaught ErrorException: fread(): SSL operation failed with code 1. OpenSSL Error messages:
error:0A000126:SSL routines::unexpected eof while reading in /root/guzzle/vendor/guzzlehttp/psr7/src/Stream.php:232
Stack trace:
#0 [internal function]: exception_error_handler()
#1 /root/guzzle/vendor/guzzlehttp/psr7/src/Stream.php(232): fread()
#2 /root/guzzle/test.php(35): GuzzleHttp\Psr7\Stream->read()
#3 {main}
  thrown in /root/guzzle/vendor/guzzlehttp/psr7/src/Stream.php on line 232
root@ubuntu-22:~/guzzle# php test.php 
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
string(6) "string"
PHP Warning:  fread(): SSL operation failed with code 1. OpenSSL Error messages:
error:0A000126:SSL routines::unexpected eof while reading in /root/guzzle/vendor/guzzlehttp/psr7/src/Stream.php on line 232
string(6) "string"
@GrahamCampbell
Copy link
Member

Hmm, that's unfortunate.

might violate read()'s contract to throw a RuntimeException is cases of failure.

Throwing something that extends Error or RuntimeException is not a violation of a contact. I always take the view those those type of throwables are always fair game to be thrown at any time, just like with Java.

@TimWolla
Copy link
Author

TimWolla commented May 2, 2022

Throwing something that extends Error or RuntimeException is not a violation of a contact. I always take the view those those type of throwables are always fair game to be thrown at any time, just like with Java.

Don't want to nitpick here more than necessary, but ErrorException is just extends Exception and thus neither RuntimeException, nor Error, probably for historical reasons.

For RuntimeException I generally agree, but the cause of the warning in this case matches Java's IOException / EOFException more closely and that one is a checked exception in Java I believe.

Generally speaking the PHP Warning can be triggered by the remote server at will and thus it is unfixable by the user of the psr7 implementation, other than by catching \Throwable (which generally should be avoided, as this might mask other errors). Thus I believe it is up to guzzle/psr7 to ensure that the warning does not leave the read() method (either by @ or by a custom error handler).

@TimWolla
Copy link
Author

TimWolla commented May 2, 2022

diff --git i/src/Stream.php w/src/Stream.php
index d389427..5baecfa 100644
--- i/src/Stream.php
+++ w/src/Stream.php
@@ -229,7 +229,12 @@ class Stream implements StreamInterface
             return '';
         }
 
-        $string = fread($this->stream, $length);
+        try {
+            $string = fread($this->stream, $length);
+        } catch (\Exception $e) {
+            throw new \RuntimeException('Unable to read from stream', 0, $e);
+        }
+
         if (false === $string) {
             throw new \RuntimeException('Unable to read from stream');
         }

might be an acceptable fix as well. This ensures that any exception is then rewrapped as a RuntimeException, which is required by the PSR-7 standard:

     * @throws \RuntimeException if an error occurs.

@GrahamCampbell
Copy link
Member

Yeh, that would be a good solution.

@TimWolla
Copy link
Author

TimWolla commented May 2, 2022

Resolved with #505

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants