From 823d463a1397bbd55d35d362d3aecdfa388d5f9d Mon Sep 17 00:00:00 2001
From: "Christoph M. Becker"
Date: Mon, 22 Feb 2021 10:44:04 +0100
Subject: [PATCH 1/5] Fix #79100: Wrong FTP error messages
First we need to properly clear the `inbuf`, what is an amendment to
commit d2881adcbc[1].
Then we need to report `php_pollfd_for_ms()` failures right away; just
setting `errno` does not really help, since at least in some cases it
would have been overwritten before we actually could check it. We use
`php_socket_strerror()` to get a proper error message, and define
`ETIMEDOUT` to the proper value on Windows; otherwise we catch the
definition in errno.h, which is not compatible with WinSock. The
proper solution for this issue would likely be to include something
like ext/sockets/windows_common.h.
Finally, we ensure that we only report warnings using `inbuf`, if it is
not empty.
[1] .
---
ext/ftp/ftp.c | 44 +++++++++++++-------------
ext/ftp/ftp.h | 5 +++
ext/ftp/php_ftp.c | 80 +++++++++++++++++++++++++++++++++++------------
3 files changed, 87 insertions(+), 42 deletions(-)
diff --git a/ext/ftp/ftp.c b/ext/ftp/ftp.c
index 9b04edcd0998d..1d66618bd6aa7 100644
--- a/ext/ftp/ftp.c
+++ b/ext/ftp/ftp.c
@@ -1295,7 +1295,8 @@ ftp_putcmd(ftpbuf_t *ftp, const char *cmd, const size_t cmd_len, const char *arg
data = ftp->outbuf;
- /* Clear the extra-lines buffer */
+ /* Clear the inbuf and extra-lines buffer */
+ ftp->inbuf[0] = '\0';
ftp->extra = NULL;
if (my_send(ftp, ftp->fd, data, size) != size) {
@@ -1468,15 +1469,15 @@ my_send(ftpbuf_t *ftp, php_socket_t s, void *buf, size_t len)
n = php_pollfd_for_ms(s, POLLOUT, ftp->timeout_sec * 1000);
if (n timeout_sec * 1000);
if (n timeout_sec * 1000);
if (n inbuf);
+ if (*ftp->inbuf) {
+ php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
+ }
RETURN_FALSE;
}
@@ -480,7 +482,9 @@ PHP_FUNCTION(ftp_pwd)
}
if (!(pwd = ftp_pwd(ftp))) {
- php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
+ if (*ftp->inbuf) {
+ php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
+ }
RETURN_FALSE;
}
@@ -504,7 +508,9 @@ PHP_FUNCTION(ftp_cdup)
}
if (!ftp_cdup(ftp)) {
- php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
+ if (*ftp->inbuf) {
+ php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
+ }
RETURN_FALSE;
}
@@ -531,7 +537,9 @@ PHP_FUNCTION(ftp_chdir)
/* change directories */
if (!ftp_chdir(ftp, dir, dir_len)) {
- php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
+ if (*ftp->inbuf) {
+ php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
+ }
RETURN_FALSE;
}
@@ -558,7 +566,9 @@ PHP_FUNCTION(ftp_exec)
/* execute serverside command */
if (!ftp_exec(ftp, cmd, cmd_len)) {
- php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
+ if (*ftp->inbuf) {
+ php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
+ }
RETURN_FALSE;
}
@@ -608,7 +618,9 @@ PHP_FUNCTION(ftp_mkdir)
/* create directory */
if (NULL == (tmp = ftp_mkdir(ftp, dir, dir_len))) {
- php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
+ if (*ftp->inbuf) {
+ php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
+ }
RETURN_FALSE;
}
@@ -635,7 +647,9 @@ PHP_FUNCTION(ftp_rmdir)
/* remove directorie */
if (!ftp_rmdir(ftp, dir, dir_len)) {
- php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
+ if (*ftp->inbuf) {
+ php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
+ }
RETURN_FALSE;
}
@@ -662,7 +676,9 @@ PHP_FUNCTION(ftp_chmod)
}
if (!ftp_chmod(ftp, mode, filename, filename_len)) {
- php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
+ if (*ftp->inbuf) {
+ php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
+ }
RETURN_FALSE;
}
@@ -816,7 +832,9 @@ PHP_FUNCTION(ftp_systype)
}
if (NULL == (syst = ftp_syst(ftp))) {
- php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
+ if (*ftp->inbuf) {
+ php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
+ }
RETURN_FALSE;
}
@@ -862,7 +880,9 @@ PHP_FUNCTION(ftp_fget)
}
if (!ftp_get(ftp, stream, file, file_len, xtype, resumepos)) {
- php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
+ if (*ftp->inbuf) {
+ php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
+ }
RETURN_FALSE;
}
@@ -912,7 +932,9 @@ PHP_FUNCTION(ftp_nb_fget)
ftp->closestream = 0; /* do not close */
if ((ret = ftp_nb_get(ftp, stream, file, file_len, xtype, resumepos)) == PHP_FTP_FAILED) {
- php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
+ if (*ftp->inbuf) {
+ php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
+ }
RETURN_LONG(ret);
}
@@ -1000,7 +1022,9 @@ PHP_FUNCTION(ftp_get)
if (!ftp_get(ftp, outstream, remote, remote_len, xtype, resumepos)) {
php_stream_close(outstream);
VCWD_UNLINK(local);
- php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
+ if (*ftp->inbuf) {
+ php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
+ }
RETURN_FALSE;
}
@@ -1069,7 +1093,9 @@ PHP_FUNCTION(ftp_nb_get)
php_stream_close(outstream);
ftp->stream = NULL;
VCWD_UNLINK(local);
- php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
+ if (*ftp->inbuf) {
+ php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
+ }
RETURN_LONG(PHP_FTP_FAILED);
}
@@ -1163,7 +1189,9 @@ PHP_FUNCTION(ftp_fput)
}
if (!ftp_put(ftp, remote, remote_len, stream, xtype, startpos)) {
- php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
+ if (*ftp->inbuf) {
+ php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
+ }
RETURN_FALSE;
}
@@ -1217,7 +1245,9 @@ PHP_FUNCTION(ftp_nb_fput)
ftp->closestream = 0; /* do not close */
if (((ret = ftp_nb_put(ftp, remote, remote_len, stream, xtype, startpos)) == PHP_FTP_FAILED)) {
- php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
+ if (*ftp->inbuf) {
+ php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
+ }
RETURN_LONG(ret);
}
@@ -1271,7 +1301,9 @@ PHP_FUNCTION(ftp_put)
if (!ftp_put(ftp, remote, remote_len, instream, xtype, startpos)) {
php_stream_close(instream);
- php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
+ if (*ftp->inbuf) {
+ php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
+ }
RETURN_FALSE;
}
php_stream_close(instream);
@@ -1307,7 +1339,9 @@ PHP_FUNCTION(ftp_append)
if (!ftp_append(ftp, remote, remote_len, instream, xtype)) {
php_stream_close(instream);
- php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
+ if (*ftp->inbuf) {
+ php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
+ }
RETURN_FALSE;
}
php_stream_close(instream);
@@ -1441,7 +1475,9 @@ PHP_FUNCTION(ftp_rename)
/* rename the file */
if (!ftp_rename(ftp, src, src_len, dest, dest_len)) {
- php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
+ if (*ftp->inbuf) {
+ php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
+ }
RETURN_FALSE;
}
@@ -1468,7 +1504,9 @@ PHP_FUNCTION(ftp_delete)
/* delete the file */
if (!ftp_delete(ftp, file, file_len)) {
- php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
+ if (*ftp->inbuf) {
+ php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
+ }
RETURN_FALSE;
}
@@ -1495,7 +1533,9 @@ PHP_FUNCTION(ftp_site)
/* send the site command */
if (!ftp_site(ftp, cmd, cmd_len)) {
- php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
+ if (*ftp->inbuf) {
+ php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
+ }
RETURN_FALSE;
}
From c46c978468357088f5e34ed8eb15289430a6bd2a Mon Sep 17 00:00:00 2001
From: "Christoph M. Becker"
Date: Thu, 22 Apr 2021 17:01:20 +0200
Subject: [PATCH 2/5] Move (re)definition of ETIMEDOUT to .c file
---
ext/ftp/ftp.c | 5 +++++
ext/ftp/ftp.h | 5 -----
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/ext/ftp/ftp.c b/ext/ftp/ftp.c
index 1d66618bd6aa7..a11aa323c1c0a 100644
--- a/ext/ftp/ftp.c
+++ b/ext/ftp/ftp.c
@@ -63,6 +63,11 @@
#include "ftp.h"
#include "ext/standard/fsock.h"
+#ifdef PHP_WIN32
+# undef ETIMEDOUT
+# define ETIMEDOUT WSAETIMEDOUT
+#endif
+
/* sends an ftp command, returns true on success, false on error.
* it sends the string "cmd args\r\n" if args is non-null, or
* "cmd\r\n" if args is null
diff --git a/ext/ftp/ftp.h b/ext/ftp/ftp.h
index 75934fe094866..2b1338d81dff4 100644
--- a/ext/ftp/ftp.h
+++ b/ext/ftp/ftp.h
@@ -34,11 +34,6 @@
#define PHP_FTP_FINISHED 1
#define PHP_FTP_MOREDATA 2
-#ifdef PHP_WIN32
-# undef ETIMEDOUT
-# define ETIMEDOUT WSAETIMEDOUT
-#endif
-
/* XXX this should be configurable at runtime XXX */
#define FTP_BUFSIZE 4096
From 71352e40771693e9eea3d2fe137e64a01f7f4a77 Mon Sep 17 00:00:00 2001
From: "Christoph M. Becker"
Date: Thu, 22 Apr 2021 17:08:39 +0200
Subject: [PATCH 3/5] Add regression test
---
ext/ftp/tests/bug79100.phpt | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
create mode 100644 ext/ftp/tests/bug79100.phpt
diff --git a/ext/ftp/tests/bug79100.phpt b/ext/ftp/tests/bug79100.phpt
new file mode 100644
index 0000000000000..4ed71e325ab76
--- /dev/null
+++ b/ext/ftp/tests/bug79100.phpt
@@ -0,0 +1,22 @@
+--TEST--
+Bug #79100 (Wrong FTP error messages)
+--SKIPIF--
+
+--FILE--
+
+--EXPECTF--
+bool(true)
+bool(true)
+
+Warning: ftp_systype(): Connection timed out in %s on line %d
\ No newline at end of file
From 2950f2a9f7a35f7505c5039fce8f37bc2374806a Mon Sep 17 00:00:00 2001
From: "Christoph M. Becker"
Date: Thu, 22 Apr 2021 17:10:27 +0200
Subject: [PATCH 4/5] Update server.inc wrt. regression test
---
ext/ftp/tests/server.inc | 2 ++
1 file changed, 2 insertions(+)
diff --git a/ext/ftp/tests/server.inc b/ext/ftp/tests/server.inc
index 71f0881dea662..b77bed904f25c 100644
--- a/ext/ftp/tests/server.inc
+++ b/ext/ftp/tests/server.inc
@@ -198,6 +198,8 @@ if ($pid) {
} elseif ($buf === "SYST\r\n") {
if (isset($bug27809)) {
fputs($s, "215 OS/400 is the remote operating system. The TCP/IP version is \"V5R2M0\"\r\n");
+ } elseif (isset($bug79100)) {
+ // do nothing so test hits timeout
} elseif (isset($bug80901)) {
fputs($s, "\r\n" . str_repeat("*", 4096) . "\r\n");
} else {
From 2dfeee2163c0b1f10b2043701bda2d2c0249167e Mon Sep 17 00:00:00 2001
From: "Christoph M. Becker"
Date: Thu, 22 Apr 2021 18:31:47 +0200
Subject: [PATCH 5/5] Fix test for macOS
Apparently, macOS's error string for ETIMEDOUT is "Operation timed out".
---
ext/ftp/tests/bug79100.phpt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ext/ftp/tests/bug79100.phpt b/ext/ftp/tests/bug79100.phpt
index 4ed71e325ab76..21876bb3db852 100644
--- a/ext/ftp/tests/bug79100.phpt
+++ b/ext/ftp/tests/bug79100.phpt
@@ -19,4 +19,4 @@ ftp_systype($ftp);
bool(true)
bool(true)
-Warning: ftp_systype(): Connection timed out in %s on line %d
\ No newline at end of file
+Warning: ftp_systype(): %rConnection|Operation%r timed out in %s on line %d
\ No newline at end of file