Fixed bug #73457

Correctly report errors when opening FTP data connection.

4 files changed, 65 insertions, 18 deletions

diff --git a/NEWS b/NEWS
index 3ef07a8179..aaaf511fa5 100644
--- a/NEWS
+++ b/NEWS
@@ -3,7 +3,11 @@ PHP                                                                        NEWS
 ?? ??? 2018, PHP 7.1.23
 
 - POSIX:
-  Fixed bug #75696 (posix_getgrnam fails to print details of group). (cmb)
+  . Fixed bug #75696 (posix_getgrnam fails to print details of group). (cmb)
+
+- Standard:
+  . Fixed bug #73457 (Wrong error message when fopen FTP wrapped fails to open
+    data connection). (Ville Hukkamäki)
 
 13 Sep 2018, PHP 7.1.22
 
diff --git a/ext/ftp/tests/server.inc b/ext/ftp/tests/server.inc
index 06c0d0d6db..2d4055f0d0 100644
--- a/ext/ftp/tests/server.inc
+++ b/ext/ftp/tests/server.inc
@@ -335,7 +335,7 @@ if ($pid) {
 				fputs($s, "550 No file named \"{$matches [1]}\"\r\n");
 				break;
 			}
-		}elseif (preg_match('/^RETR ([\w\h]+)/', $buf, $matches)) {
+		}elseif (preg_match('/^RETR ([\/]*[\w\h]+)/', $buf, $matches)) {
 			if(!empty($pasv)){
 				;
 			}
@@ -391,6 +391,10 @@ if ($pid) {
 						fputs($fs, "This is line $i of the test data.\n");
 					}
 					fputs($s, "226 Closing data Connection.\r\n");
+					break;
+				case "/bug73457":
+					fputs($s, "150 File status okay; about to open data connection.\r\n");
+					break;
 
 				default:
 					fputs($s, "550 {$matches[1]}: No such file or directory \r\n");
@@ -405,29 +409,35 @@ if ($pid) {
 			$host = "127.0.0.1";
 			$i=0;
 
-			do {
-				if (!empty($ssl)) {
-					$soc = @stream_socket_server("tcp://127.0.0.1:$pasv_port", $errno, $errstr, STREAM_SERVER_BIND|STREAM_SERVER_LISTEN, $context);
-				} else {
-					$soc = @stream_socket_server("tcp://127.0.0.1:$pasv_port");
-				}
-				/* Could bind port, Try another port */
+			if (empty($bug73457)) {
+				do {
+					if (!empty($ssl)) {
+						$soc = @stream_socket_server("tcp://127.0.0.1:$pasv_port", $errno, $errstr, STREAM_SERVER_BIND|STREAM_SERVER_LISTEN, $context);
+					} else {
+						$soc = @stream_socket_server("tcp://127.0.0.1:$pasv_port");
+					}
+					/* Could bind port, Try another port */
+					if (!$soc) {
+						$pasv_port = rand(50000, 65535);
+					}
+					$i++;
+				} while ($i<10 && !$soc);
+
 				if (!$soc) {
-					$pasv_port = rand(50000, 65535);
+					echo "$errstr ($errno)\n";
+					die("could not bind passive port\n");
 				}
-				$i++;
-			} while ($i<10 && !$soc);
-
-			if (!$soc) {
-				echo "$errstr ($errno)\n";
-				die("could not bind passive port\n");
+			} else {
+				$pasv_port=1234;
 			}
 
 			$p2 = $pasv_port % ((int) 1 << 8);
 			$p1 = ($pasv_port-$p2)/((int) 1 << 8);
 			fputs($s, "227 Entering Passive Mode. (127,0,0,1,{$p1},{$p2})\r\n");
 
-			$pasvs = stream_socket_accept($soc,10);
+            if (empty($bug73457)) {
+                $pasvs = stream_socket_accept($soc,10);
+            }
 
 		} elseif (preg_match('/^EPSV/', $buf, $matches)) {
 			fputs($s, "550 Extended passsive mode not supported.\r\n");
@@ -455,6 +465,8 @@ if ($pid) {
 			fputs($s, "350 OK\r\n");
                 }elseif (preg_match('/^SIZE largefile/', $buf)) {
                         fputs($s, "213 5368709120\r\n");
+                }elseif (preg_match('/^SIZE \/bug73457/', $buf)) {
+                        fputs($s, "213 10\r\n");
                 }else {
 			fputs($s, "500 Syntax error, command unrecognized.\r\n");
 			dump_and_exit($buf);
diff --git a/ext/standard/ftp_fopen_wrapper.c b/ext/standard/ftp_fopen_wrapper.c
index e73787cf3d..9ab27f4868 100644
--- a/ext/standard/ftp_fopen_wrapper.c
+++ b/ext/standard/ftp_fopen_wrapper.c
@@ -430,6 +430,7 @@ php_stream * php_stream_url_wrap_ftp(php_stream_wrapper *wrapper, const char *pa
 	int8_t read_write = 0;
 	char *transport;
 	int transport_len;
+	zend_string *error_message = NULL;
 
 	tmp_line[0] = '\0';
 
@@ -557,9 +558,10 @@ php_stream * php_stream_url_wrap_ftp(php_stream_wrapper *wrapper, const char *pa
 		hoststart = resource->host;
 	}
 	transport_len = (int)spprintf(&transport, 0, "tcp://%s:%d", hoststart, portno);
-	datastream = php_stream_xport_create(transport, transport_len, REPORT_ERRORS, STREAM_XPORT_CLIENT | STREAM_XPORT_CONNECT, NULL, NULL, context, NULL, NULL);
+	datastream = php_stream_xport_create(transport, transport_len, REPORT_ERRORS, STREAM_XPORT_CLIENT | STREAM_XPORT_CONNECT, NULL, NULL, context, &error_message, NULL);
 	efree(transport);
 	if (datastream == NULL) {
+		tmp_line[0]='\0';
 		goto errexit;
 	}
 
@@ -583,6 +585,7 @@ php_stream * php_stream_url_wrap_ftp(php_stream_wrapper *wrapper, const char *pa
 		php_stream_wrapper_log_error(wrapper, options, "Unable to activate SSL mode");
 		php_stream_close(datastream);
 		datastream = NULL;
+		tmp_line[0]='\0';
 		goto errexit;
 	}
 
@@ -602,6 +605,11 @@ errexit:
 	}
 	if (tmp_line[0] != '\0')
 		php_stream_wrapper_log_error(wrapper, options, "FTP server reports %s", tmp_line);
+
+	if (error_message) {
+		php_stream_wrapper_log_error(wrapper, options, "Failed to set up data channel: %s", ZSTR_VAL(error_message));
+		zend_string_release(error_message);
+	}
 	return NULL;
 }
 /* }}} */
diff --git a/ext/standard/tests/streams/bug73457.phpt b/ext/standard/tests/streams/bug73457.phpt
new file mode 100644
index 0000000000..f44c26d8a6
--- /dev/null
+++ b/ext/standard/tests/streams/bug73457.phpt
@@ -0,0 +1,23 @@
+--TEST--
+Bug #73457. Wrong error message when fopen FTP wrapped fails to open data connection
+--SKIPIF--
+<?php
+if (array_search('ftp',stream_get_wrappers()) === FALSE) die("skip ftp wrapper not available.");
+if (!function_exists('pcntl_fork')) die("skip pcntl_fork() not available.");
+?>
+--FILE--
+<?php
+
+$bug73457=true;
+require __DIR__ . "/../../../ftp/tests/server.inc";
+
+$path="ftp://127.0.0.1:" . $port."/bug73457";
+
+$ds=file_get_contents($path);
+var_dump($ds);
+?>
+==DONE== 
+--EXPECTF--
+Warning: file_get_contents(ftp://127.0.0.1:%d/bug73457): failed to open stream: Failed to set up data channel: Connection refused in %s on line %d
+bool(false)
+==DONE==