Skip to content

Commit 42c72ef

Browse files
cmb69cmb69
committed
Fix #79100: Wrong FTP error messages
First we need to properly clear the `inbuf`, what is an amendment to commit d2881ad[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] <http://git.php.net/?p=php-src.git;a=commit;h=d2881adcbc9be60de7e7d45a3316b0e11b7eb1e8>. Closes GH-6718.
1 parent 272df44 commit 42c72ef

File tree

5 files changed

+112
-42
lines changed

5 files changed

+112
-42
lines changed

‎NEWS

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ PHP NEWS
1313

1414
- FTP:
1515
. Fixed bug #80901 (Info leak in ftp extension). (cmb)
16+
. Fixed bug #79100 (Wrong FTP error messages). (cmb)
1617

1718
- ODBC:
1819
. Fixed bug #80460 (ODBC doesn't account for SQL_NO_TOTAL indicator). (cmb)

‎ext/ftp/ftp.c

Lines changed: 27 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,11 @@
6363
#include "ftp.h"
6464
#include "ext/standard/fsock.h"
6565

66+
#ifdef PHP_WIN32
67+
# undef ETIMEDOUT
68+
# define ETIMEDOUT WSAETIMEDOUT
69+
#endif
70+
6671
/* sends an ftp command, returns true on success, false on error.
6772
* it sends the string "cmd args\r\n" if args is non-null, or
6873
* "cmd\r\n" if args is null
@@ -1295,7 +1300,8 @@ ftp_putcmd(ftpbuf_t *ftp, const char *cmd, const size_t cmd_len, const char *arg
12951300

12961301
data = ftp->outbuf;
12971302

1298-
/* Clear the extra-lines buffer */
1303+
/* Clear the inbuf and extra-lines buffer */
1304+
ftp->inbuf[0] = '\0';
12991305
ftp->extra = NULL;
13001306

13011307
if (my_send(ftp, ftp->fd, data, size) != size) {
@@ -1468,15 +1474,15 @@ my_send(ftpbuf_t *ftp, php_socket_t s, void *buf, size_t len)
14681474
n = php_pollfd_for_ms(s, POLLOUT, ftp->timeout_sec * 1000);
14691475

14701476
if (n < 1) {
1471-
#ifdef PHP_WIN32
1477+
char buf[256];
14721478
if (n == 0) {
1479+
#ifdef PHP_WIN32
14731480
_set_errno(ETIMEDOUT);
1474-
}
14751481
#else
1476-
if (n == 0) {
14771482
errno = ETIMEDOUT;
1478-
}
14791483
#endif
1484+
}
1485+
php_error_docref(NULL, E_WARNING, "%s", php_socket_strerror(errno, buf, sizeof buf));
14801486
return -1;
14811487
}
14821488

@@ -1505,18 +1511,17 @@ my_recv(ftpbuf_t *ftp, php_socket_t s, void *buf, size_t len)
15051511
SSL *handle = NULL;
15061512
php_socket_t fd;
15071513
#endif
1508-
15091514
n = php_pollfd_for_ms(s, PHP_POLLREADABLE, ftp->timeout_sec * 1000);
15101515
if (n < 1) {
1511-
#ifdef PHP_WIN32
1516+
char buf[256];
15121517
if (n == 0) {
1518+
#ifdef PHP_WIN32
15131519
_set_errno(ETIMEDOUT);
1514-
}
15151520
#else
1516-
if (n == 0) {
15171521
errno = ETIMEDOUT;
1518-
}
15191522
#endif
1523+
}
1524+
php_error_docref(NULL, E_WARNING, "%s", php_socket_strerror(errno, buf, sizeof buf));
15201525
return -1;
15211526
}
15221527

@@ -1583,15 +1588,15 @@ data_available(ftpbuf_t *ftp, php_socket_t s)
15831588

15841589
n = php_pollfd_for_ms(s, PHP_POLLREADABLE, 1000);
15851590
if (n < 1) {
1586-
#ifdef PHP_WIN32
1591+
char buf[256];
15871592
if (n == 0) {
1593+
#ifdef PHP_WIN32
15881594
_set_errno(ETIMEDOUT);
1589-
}
15901595
#else
1591-
if (n == 0) {
15921596
errno = ETIMEDOUT;
1593-
}
15941597
#endif
1598+
}
1599+
php_error_docref(NULL, E_WARNING, "%s", php_socket_strerror(errno, buf, sizeof buf));
15951600
return 0;
15961601
}
15971602

@@ -1607,15 +1612,15 @@ data_writeable(ftpbuf_t *ftp, php_socket_t s)
16071612

16081613
n = php_pollfd_for_ms(s, POLLOUT, 1000);
16091614
if (n < 1) {
1610-
#ifdef PHP_WIN32
1615+
char buf[256];
16111616
if (n == 0) {
1617+
#ifdef PHP_WIN32
16121618
_set_errno(ETIMEDOUT);
1613-
}
16141619
#else
1615-
if (n == 0) {
16161620
errno = ETIMEDOUT;
1617-
}
16181621
#endif
1622+
}
1623+
php_error_docref(NULL, E_WARNING, "%s", php_socket_strerror(errno, buf, sizeof buf));
16191624
return 0;
16201625
}
16211626

@@ -1632,15 +1637,15 @@ my_accept(ftpbuf_t *ftp, php_socket_t s, struct sockaddr *addr, socklen_t *addrl
16321637

16331638
n = php_pollfd_for_ms(s, PHP_POLLREADABLE, ftp->timeout_sec * 1000);
16341639
if (n < 1) {
1635-
#ifdef PHP_WIN32
1640+
char buf[256];
16361641
if (n == 0) {
1642+
#ifdef PHP_WIN32
16371643
_set_errno(ETIMEDOUT);
1638-
}
16391644
#else
1640-
if (n == 0) {
16411645
errno = ETIMEDOUT;
1642-
}
16431646
#endif
1647+
}
1648+
php_error_docref(NULL, E_WARNING, "%s", php_socket_strerror(errno, buf, sizeof buf));
16441649
return -1;
16451650
}
16461651

‎ext/ftp/php_ftp.c

Lines changed: 60 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -455,7 +455,9 @@ PHP_FUNCTION(ftp_login)
455455

456456
/* log in */
457457
if (!ftp_login(ftp, user, user_len, pass, pass_len)) {
458-
php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
458+
if (*ftp->inbuf) {
459+
php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
460+
}
459461
RETURN_FALSE;
460462
}
461463

@@ -480,7 +482,9 @@ PHP_FUNCTION(ftp_pwd)
480482
}
481483

482484
if (!(pwd = ftp_pwd(ftp))) {
483-
php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
485+
if (*ftp->inbuf) {
486+
php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
487+
}
484488
RETURN_FALSE;
485489
}
486490

@@ -504,7 +508,9 @@ PHP_FUNCTION(ftp_cdup)
504508
}
505509

506510
if (!ftp_cdup(ftp)) {
507-
php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
511+
if (*ftp->inbuf) {
512+
php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
513+
}
508514
RETURN_FALSE;
509515
}
510516

@@ -531,7 +537,9 @@ PHP_FUNCTION(ftp_chdir)
531537

532538
/* change directories */
533539
if (!ftp_chdir(ftp, dir, dir_len)) {
534-
php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
540+
if (*ftp->inbuf) {
541+
php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
542+
}
535543
RETURN_FALSE;
536544
}
537545

@@ -558,7 +566,9 @@ PHP_FUNCTION(ftp_exec)
558566

559567
/* execute serverside command */
560568
if (!ftp_exec(ftp, cmd, cmd_len)) {
561-
php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
569+
if (*ftp->inbuf) {
570+
php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
571+
}
562572
RETURN_FALSE;
563573
}
564574

@@ -608,7 +618,9 @@ PHP_FUNCTION(ftp_mkdir)
608618

609619
/* create directory */
610620
if (NULL == (tmp = ftp_mkdir(ftp, dir, dir_len))) {
611-
php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
621+
if (*ftp->inbuf) {
622+
php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
623+
}
612624
RETURN_FALSE;
613625
}
614626

@@ -635,7 +647,9 @@ PHP_FUNCTION(ftp_rmdir)
635647

636648
/* remove directorie */
637649
if (!ftp_rmdir(ftp, dir, dir_len)) {
638-
php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
650+
if (*ftp->inbuf) {
651+
php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
652+
}
639653
RETURN_FALSE;
640654
}
641655

@@ -662,7 +676,9 @@ PHP_FUNCTION(ftp_chmod)
662676
}
663677

664678
if (!ftp_chmod(ftp, mode, filename, filename_len)) {
665-
php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
679+
if (*ftp->inbuf) {
680+
php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
681+
}
666682
RETURN_FALSE;
667683
}
668684

@@ -816,7 +832,9 @@ PHP_FUNCTION(ftp_systype)
816832
}
817833

818834
if (NULL == (syst = ftp_syst(ftp))) {
819-
php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
835+
if (*ftp->inbuf) {
836+
php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
837+
}
820838
RETURN_FALSE;
821839
}
822840

@@ -862,7 +880,9 @@ PHP_FUNCTION(ftp_fget)
862880
}
863881

864882
if (!ftp_get(ftp, stream, file, file_len, xtype, resumepos)) {
865-
php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
883+
if (*ftp->inbuf) {
884+
php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
885+
}
866886
RETURN_FALSE;
867887
}
868888

@@ -912,7 +932,9 @@ PHP_FUNCTION(ftp_nb_fget)
912932
ftp->closestream = 0; /* do not close */
913933

914934
if ((ret = ftp_nb_get(ftp, stream, file, file_len, xtype, resumepos)) == PHP_FTP_FAILED) {
915-
php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
935+
if (*ftp->inbuf) {
936+
php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
937+
}
916938
RETURN_LONG(ret);
917939
}
918940

@@ -1000,7 +1022,9 @@ PHP_FUNCTION(ftp_get)
10001022
if (!ftp_get(ftp, outstream, remote, remote_len, xtype, resumepos)) {
10011023
php_stream_close(outstream);
10021024
VCWD_UNLINK(local);
1003-
php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
1025+
if (*ftp->inbuf) {
1026+
php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
1027+
}
10041028
RETURN_FALSE;
10051029
}
10061030

@@ -1069,7 +1093,9 @@ PHP_FUNCTION(ftp_nb_get)
10691093
php_stream_close(outstream);
10701094
ftp->stream = NULL;
10711095
VCWD_UNLINK(local);
1072-
php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
1096+
if (*ftp->inbuf) {
1097+
php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
1098+
}
10731099
RETURN_LONG(PHP_FTP_FAILED);
10741100
}
10751101

@@ -1163,7 +1189,9 @@ PHP_FUNCTION(ftp_fput)
11631189
}
11641190

11651191
if (!ftp_put(ftp, remote, remote_len, stream, xtype, startpos)) {
1166-
php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
1192+
if (*ftp->inbuf) {
1193+
php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
1194+
}
11671195
RETURN_FALSE;
11681196
}
11691197

@@ -1217,7 +1245,9 @@ PHP_FUNCTION(ftp_nb_fput)
12171245
ftp->closestream = 0; /* do not close */
12181246

12191247
if (((ret = ftp_nb_put(ftp, remote, remote_len, stream, xtype, startpos)) == PHP_FTP_FAILED)) {
1220-
php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
1248+
if (*ftp->inbuf) {
1249+
php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
1250+
}
12211251
RETURN_LONG(ret);
12221252
}
12231253

@@ -1271,7 +1301,9 @@ PHP_FUNCTION(ftp_put)
12711301

12721302
if (!ftp_put(ftp, remote, remote_len, instream, xtype, startpos)) {
12731303
php_stream_close(instream);
1274-
php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
1304+
if (*ftp->inbuf) {
1305+
php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
1306+
}
12751307
RETURN_FALSE;
12761308
}
12771309
php_stream_close(instream);
@@ -1307,7 +1339,9 @@ PHP_FUNCTION(ftp_append)
13071339

13081340
if (!ftp_append(ftp, remote, remote_len, instream, xtype)) {
13091341
php_stream_close(instream);
1310-
php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
1342+
if (*ftp->inbuf) {
1343+
php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
1344+
}
13111345
RETURN_FALSE;
13121346
}
13131347
php_stream_close(instream);
@@ -1441,7 +1475,9 @@ PHP_FUNCTION(ftp_rename)
14411475

14421476
/* rename the file */
14431477
if (!ftp_rename(ftp, src, src_len, dest, dest_len)) {
1444-
php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
1478+
if (*ftp->inbuf) {
1479+
php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
1480+
}
14451481
RETURN_FALSE;
14461482
}
14471483

@@ -1468,7 +1504,9 @@ PHP_FUNCTION(ftp_delete)
14681504

14691505
/* delete the file */
14701506
if (!ftp_delete(ftp, file, file_len)) {
1471-
php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
1507+
if (*ftp->inbuf) {
1508+
php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
1509+
}
14721510
RETURN_FALSE;
14731511
}
14741512

@@ -1495,7 +1533,9 @@ PHP_FUNCTION(ftp_site)
14951533

14961534
/* send the site command */
14971535
if (!ftp_site(ftp, cmd, cmd_len)) {
1498-
php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
1536+
if (*ftp->inbuf) {
1537+
php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf);
1538+
}
14991539
RETURN_FALSE;
15001540
}
15011541

‎ext/ftp/tests/bug79100.phpt

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
--TEST--
2+
Bug #79100 (Wrong FTP error messages)
3+
--SKIPIF--
4+
<?php
5+
require 'skipif.inc';
6+
?>
7+
--FILE--
8+
<?php
9+
$bug79100 = true;
10+
require 'server.inc';
11+
12+
$ftp = ftp_connect("127.0.0.1", $port);
13+
if (!$ftp) die("Couldn't connect to the server");
14+
var_dump(ftp_login($ftp, 'user', 'pass'));
15+
var_dump(ftp_set_option($ftp, FTP_TIMEOUT_SEC, 1));
16+
ftp_systype($ftp);
17+
?>
18+
--EXPECTF--
19+
bool(true)
20+
bool(true)
21+
22+
Warning: ftp_systype(): %rConnection|Operation%r timed out in %s on line %d

‎ext/ftp/tests/server.inc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,8 @@ if ($pid) {
198198
} elseif ($buf === "SYST\r\n") {
199199
if (isset($bug27809)) {
200200
fputs($s, "215 OS/400 is the remote operating system. The TCP/IP version is \"V5R2M0\"\r\n");
201+
} elseif (isset($bug79100)) {
202+
// do nothing so test hits timeout
201203
} elseif (isset($bug80901)) {
202204
fputs($s, "\r\n" . str_repeat("*", 4096) . "\r\n");
203205
} else {

0 commit comments

Comments
 (0)