From 9e58a1d9822687e42d8e6f3d420979fcb5f4a28e Mon Sep 17 00:00:00 2001 From: Louis-Arnaud Date: Mon, 7 Sep 2026 15:07:47 +0200 Subject: [PATCH 1/2] ext/ftp: throw an Error when ftp_nb_fget()/ftp_nb_fput() hit a busy connection (#23540) * ext/ftp: throw an Error when the connection is already transferring ftp_nb_fget() and ftp_nb_fput() answered an already busy connection with a warning and false, against a declared int return type. Reaching that guard means a transfer was started from inside another transfer, which is a programming mistake, so throw an Error instead, as ftp_close() already does on the same in_use flag. The declarations stay int. The check stays ahead of the direction and closestream writes, so the running transfer is left untouched. The test reaches the guard through a stream wrapper that calls back into the extension mid transfer. * [skip ci] Note the ftp_nb_fget()/ftp_nb_fput() Error in UPGRADING * Address review: extend the Error to ftp_nb_get() and ftp_nb_put() The in_use guard is a programming error in all four non-blocking transfer functions, so all four now throw the same Error instead of two of them emitting a warning and returning false. ftp_nb_get() and ftp_nb_put() keep their int|false declaration, which is still returned when the local file cannot be opened. ftp_nb_put() closes the local stream before throwing, and the guards stay ahead of the direction/closestream writes, so a rejected re-entrant call leaves the running transfer untouched. ftp_nb_get_during_transfer.phpt and ftp_nb_get_during_nb_transfer.phpt asserted the old warning; they now record the Error and still assert that the outer transfer completes. * Catch Throwable in the re-entrant transfer test The test asserts the class it prints, so it must not presume Error in the catch: a change of thrown class has to fail the test rather than escape it. --- UPGRADING | 6 ++ ext/ftp/php_ftp.c | 16 ++--- .../tests/ftp_nb_get_during_nb_transfer.phpt | 11 ++- ext/ftp/tests/ftp_nb_get_during_transfer.phpt | 11 ++- .../ftp_nb_transfer_during_transfer.phpt | 67 +++++++++++++++++++ 5 files changed, 101 insertions(+), 10 deletions(-) create mode 100644 ext/ftp/tests/ftp_nb_transfer_during_transfer.phpt diff --git a/UPGRADING b/UPGRADING index c795bc347dfd..14dbf5a96f02 100644 --- a/UPGRADING +++ b/UPGRADING @@ -48,6 +48,12 @@ PHP 8.6 UPGRADE NOTES the integer index is greater than INT_MAX instead of overflowing to a smaller index. +- FTP: + . ftp_nb_fget(), ftp_nb_fput(), ftp_nb_get() and ftp_nb_put() now throw an + Error when the connection is already transferring, instead of emitting a + warning and returning false. ftp_close() already throws on the same + condition. + - GD: . imagesetstyle(), imagefilter() and imagecrop() filter the types / values of their array arguments and raise a TypeError / ValueError accordingly. diff --git a/ext/ftp/php_ftp.c b/ext/ftp/php_ftp.c index 312f87c2781b..31947af455cf 100644 --- a/ext/ftp/php_ftp.c +++ b/ext/ftp/php_ftp.c @@ -655,8 +655,8 @@ PHP_FUNCTION(ftp_nb_fget) /* configuration */ if (ftp->in_use) { - php_error_docref(NULL, E_WARNING, "FTP\\Connection is already in use"); - RETURN_FALSE; + zend_throw_error(NULL, "Cannot start a transfer while another transfer is in progress"); + RETURN_THROWS(); } ftp->direction = 0; /* recv */ @@ -770,8 +770,8 @@ PHP_FUNCTION(ftp_nb_get) } GET_FTPBUF(ftp, z_ftp); if (ftp->in_use) { - php_error_docref(NULL, E_WARNING, "FTP\\Connection is already in use"); - RETURN_FALSE; + zend_throw_error(NULL, "Cannot start a transfer while another transfer is in progress"); + RETURN_THROWS(); } XTYPE(xtype, mode); @@ -960,8 +960,8 @@ PHP_FUNCTION(ftp_nb_fput) /* configuration */ if (ftp->in_use) { - php_error_docref(NULL, E_WARNING, "FTP\\Connection is already in use"); - RETURN_FALSE; + zend_throw_error(NULL, "Cannot start a transfer while another transfer is in progress"); + RETURN_THROWS(); } ftp->direction = true; /* send */ @@ -1106,8 +1106,8 @@ PHP_FUNCTION(ftp_nb_put) if (ftp->in_use) { php_stream_close(instream); - php_error_docref(NULL, E_WARNING, "FTP\\Connection is already in use"); - RETURN_FALSE; + zend_throw_error(NULL, "Cannot start a transfer while another transfer is in progress"); + RETURN_THROWS(); } /* configuration */ diff --git a/ext/ftp/tests/ftp_nb_get_during_nb_transfer.phpt b/ext/ftp/tests/ftp_nb_get_during_nb_transfer.phpt index deb1698c77c5..c36f63ab0cbb 100644 --- a/ext/ftp/tests/ftp_nb_get_during_nb_transfer.phpt +++ b/ext/ftp/tests/ftp_nb_get_during_nb_transfer.phpt @@ -10,11 +10,17 @@ require 'server.inc'; class NbGetDuringNbGet { public $context; public static $ftp; + public static $error; public function stream_open($path, $mode, $options, &$opened_path) { return true; } public function stream_write($data) { - @ftp_nb_get(self::$ftp, 'php://memory', 'a story.txt', FTP_BINARY); + try { + ftp_nb_get(self::$ftp, 'php://memory', 'a story.txt', FTP_BINARY); + } catch (Throwable $e) { + /* recorded rather than echoed: stream_write() may run more than once */ + self::$error = $e::class . ': ' . $e->getMessage(); + } return strlen($data); } public function stream_close() {} @@ -35,10 +41,13 @@ while ($r == FTP_MOREDATA) { } var_dump($r === FTP_FINISHED); +var_dump(NbGetDuringNbGet::$error); + ftp_close($ftp); echo "closed\n"; ?> --EXPECT-- bool(true) bool(true) +string(68) "Error: Cannot start a transfer while another transfer is in progress" closed diff --git a/ext/ftp/tests/ftp_nb_get_during_transfer.phpt b/ext/ftp/tests/ftp_nb_get_during_transfer.phpt index c7920496feb4..e22da6071477 100644 --- a/ext/ftp/tests/ftp_nb_get_during_transfer.phpt +++ b/ext/ftp/tests/ftp_nb_get_during_transfer.phpt @@ -10,11 +10,17 @@ require 'server.inc'; class NbGetDuringGet { public $context; public static $ftp; + public static $error; public function stream_open($path, $mode, $options, &$opened_path) { return true; } public function stream_write($data) { - @ftp_nb_get(self::$ftp, 'php://memory', 'a story.txt', FTP_BINARY); + try { + ftp_nb_get(self::$ftp, 'php://memory', 'a story.txt', FTP_BINARY); + } catch (Throwable $e) { + /* recorded rather than echoed: stream_write() may run more than once */ + self::$error = $e::class . ': ' . $e->getMessage(); + } return strlen($data); } public function stream_close() {} @@ -31,10 +37,13 @@ NbGetDuringGet::$ftp = $ftp; var_dump(@ftp_get($ftp, 'reentrantget://sink', 'a story.txt', FTP_BINARY)); +var_dump(NbGetDuringGet::$error); + ftp_close($ftp); echo "closed\n"; ?> --EXPECT-- bool(true) bool(true) +string(68) "Error: Cannot start a transfer while another transfer is in progress" closed diff --git a/ext/ftp/tests/ftp_nb_transfer_during_transfer.phpt b/ext/ftp/tests/ftp_nb_transfer_during_transfer.phpt new file mode 100644 index 000000000000..db79f1ecea28 --- /dev/null +++ b/ext/ftp/tests/ftp_nb_transfer_during_transfer.phpt @@ -0,0 +1,67 @@ +--TEST-- +ftp_nb_fget(), ftp_nb_fput(), ftp_nb_get() and ftp_nb_put() throw when a transfer is already in progress +--EXTENSIONS-- +ftp +pcntl +--FILE-- +getMessage(), "\n"; + } + return strlen($data); + } + public function stream_close() {} + public function stream_eof() { + return true; + } +} + +stream_wrapper_register('reentrantnb', TransferDuringNbWrite::class); + +$ftp = ftp_connect('127.0.0.1', $port); +var_dump(ftp_login($ftp, 'user', 'pass')); +TransferDuringNbWrite::$ftp = $ftp; + +$sink = fopen('php://memory', 'w+'); +/* ftp_nb_put() opens the local file before it reaches the guard, so it has to exist. */ +$local = __DIR__ . '/ftp_nb_transfer_during_transfer.tmp'; +file_put_contents($local, 'payload'); + +$calls = [ + static fn ($ftp) => ftp_nb_fget($ftp, $sink, 'a story.txt', FTP_BINARY), + static fn ($ftp) => ftp_nb_fput($ftp, 'a story.txt', $sink, FTP_BINARY), + static fn ($ftp) => ftp_nb_get($ftp, $local, 'a story.txt', FTP_BINARY), + static fn ($ftp) => ftp_nb_put($ftp, 'a story.txt', $local, FTP_BINARY), +]; + +foreach ($calls as $call) { + TransferDuringNbWrite::$call = $call; + @ftp_nb_get($ftp, 'reentrantnb://sink', 'a story.txt', FTP_BINARY); +} + +ftp_close($ftp); +echo "closed\n"; +?> +--CLEAN-- + +--EXPECT-- +bool(true) +Error: Cannot start a transfer while another transfer is in progress +Error: Cannot start a transfer while another transfer is in progress +Error: Cannot start a transfer while another transfer is in progress +Error: Cannot start a transfer while another transfer is in progress +closed From cafbcde1620550be24034c24f50539230366bf37 Mon Sep 17 00:00:00 2001 From: Georgij Tsarin Date: Mon, 7 Sep 2026 16:13:40 +0300 Subject: [PATCH 2/2] ftp: use SSL_write_ex() in single_send() to fix signed/unsigned handling (#22967) * ftp: use SSL_write_ex() in single_send() Replace SSL_write() with SSL_write_ex() and pass its return value to SSL_get_error(). This preserves the original API contract and avoids signed/unsigned conversion issues when handling errors. Signed-off-by: Denis Sergeev * ftp: widen single_send() return type to ssize_t * ftp: initialize sent before SSL_write_ex() --------- Signed-off-by: Denis Sergeev Co-authored-by: Denis Sergeev --- ext/ftp/ftp.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/ext/ftp/ftp.c b/ext/ftp/ftp.c index 20a67950627b..eaa1768156eb 100644 --- a/ext/ftp/ftp.c +++ b/ext/ftp/ftp.c @@ -1349,13 +1349,14 @@ static ssize_t my_recv_wrapper_with_restart(php_socket_t fd, void *buf, size_t s return n; } -static int single_send(ftpbuf_t *ftp, php_socket_t s, void *buf, size_t size) { +static ssize_t single_send(ftpbuf_t *ftp, php_socket_t s, void *buf, size_t size) { #ifdef HAVE_FTP_SSL int err; bool retry = false; SSL *handle = NULL; php_socket_t fd; size_t sent; + int ret; if (ftp->use_ssl && ftp->fd == s && ftp->ssl_active) { handle = ftp->ssl_handle; @@ -1368,8 +1369,9 @@ static int single_send(ftpbuf_t *ftp, php_socket_t s, void *buf, size_t size) { } do { - sent = SSL_write(handle, buf, size); - err = SSL_get_error(handle, sent); + sent = 0; + ret = SSL_write_ex(handle, buf, size, &sent); + err = SSL_get_error(handle, ret); switch (err) { case SSL_ERROR_NONE: