WIP: Fix fwrite() on non-blocking SSL sockets

NEWS

@@ -16,8 +16,8 @@ PHP                                                                        NEWS
     (somedaysummer)
 
 - OpenSSL:
-  . Fixed bug #74159 (Writing a large buffer to a non-blocking encrypted stream
-    fails with "bad write retry"). (trowski)
+  . Fixed bug #72333 (fwrite() on non-blocking SSL sockets doesn't work).
+    (Jakub Zelenka)
 
 - PDO MySQL:
   . Fixed bug #71003 (Expose MYSQLI_CLIENT_SSL_DONT_VERIFY_SERVER_CERT to PDO

ext/openssl/tests/bug72333.phpt

@@ -0,0 +1,54 @@
+--TEST--
+Bug #72333: fwrite() on non-blocking SSL sockets doesn't work
+--SKIPIF--
+<?php
+if (!extension_loaded("openssl")) die("skip openssl not loaded");
+if (!function_exists("proc_open")) die("skip no proc_open");
+?>
+--FILE--
+<?php
+$serverCode = <<<'CODE'
+	$context = stream_context_create(['ssl' => ['local_cert' => __DIR__ . '/bug54992.pem']]);
+
+	$flags = STREAM_SERVER_BIND|STREAM_SERVER_LISTEN;
+	$fp = stream_socket_server("ssl://127.0.0.1:10011", $errornum, $errorstr, $flags, $context);
+	phpt_notify();
+	$conn = stream_socket_accept($fp);
+
+	for ($i = 0; $i < 5; $i++) {
+		fread($conn, 100000);
+		usleep(20000);
+	}
+CODE;
+
+$clientCode = <<<'CODE'
+	$context = stream_context_create(['ssl' => ['verify_peer' => false, 'peer_name' => 'bug54992.local']]);
+
+	phpt_wait();
+	$fp = stream_socket_client("ssl://127.0.0.1:10011", $errornum, $errorstr, 3000, STREAM_CLIENT_CONNECT, $context);
+	stream_set_blocking($fp, 0);
+
+	function blocking_fwrite($fp, $buf) {
+		$write = [$fp];
+		$total = 0;
+		while (stream_select($read, $write, $except, 180)) {
+			$result = fwrite($fp, $buf);
+			$total += $result;
+			if ($total >= strlen($buf)) {
+				return $total;
+			}
+			$buf = substr($buf, $total);
+		}
+	}
+
+	$str1 = str_repeat("a", 5000000);
+	blocking_fwrite($fp, $str1);
+	echo "done";
+CODE;
+
+include 'ServerClientTestCase.inc';
+ServerClientTestCase::getInstance()->run($clientCode, $serverCode);
+?>
+--EXPECT--
+done
+

ext/openssl/tests/bug74159.phpt

@@ -4,103 +4,109 @@ Bug #74159: Writing a large buffer to non-blocking encrypted streams fails
 <?php
 if (!extension_loaded("openssl")) die("skip openssl not loaded");
 if (!function_exists("proc_open")) die("skip no proc_open");
+if (OPENSSL_VERSION_NUMBER < 0x10001001) die("skip OpenSSLv1.0.1 required");
+?>
 --FILE--
 <?php
+// the server code is doing many readings in a short interval which is
+// not really reliable on more powerful machine but cover different
+// scenarios which might be useful. More reliable test is bug72333.phpt
 $serverCode = <<<'CODE'
-    $serverUri = "ssl://127.0.0.1:64321";
-    $serverFlags = STREAM_SERVER_BIND | STREAM_SERVER_LISTEN;
-    $serverCtx = stream_context_create(['ssl' => [
-        'local_cert' => __DIR__ . '/bug54992.pem',
-        'crypto_method' => STREAM_CRYPTO_METHOD_TLSv1_2_SERVER,
-    ]]);
+    $serverUri = "ssl://127.0.0.1:10012";
+	$serverFlags = STREAM_SERVER_BIND | STREAM_SERVER_LISTEN;
+	$serverCtx = stream_context_create(['ssl' => [
+	    'local_cert' => __DIR__ . '/bug54992.pem',
+		'crypto_method' => STREAM_CRYPTO_METHOD_TLSv1_2_SERVER,
+	]]);
 
     $server = stream_socket_server($serverUri, $errno, $errstr, $serverFlags, $serverCtx);
-    phpt_notify();
+	phpt_notify();
 
     $client = stream_socket_accept($server, 1);
 
     if (!$client) {
-        exit();
-    }
+	    exit();
+	}
 
     $data = '';
-    while (strlen($data) < 0xfffff) {
-        $buffer = fread($client, 8192);
-        if (empty($buffer)) {
-            exit();
-        }
-        $data .= $buffer;
-        usleep(100);
-    }
-    
+	while (strlen($data) < 0xfffff) {
+	    $buffer = fread($client, 8192);
+		if (empty($buffer)) {
+		    exit();
+		}
+		$data .= $buffer;
+		usleep(100);
+	}
+
     fclose($client);
 CODE;
 
 $clientCode = <<<'CODE'
     function streamRead($stream) : int {
-        return strlen(fread($stream, 8192));
-    }
+	    return strlen(fread($stream, 8192));
+	}
 
     function streamWrite($stream, $data) : int {
-        return fwrite($stream, $data);
-    }
+	    return fwrite($stream, $data);
+	}
 
     function waitForWrite(...$streams) : bool {
-        $read = null;
-        $except = null;
-        while($streams && !($n = stream_select($read, $streams, $except, 1)));
-        return $n > 0;
-    }
+	    $read = null;
+		$except = null;
+		while($streams && !($n = stream_select($read, $streams, $except, 1)));
+		return $n > 0;
+	}
 
     function waitForRead(...$streams) : bool {
-        $write = null;
-        $except = null;
-        while ($streams && !($n = stream_select($streams, $write, $except, 1)));
-        return $n > 0;
-    }
+	    $write = null;
+		$except = null;
+		while ($streams && !($n = stream_select($streams, $write, $except, 1)));
+		return $n > 0;
+	}
 
     set_error_handler(function ($errno, $errstr) {
-        exit("$errstr\n");
-    });
+	    exit("$errstr\n");
+	});
 
-    $serverUri = "tcp://127.0.0.1:64321";
-    $clientFlags = STREAM_CLIENT_CONNECT;
-    $clientCtx = stream_context_create(['ssl' => [
-        'verify_peer' => true,
-        'cafile' => __DIR__ . '/bug54992-ca.pem',
-        'peer_name' => 'bug54992.local',
-    ]]);
+    $serverUri = "tcp://127.0.0.1:10012";
+	$clientFlags = STREAM_CLIENT_CONNECT;
+	$clientCtx = stream_context_create(['ssl' => [
+	    'verify_peer' => true,
+		'cafile' => __DIR__ . '/bug54992-ca.pem',
+		'peer_name' => 'bug54992.local',
+	]]);
 
     phpt_wait();
 
     $fp = stream_socket_client($serverUri, $errno, $errstr, 1, $clientFlags, $clientCtx);
 
     stream_set_blocking($fp, false);
-    while (0 === ($n = stream_socket_enable_crypto($fp, true, STREAM_CRYPTO_METHOD_TLSv1_2_CLIENT)));
+	while (0 === ($n = stream_socket_enable_crypto($fp, true, STREAM_CRYPTO_METHOD_TLSv1_2_CLIENT)));
 
     $data = str_repeat("a", 0xfffff);
-    $written = 0;
-    $total = $written;
-    while(!empty($data)) {
-        $written = streamWrite($fp, $data);
-        $total += $written;
-        $data = substr($data, $written);
-        waitForWrite($fp);
-    }
-    printf("Written %d bytes\n", $total);
+	$written = 0;
+	$total = $written;
+	while(!empty($data)) {
+	    $written = streamWrite($fp, $data);
+		$total += $written;
+		$data = substr($data, $written);
+		waitForWrite($fp);
+	}
+	printf("Written %d bytes\n", $total);
 
     while(waitForRead($fp)) {
-        streamRead($fp);
-        if (feof($fp)) {
-            break;
-        }
-    }
+	    streamRead($fp);
+		if (feof($fp)) {
+		    break;
+		}
+	}
 
     exit("DONE\n");
 CODE;
 
 include 'ServerClientTestCase.inc';
 ServerClientTestCase::getInstance()->run($clientCode, $serverCode);
+?>
 --EXPECTF--
 Written 1048575 bytes
 DONE

ext/openssl/xp_ssl.c

@@ -1661,21 +1661,8 @@ int php_openssl_setup_crypto(php_stream *stream,
 	}
 
 #ifdef SSL_MODE_RELEASE_BUFFERS
-	do {
-		long mode = SSL_get_mode(sslsock->ssl_handle);
-		SSL_set_mode(sslsock->ssl_handle, mode | SSL_MODE_RELEASE_BUFFERS);
-	} while (0);
-#endif
-
-	do {
-		long mode = SSL_get_mode(sslsock->ssl_handle);
-		SSL_set_mode(sslsock->ssl_handle, mode | SSL_MODE_ENABLE_PARTIAL_WRITE);
-	} while (0);
-
-	do {
-		long mode = SSL_get_mode(sslsock->ssl_handle);
-		SSL_set_mode(sslsock->ssl_handle, mode | SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER);
-	} while (0);
+	SSL_set_mode(sslsock->ssl_handle, SSL_get_mode(sslsock->ssl_handle) | SSL_MODE_RELEASE_BUFFERS);
+#endif
 
 	if (cparam->inputs.session) {
 		if (cparam->inputs.session->ops != &php_openssl_socket_ops) {
@@ -1814,6 +1801,16 @@ static int php_openssl_enable_crypto(php_stream *stream,
 
 		if (SUCCESS == php_set_sock_blocking(sslsock->s.socket, 0)) {
 			sslsock->s.is_blocked = 0;
+			/* The following mode are added only if we are able to change socket
+			 * to non blocking mode which is also used for read and write */
+			SSL_set_mode(
+				sslsock->ssl_handle,
+				(
+					SSL_get_mode(sslsock->ssl_handle) |
+					SSL_MODE_ENABLE_PARTIAL_WRITE |
+					SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER
+				)
+			);
 		}
 
 		timeout = sslsock->is_client ? &sslsock->connect_timeout : &sslsock->s.timeout;