Skip to content

ext/ftp: declare the false returned by ftp_nb_fget() and ftp_nb_fput() - #23540

Open
lacatoire wants to merge 1 commit into
php:masterfrom
lacatoire:fix/ftp-nb-return-types-upstream
Open

ext/ftp: declare the false returned by ftp_nb_fget() and ftp_nb_fput()#23540
lacatoire wants to merge 1 commit into
php:masterfrom
lacatoire:fix/ftp-nb-return-types-upstream

Conversation

@lacatoire

Copy link
Copy Markdown
Member

ftp_nb_fget() and ftp_nb_fput() are declared int, but both answer an already busy connection with false:

if (ftp->in_use) {
	php_error_docref(NULL, E_WARNING, "FTP\\Connection is already in use");
	RETURN_FALSE;
}

ftp_nb_get() and ftp_nb_put() carry the same guard and already declare int|false; the two stream variants were left behind. Internal functions do not coerce their return value, so a caller hitting the guard receives false against a declaration promising int, and === FTP_FAILED (integer 0) does not match it.

ftp_nb_continue() has no such guard — every exit is a RETURN_LONG — so it stays int.

Only the declarations change. in_use is set and cleared within a single engine call, so the guard is reachable through re-entrancy alone and nothing at runtime moves. The added test reads the declared types of all five ftp_nb_* functions and needs no FTP server.

Both answer an already busy connection with a warning and RETURN_FALSE,
the same guard ftp_nb_get() and ftp_nb_put() have, and those two already
declare int|false.
@ndossche

ndossche commented Sep 2, 2026

Copy link
Copy Markdown
Member

Perhaps the code should be revised to throw an Error rather than returning false. Generally, Error is used to indicate a programming mistake, and this is the case here. I believe that's more appropriate than widening the return type

@ndossche

ndossche commented Sep 2, 2026

Copy link
Copy Markdown
Member

cc @iliaal

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants