Fix #79019: Copied cURL handles upload empty file

cmb69 · cmb69 · commit c06777025814 · 2019-12-31T11:06:27.000+01:00
To cater to `curl_copy_handle()` of cURL handles with attached
`CURLFile`s, we must not attach the opened stream, because the stream
may not be seekable, so that we could rewind, when the same stream is
going to be uploaded multiple times.  Instead, we're opening the stream
lazily in the read or seek callback, and close it when reading finished
(either successfully or not) and when seeking failed, respectively.

This behavior is basically the same as for libcurl &lt; 7.56.0, where we
attach the file name, and libcurl reads and uploads the file contents.

To be able to test this behavior, we extend the test responder to print
the size of the upload, and patch the existing tests accordingly.
diff --git a/ext/curl/interface.c b/ext/curl/interface.c
@@ -1796,11 +1796,20 @@ static void curl_free_post(void **post)
 }
 /* }}} */
 
+struct mime_file {
+	zend_string *name;
+	php_stream *stream;
+};
+
 /* {{{ curl_free_stream
  */
 static void curl_free_stream(void **post)
 {
-	php_stream_close((php_stream *)*post);
+	struct mime_file *mime_file = (struct mime_file *) *post;
+
+	ZEND_ASSERT(mime_file->stream == NULL);
+	zend_string_release(mime_file->name);
+	efree(mime_file);
 }
 /* }}} */
 
@@ -1901,7 +1910,7 @@ php_curl *alloc_curl_handle()
 
 	zend_llist_init(&ch->to_free->str,   sizeof(char *),          (llist_dtor_func_t)curl_free_string, 0);
 	zend_llist_init(&ch->to_free->post,  sizeof(struct HttpPost *), (llist_dtor_func_t)curl_free_post,   0);
-	zend_llist_init(&ch->to_free->stream, sizeof(php_stream *),   (llist_dtor_func_t)curl_free_stream, 0);
+	zend_llist_init(&ch->to_free->stream, sizeof(struct mime_file *), (llist_dtor_func_t)curl_free_stream, 0);
 
 	ch->to_free->slist = emalloc(sizeof(HashTable));
 	zend_hash_init(ch->to_free->slist, 4, NULL, curl_free_slist, 0);
@@ -2129,28 +2138,42 @@ PHP_FUNCTION(curl_copy_handle)
 }
 /* }}} */
 
-#if LIBCURL_VERSION_NUM >= 0x073800
+#if LIBCURL_VERSION_NUM >= 0x073800 /* 7.56.0 */
 static size_t read_cb(char *buffer, size_t size, size_t nitems, void *arg) /* {{{ */
 {
-	php_stream *stream = (php_stream *) arg;
-	ssize_t numread = php_stream_read(stream, buffer, nitems * size);
+	struct mime_file *mime_file = (struct mime_file *) arg;
+	ssize_t numread;
 
-	if (numread < 0) {
-		return CURL_READFUNC_ABORT;
+	if (mime_file->stream == NULL) {
+		if (!(mime_file->stream = php_stream_open_wrapper(ZSTR_VAL(mime_file->name), "rb", IGNORE_PATH, NULL))) {
+			return CURL_READFUNC_ABORT;
+		}
 	}
-	return numread;
+	numread = php_stream_read(mime_file->stream, buffer, nitems * size);
+	if (numread <= 0) {
+		php_stream_close(mime_file->stream);
+		mime_file->stream = NULL;
+	}
+	return (numread >= 0) ? numread : CURL_READFUNC_ABORT;
 }
 /* }}} */
 
 static int seek_cb(void *arg, curl_off_t offset, int origin) /* {{{ */
 {
-	php_stream *stream = (php_stream *) arg;
-	int res = php_stream_seek(stream, offset, origin);
+	struct mime_file *mime_file = (struct mime_file *) arg;
+	int res;
 
+	if (mime_file->stream == NULL) {
+		if (!(mime_file->stream = php_stream_open_wrapper(ZSTR_VAL(mime_file->name), "rb", IGNORE_PATH, NULL))) {
+			return CURL_SEEKFUNC_CANTSEEK;
+		}
+	}
+	res = php_stream_seek(mime_file->stream, offset, origin);
 	if (res) {
-		return CURL_SEEKFUNC_CANTSEEK;
+		php_stream_close(mime_file->stream);
+		mime_file->stream = NULL;
 	}
-	return CURL_SEEKFUNC_OK;
+	return !res ? CURL_SEEKFUNC_OK : CURL_SEEKFUNC_CANTSEEK;
 }
 /* }}} */
 #endif
@@ -2793,7 +2816,7 @@ static int _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue) /* {{{
 						zval *prop, rv;
 						char *type = NULL, *filename = NULL;
 #if LIBCURL_VERSION_NUM >= 0x073800 /* 7.56.0 */
-						php_stream *stream;
+						struct mime_file *mime_file;
 #endif
 
 						prop = zend_read_property(curl_CURLFile_class, current, "name", sizeof("name")-1, 0, &rv);
@@ -2816,24 +2839,21 @@ static int _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue) /* {{{
 							}
 
 #if LIBCURL_VERSION_NUM >= 0x073800 /* 7.56.0 */
-							if (!(stream = php_stream_open_wrapper(ZSTR_VAL(postval), "rb", IGNORE_PATH, NULL))) {
-								zend_string_release_ex(string_key, 0);
-								return FAILURE;
-							}
+							mime_file = emalloc(sizeof *mime_file);
+							mime_file->name = zend_string_copy(postval);
+							mime_file->stream = NULL;
 							part = curl_mime_addpart(mime);
 							if (part == NULL) {
-								php_stream_close(stream);
 								zend_string_release_ex(string_key, 0);
 								return FAILURE;
 							}
 							if ((form_error = curl_mime_name(part, ZSTR_VAL(string_key))) != CURLE_OK
-								|| (form_error = curl_mime_data_cb(part, -1, read_cb, seek_cb, NULL, stream)) != CURLE_OK
+								|| (form_error = curl_mime_data_cb(part, -1, read_cb, seek_cb, NULL, mime_file)) != CURLE_OK
 								|| (form_error = curl_mime_filename(part, filename ? filename : ZSTR_VAL(postval))) != CURLE_OK
 								|| (form_error = curl_mime_type(part, type ? type : "application/octet-stream")) != CURLE_OK) {
-								php_stream_close(stream);
 								error = form_error;
 							}
-							zend_llist_add_element(&ch->to_free->stream, &stream);
+							zend_llist_add_element(&ch->to_free->stream, &mime_file);
 #else
 							form_error = curl_formadd(&first, &last,
 											CURLFORM_COPYNAME, ZSTR_VAL(string_key),
diff --git a/ext/curl/tests/bug27023.phpt b/ext/curl/tests/bug27023.phpt
@@ -38,7 +38,7 @@ var_dump(curl_exec($ch));
 curl_close($ch);
 ?>
 --EXPECTF--
-string(%d) "curl_testdata1.txt|application/octet-stream"
-string(%d) "curl_testdata1.txt|text/plain"
-string(%d) "foo.txt|application/octet-stream"
-string(%d) "foo.txt|text/plain"
+string(%d) "curl_testdata1.txt|application/octet-stream|6"
+string(%d) "curl_testdata1.txt|text/plain|6"
+string(%d) "foo.txt|application/octet-stream|6"
+string(%d) "foo.txt|text/plain|6"
diff --git a/ext/curl/tests/bug77711.phpt b/ext/curl/tests/bug77711.phpt
@@ -24,7 +24,7 @@ curl_close($ch);
 ===DONE===
 --EXPECTF--
 bool(true)
-string(%d) "АБВ.txt|application/octet-stream"
+string(%d) "АБВ.txt|application/octet-stream|5"
 ===DONE===
 --CLEAN--
 <?php
diff --git a/ext/curl/tests/curl_copy_handle_variation3.phpt b/ext/curl/tests/curl_copy_handle_variation3.phpt
@@ -29,8 +29,8 @@ curl_close($ch2);
 ===DONE===
 --EXPECTF--
 bool(true)
-string(%d) "АБВ.txt|application/octet-stream"
-string(%d) "АБВ.txt|application/octet-stream"
+string(%d) "АБВ.txt|application/octet-stream|5"
+string(%d) "АБВ.txt|application/octet-stream|5"
 ===DONE===
 --CLEAN--
 <?php
diff --git a/ext/curl/tests/curl_file_upload.phpt b/ext/curl/tests/curl_file_upload.phpt
@@ -60,15 +60,15 @@ var_dump(curl_exec($ch));
 curl_close($ch);
 ?>
 --EXPECTF--
-string(%d) "curl_testdata1.txt|application/octet-stream"
-string(%d) "curl_testdata1.txt|text/plain"
-string(%d) "foo.txt|application/octet-stream"
-string(%d) "foo.txt|text/plain"
+string(%d) "curl_testdata1.txt|application/octet-stream|6"
+string(%d) "curl_testdata1.txt|text/plain|6"
+string(%d) "foo.txt|application/octet-stream|6"
+string(%d) "foo.txt|text/plain|6"
 string(%d) "text/plain"
 string(%d) "%s/curl_testdata1.txt"
-string(%d) "curl_testdata1.txt|text/plain"
+string(%d) "curl_testdata1.txt|text/plain|6"
 string(%d) "foo.txt"
-string(%d) "foo.txt|application/octet-stream"
+string(%d) "foo.txt|application/octet-stream|6"
 
 Warning: curl_setopt(): Disabling safe uploads is no longer supported in %s on line %d
 string(0) ""
diff --git a/ext/curl/tests/curl_file_upload_stream.phpt b/ext/curl/tests/curl_file_upload_stream.phpt
@@ -24,5 +24,5 @@ curl_close($ch);
 ===DONE===
 --EXPECT--
 bool(true)
-string(21) "i-love-php|text/plain"
+string(24) "i-love-php|text/plain|11"
 ===DONE===
diff --git a/ext/curl/tests/responder/get.inc b/ext/curl/tests/responder/get.inc
@@ -28,7 +28,7 @@
       break;
     case 'file':
       if (isset($_FILES['file'])) {
-          echo $_FILES['file']['name'] . '|' . $_FILES['file']['type'];
+          echo $_FILES['file']['name'] . '|' . $_FILES['file']['type'] . '|' . $_FILES['file']['size'];
       }
       break;
     case 'method':

Original file line number	Diff line number	Diff line change
`@@ -28,7 +28,7 @@`
`28`	`28`	`break;`
`29`	`29`	`case 'file':`
`30`	`30`	`if (isset($_FILES['file'])) {`
`31`		`- echo $_FILES['file']['name'] . '\|' . $_FILES['file']['type'];`
	`31`	`+ echo $_FILES['file']['name'] . '\|' . $_FILES['file']['type'] . '\|' . $_FILES['file']['size'];`
`32`	`32`	`}`
`33`	`33`	`break;`
`34`	`34`	`case 'method':`