Skip to content

Fix GH-16800: ftp functions can abort with EINTR #17327

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

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 49 additions & 8 deletions ext/ftp/ftp.c
Original file line number Diff line number Diff line change
Expand Up @@ -1387,6 +1387,22 @@ ftp_getresp(ftpbuf_t *ftp)
}
/* }}} */

static ssize_t my_send_wrapper_with_restart(php_socket_t fd, const void *buf, size_t size, int flags) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe?

Suggested change
static ssize_t my_send_wrapper_with_restart(php_socket_t fd, const void *buf, size_t size, int flags) {
static ssize_t php_ftp_send_wrapper_with_restart(php_socket_t fd, const void *buf, size_t size, int flags) {

And similar for the other ones

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that these are static functions, I don't think a php_ftp_ prefix is necessary.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed what cmb says, and the fact that the other static functions don't have an php_ftp prefix either

ssize_t n;
do {
n = send(fd, buf, size, flags);
} while (n == -1 && php_socket_errno() == EINTR);
return n;
}

static ssize_t my_recv_wrapper_with_restart(php_socket_t fd, void *buf, size_t size, int flags) {
ssize_t n;
do {
n = recv(fd, buf, size, flags);
} while (n == -1 && php_socket_errno() == EINTR);
return n;
}

int single_send(ftpbuf_t *ftp, php_socket_t s, void *buf, size_t size) {
#ifdef HAVE_FTP_SSL
int err;
Expand All @@ -1402,7 +1418,7 @@ int single_send(ftpbuf_t *ftp, php_socket_t s, void *buf, size_t size) {
handle = ftp->data->ssl_handle;
fd = ftp->data->fd;
} else {
return send(s, buf, size, 0);
return my_send_wrapper_with_restart(s, buf, size, 0);
}

do {
Expand Down Expand Up @@ -1441,8 +1457,33 @@ int single_send(ftpbuf_t *ftp, php_socket_t s, void *buf, size_t size) {
} while (retry);
return sent;
#else
return send(s, buf, size, 0);
return my_send_wrapper_with_restart(s, buf, size, 0);
#endif
}

static int my_poll(php_socket_t fd, int events, int timeout) {
int n;
zend_hrtime_t timeout_hr = (zend_hrtime_t) timeout * 1000000;

while (true) {
zend_hrtime_t start_ns = zend_hrtime();
n = php_pollfd_for_ms(fd, events, (int) (timeout_hr / 1000000));

if (n == -1 && php_socket_errno() == EINTR) {
zend_hrtime_t delta_ns = zend_hrtime() - start_ns;
if (delta_ns > timeout_hr) {
#ifndef PHP_WIN32
errno = ETIMEDOUT;
#endif
break;
}
timeout_hr -= delta_ns;
} else {
break;
}
}

return n;
}

/* {{{ my_send */
Expand All @@ -1454,7 +1495,7 @@ my_send(ftpbuf_t *ftp, php_socket_t s, void *buf, size_t len)

size = len;
while (size) {
n = php_pollfd_for_ms(s, POLLOUT, ftp->timeout_sec * 1000);
n = my_poll(s, POLLOUT, ftp->timeout_sec * 1000);

if (n < 1) {
char buf[256];
Expand Down Expand Up @@ -1493,7 +1534,7 @@ my_recv(ftpbuf_t *ftp, php_socket_t s, void *buf, size_t len)
SSL *handle = NULL;
php_socket_t fd;
#endif
n = php_pollfd_for_ms(s, PHP_POLLREADABLE, ftp->timeout_sec * 1000);
n = my_poll(s, PHP_POLLREADABLE, ftp->timeout_sec * 1000);
if (n < 1) {
char buf[256];
if (n == 0) {
Expand Down Expand Up @@ -1553,7 +1594,7 @@ my_recv(ftpbuf_t *ftp, php_socket_t s, void *buf, size_t len)
} while (retry);
} else {
#endif
nr_bytes = recv(s, buf, len, 0);
nr_bytes = my_recv_wrapper_with_restart(s, buf, len, 0);
#ifdef HAVE_FTP_SSL
}
#endif
Expand All @@ -1567,7 +1608,7 @@ data_available(ftpbuf_t *ftp, php_socket_t s)
{
int n;

n = php_pollfd_for_ms(s, PHP_POLLREADABLE, 1000);
n = my_poll(s, PHP_POLLREADABLE, 1000);
if (n < 1) {
char buf[256];
if (n == 0) {
Expand All @@ -1590,7 +1631,7 @@ data_writeable(ftpbuf_t *ftp, php_socket_t s)
{
int n;

n = php_pollfd_for_ms(s, POLLOUT, 1000);
n = my_poll(s, POLLOUT, 1000);
if (n < 1) {
char buf[256];
if (n == 0) {
Expand All @@ -1614,7 +1655,7 @@ my_accept(ftpbuf_t *ftp, php_socket_t s, struct sockaddr *addr, socklen_t *addrl
{
int n;

n = php_pollfd_for_ms(s, PHP_POLLREADABLE, ftp->timeout_sec * 1000);
n = my_poll(s, PHP_POLLREADABLE, ftp->timeout_sec * 1000);
if (n < 1) {
char buf[256];
if (n == 0) {
Expand Down
Loading