Fix bug #63937: Upload speed 10 times slower with PHP

ext/ftp/ftp.c

@@ -976,14 +976,84 @@ ftp_get(ftpbuf_t *ftp, php_stream *outstream, const char *path, const size_t pat
 }
 /* }}} */
 
+static zend_result ftp_send_stream_to_data_socket(ftpbuf_t *ftp, databuf_t *data, php_stream *instream, ftptype_t type, bool send_once_and_return)
+{
+	if (type == FTPTYPE_ASCII) {
+		/* Change (and later restore) flags to make sure php_stream_get_line() searches '\n'. */
+		const uint32_t flags_mask = PHP_STREAM_FLAG_EOL_UNIX | PHP_STREAM_FLAG_DETECT_EOL | PHP_STREAM_FLAG_EOL_MAC;
+		uint32_t old_flags = instream->flags & flags_mask;
+		instream->flags = (instream->flags & ~flags_mask) | PHP_STREAM_FLAG_EOL_UNIX;
+
+		char *ptr = data->buf;
+		const char *end = data->buf + FTP_BUFSIZE;
+		while (!php_stream_eof(instream)) {
+			size_t line_length;
+			if (!php_stream_get_line(instream, ptr, end - ptr, &line_length)) {
+				break;
+			}
+
+			ZEND_ASSERT(line_length != 0);
+
+			ptr += line_length - 1;
+			/* Replace \n with \r\n */
+			if (*ptr == '\n') {
+				*ptr = '\r';
+				/* The streams layer always puts a \0 byte at the end of a line,
+				 * so there is always place to add an extra byte. */
+				*++ptr = '\n';
+			}
+
+			ptr++;
+
+			/* If less than 2 bytes remain, either the buffer is completely full or there is a single byte left to put a '\0'
+			 * which isn't really useful, in this case send and reset the buffer. */
+			if (end - ptr < 2) {
+				size_t send_size = FTP_BUFSIZE - (end - ptr);
+				if (UNEXPECTED(my_send(ftp, data->fd, data->buf, send_size) != send_size)) {
+					instream->flags = (instream->flags & ~flags_mask) | old_flags;
+					return FAILURE;
+				}
+				ptr = data->buf;
+				if (send_once_and_return) {
+					break;
+				}
+			}
+		}
+
+		instream->flags = (instream->flags & ~flags_mask) | old_flags;
+
+		if (end - ptr < FTP_BUFSIZE) {
+			size_t send_size = FTP_BUFSIZE - (end - ptr);
+			if (UNEXPECTED(my_send(ftp, data->fd, data->buf, send_size) != send_size)) {
+				return FAILURE;
+			}
+		}
+	} else {
+		while (!php_stream_eof(instream)) {
+			ssize_t size = php_stream_read(instream, data->buf, FTP_BUFSIZE);
+			if (size == 0) {
+				break;
+			}
+			if (UNEXPECTED(size < 0)) {
+				return FAILURE;
+			}
+			if (UNEXPECTED(my_send(ftp, data->fd, data->buf, size) != size)) {
+				return FAILURE;
+			}
+			if (send_once_and_return) {
+				break;
+			}
+		}
+	}
+
+	return SUCCESS;
+}
+
 /* {{{ ftp_put */
 int
 ftp_put(ftpbuf_t *ftp, const char *path, const size_t path_len, php_stream *instream, ftptype_t type, zend_long startpos)
 {
 	databuf_t		*data = NULL;
-	zend_long			size;
-	char			*ptr;
-	int			ch;
 	char			arg[MAX_LENGTH_OF_LONG];
 
 	if (ftp == NULL) {
@@ -1020,30 +1090,10 @@ ftp_put(ftpbuf_t *ftp, const char *path, const size_t path_len, php_stream *inst
 		goto bail;
 	}
 
-	size = 0;
-	ptr = data->buf;
-	while (!php_stream_eof(instream) && (ch = php_stream_getc(instream))!=EOF) {
-		/* flush if necessary */
-		if (FTP_BUFSIZE - size < 2) {
-			if (my_send(ftp, data->fd, data->buf, size) != size) {
-				goto bail;
-			}
-			ptr = data->buf;
-			size = 0;
-		}
-
-		if (ch == '\n' && type == FTPTYPE_ASCII) {
-			*ptr++ = '\r';
-			size++;
-		}
-
-		*ptr++ = ch;
-		size++;
-	}
-
-	if (size && my_send(ftp, data->fd, data->buf, size) != size) {
+	if (ftp_send_stream_to_data_socket(ftp, data, instream, type, false) != SUCCESS) {
 		goto bail;
 	}
+
 	data_close(ftp);
 
 	if (!ftp_getresp(ftp) || (ftp->resp != 226 && ftp->resp != 250 && ftp->resp != 200)) {
@@ -1062,9 +1112,6 @@ int
 ftp_append(ftpbuf_t *ftp, const char *path, const size_t path_len, php_stream *instream, ftptype_t type)
 {
 	databuf_t		*data = NULL;
-	zend_long			size;
-	char			*ptr;
-	int			ch;
 
 	if (ftp == NULL) {
 		return 0;
@@ -1087,30 +1134,10 @@ ftp_append(ftpbuf_t *ftp, const char *path, const size_t path_len, php_stream *i
 		goto bail;
 	}
 
-	size = 0;
-	ptr = data->buf;
-	while (!php_stream_eof(instream) && (ch = php_stream_getc(instream))!=EOF) {
-		/* flush if necessary */
-		if (FTP_BUFSIZE - size < 2) {
-			if (my_send(ftp, data->fd, data->buf, size) != size) {
-				goto bail;
-			}
-			ptr = data->buf;
-			size = 0;
-		}
-
-		if (ch == '\n' && type == FTPTYPE_ASCII) {
-			*ptr++ = '\r';
-			size++;
-		}
-
-		*ptr++ = ch;
-		size++;
-	}
-
-	if (size && my_send(ftp, data->fd, data->buf, size) != size) {
+	if (ftp_send_stream_to_data_socket(ftp, data, instream, type, false) != SUCCESS) {
 		goto bail;
 	}
+
 	data_close(ftp);
 
 	if (!ftp_getresp(ftp) || (ftp->resp != 226 && ftp->resp != 250 && ftp->resp != 200)) {
@@ -2258,39 +2285,19 @@ ftp_nb_put(ftpbuf_t *ftp, const char *path, const size_t path_len, php_stream *i
 int
 ftp_nb_continue_write(ftpbuf_t *ftp)
 {
-	long			size;
-	char			*ptr;
-	int 			ch;
-
 	/* check if we can write more data */
 	if (!data_writeable(ftp, ftp->data->fd)) {
 		return PHP_FTP_MOREDATA;
 	}
 
-	size = 0;
-	ptr = ftp->data->buf;
-	while (!php_stream_eof(ftp->stream) && (ch = php_stream_getc(ftp->stream)) != EOF) {
-
-		if (ch == '\n' && ftp->type == FTPTYPE_ASCII) {
-			*ptr++ = '\r';
-			size++;
-		}
-
-		*ptr++ = ch;
-		size++;
-
-		/* flush if necessary */
-		if (FTP_BUFSIZE - size < 2) {
-			if (my_send(ftp, ftp->data->fd, ftp->data->buf, size) != size) {
-				goto bail;
-			}
-			return PHP_FTP_MOREDATA;
-		}
+	if (ftp_send_stream_to_data_socket(ftp, ftp->data, ftp->stream, ftp->type, true) != SUCCESS) {
+		goto bail;
 	}
 
-	if (size && my_send(ftp, ftp->data->fd, ftp->data->buf, size) != size) {
-		goto bail;
+	if (!php_stream_eof(ftp->stream)) {
+		return PHP_FTP_MOREDATA;
 	}
+
 	data_close(ftp);
 
 	if (!ftp_getresp(ftp) || (ftp->resp != 226 && ftp->resp != 250)) {

ext/ftp/tests/ftp_fput_ascii_over_4_kib.phpt

@@ -0,0 +1,33 @@
+--TEST--
+Testing ftp_fput basic functionality
+--EXTENSIONS--
+ftp
+pcntl
+--FILE--
+<?php
+require 'server.inc';
+
+$ftp = ftp_connect('127.0.0.1', $port);
+ftp_login($ftp, 'user', 'pass');
+$ftp or die("Couldn't connect to the server");
+
+$filename = "large_file.txt";
+$filepath = __DIR__ . "/" . $filename;
+
+// Test on boundary of 2 buffers
+for ($i = 4094; $i < 4098; $i++) {
+    $contents = str_repeat("a", $i) . "\n" . str_repeat("b", 10);
+    file_put_contents($filepath, $contents);
+    var_dump(ftp_put($ftp, "large_file.txt", $filepath, FTP_ASCII));
+}
+
+?>
+--CLEAN--
+<?php
+@unlink(__DIR__ . "/large_file.txt");
+?>
+--EXPECT--
+bool(true)
+bool(true)
+bool(true)
+bool(true)