From e044d36d0dc9d5e231c7017f071c1efd483e77fa Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Tue, 16 Apr 2019 10:38:03 +0200 Subject: [PATCH 1/4] Extend CURLFile to support streams Due to former restrictions of the libcurl API, curl multipart/formdata file uploads supported only proper files. However, as of curl 7.56.0 the new `curl_mime_*()` API is available (and already supported by PHP[1]), which allows us to support arbitrary *seekable* streams, which is generally desirable, and particularly resolves issues with the transparent Unicode and long part support on Windows (see bug #77711). Note that older curl versions are still supported, but CURLFile is still restricted to proper files in this case. [1] --- ext/curl/interface.c | 48 +++++++++++++++++++++++++++++++++++- ext/curl/tests/bug77711.phpt | 32 ++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 ext/curl/tests/bug77711.phpt diff --git a/ext/curl/interface.c b/ext/curl/interface.c index d8b9fcb7ed3b7..5bfbc5ef250ad 100644 --- a/ext/curl/interface.c +++ b/ext/curl/interface.c @@ -2121,6 +2121,40 @@ PHP_FUNCTION(curl_copy_handle) } /* }}} */ +#if LIBCURL_VERSION_NUM >= 0x073800 +static size_t read_cb(char *buffer, size_t size, size_t nitems, void *arg) /* {{{ */ +{ + php_stream *stream = (php_stream *) arg; + size_t numread = php_stream_read(stream, buffer, nitems * size); + + if (numread == (size_t)-1) { + return CURL_READFUNC_ABORT; + } + return numread; +} +/* }}} */ + +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); + + if (res) { + return CURL_SEEKFUNC_FAIL; + } + return CURL_SEEKFUNC_OK; +} +/* }}} */ + +static void free_cb(void *arg) /* {{{ */ +{ + php_stream *stream = (php_stream *) arg; + + php_stream_close(stream); +} +/* }}} */ +#endif + static int _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue) /* {{{ */ { CURLcode error = CURLE_OK; @@ -2756,6 +2790,10 @@ static int _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue) /* {{{ /* new-style file upload */ zval *prop, rv; char *type = NULL, *filename = NULL; +#if LIBCURL_VERSION_NUM >= 0x073800 /* 7.56.0 */ + php_stream *stream; + php_stream_statbuf ssb; +#endif prop = zend_read_property(curl_CURLFile_class, current, "name", sizeof("name")-1, 0, &rv); if (Z_TYPE_P(prop) != IS_STRING) { @@ -2777,13 +2815,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", STREAM_MUST_SEEK, NULL))) { + zend_string_release_ex(string_key, 0); + return FAILURE; + } + if (php_stream_stat(stream, &ssb)) { + zend_string_release_ex(string_key, 0); + return FAILURE; + } part = curl_mime_addpart(mime); if (part == NULL) { 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_filedata(part, ZSTR_VAL(postval))) != CURLE_OK + || (form_error = curl_mime_data_cb(part, ssb.sb.st_size, read_cb, seek_cb, free_cb, stream)) != 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) { error = form_error; diff --git a/ext/curl/tests/bug77711.phpt b/ext/curl/tests/bug77711.phpt new file mode 100644 index 0000000000000..148c26322a64b --- /dev/null +++ b/ext/curl/tests/bug77711.phpt @@ -0,0 +1,32 @@ +--TEST-- +FR #77711 (CURLFile should support UNICODE filenames) +--SKIPIF-- + +--FILE-- + $file); +var_dump(curl_setopt($ch, CURLOPT_POSTFIELDS, $params)); + +var_dump(curl_exec($ch)); +curl_close($ch); +?> +===DONE=== +--EXPECTF-- +bool(true) +string(%d) "АБВ.txt|application/octet-stream" +===DONE=== +--CLEAN-- + From f57be0d2da2bbef56c789ad41105b88e98091153 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Tue, 16 Apr 2019 13:02:21 +0200 Subject: [PATCH 2/4] Also support non-statable and possibly non-seekable streams We pass `-1` as `datasize` to avoid the `stat` call, and let curl handle non-seekable streams. --- ext/curl/interface.c | 11 +++------ ext/curl/tests/curl_file_upload_stream.phpt | 26 +++++++++++++++++++++ 2 files changed, 29 insertions(+), 8 deletions(-) create mode 100644 ext/curl/tests/curl_file_upload_stream.phpt diff --git a/ext/curl/interface.c b/ext/curl/interface.c index 5bfbc5ef250ad..d88cf77b9ff5b 100644 --- a/ext/curl/interface.c +++ b/ext/curl/interface.c @@ -2140,7 +2140,7 @@ static int seek_cb(void *arg, curl_off_t offset, int origin) /* {{{ */ int res = php_stream_seek(stream, offset, origin); if (res) { - return CURL_SEEKFUNC_FAIL; + return CURL_SEEKFUNC_CANTSEEK; } return CURL_SEEKFUNC_OK; } @@ -2792,7 +2792,6 @@ static int _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue) /* {{{ char *type = NULL, *filename = NULL; #if LIBCURL_VERSION_NUM >= 0x073800 /* 7.56.0 */ php_stream *stream; - php_stream_statbuf ssb; #endif prop = zend_read_property(curl_CURLFile_class, current, "name", sizeof("name")-1, 0, &rv); @@ -2815,11 +2814,7 @@ 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", STREAM_MUST_SEEK, NULL))) { - zend_string_release_ex(string_key, 0); - return FAILURE; - } - if (php_stream_stat(stream, &ssb)) { + if (!(stream = php_stream_open_wrapper(ZSTR_VAL(postval), "rb", IGNORE_PATH, NULL))) { zend_string_release_ex(string_key, 0); return FAILURE; } @@ -2829,7 +2824,7 @@ static int _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue) /* {{{ return FAILURE; } if ((form_error = curl_mime_name(part, ZSTR_VAL(string_key))) != CURLE_OK - || (form_error = curl_mime_data_cb(part, ssb.sb.st_size, read_cb, seek_cb, free_cb, stream)) != CURLE_OK + || (form_error = curl_mime_data_cb(part, -1, read_cb, seek_cb, free_cb, stream)) != 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) { error = form_error; diff --git a/ext/curl/tests/curl_file_upload_stream.phpt b/ext/curl/tests/curl_file_upload_stream.phpt new file mode 100644 index 0000000000000..16e4a65b297cb --- /dev/null +++ b/ext/curl/tests/curl_file_upload_stream.phpt @@ -0,0 +1,26 @@ +--TEST-- +CURL file uploading from stream +--SKIPIF-- + +--FILE-- + $file); +var_dump(curl_setopt($ch, CURLOPT_POSTFIELDS, $params)); + +var_dump(curl_exec($ch)); +curl_close($ch); +?> +===DONE=== +--EXPECT-- +bool(true) +string(21) "i-love-php|text/plain" +===DONE=== From 81ff9a6ddfd3a1f94ae886c3ba0b9b0e91812471 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Tue, 16 Apr 2019 15:28:20 +0200 Subject: [PATCH 3/4] Skip test for old curl versions --- ext/curl/tests/curl_file_upload_stream.phpt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ext/curl/tests/curl_file_upload_stream.phpt b/ext/curl/tests/curl_file_upload_stream.phpt index 16e4a65b297cb..03c85b4b82be9 100644 --- a/ext/curl/tests/curl_file_upload_stream.phpt +++ b/ext/curl/tests/curl_file_upload_stream.phpt @@ -2,6 +2,8 @@ CURL file uploading from stream --SKIPIF-- += 7.56.0'); --FILE-- Date: Fri, 26 Apr 2019 16:55:03 +0200 Subject: [PATCH 4/4] Cater to duplicated CURL handles If the CURL handle is duplicated, the `freefunc` is called multiple times, so we must take care not to close the stream prematurely. Thus, instead of passing a `freefunc` to `curl_mime_data_cb`, we add the streams to a dedicated `zend_llist` which we release when the last duplicated CURL handle is destroyed. --- ext/curl/interface.c | 23 ++++++----- ext/curl/php_curl.h | 1 + .../tests/curl_copy_handle_variation3.phpt | 38 +++++++++++++++++++ 3 files changed, 53 insertions(+), 9 deletions(-) create mode 100644 ext/curl/tests/curl_copy_handle_variation3.phpt diff --git a/ext/curl/interface.c b/ext/curl/interface.c index d88cf77b9ff5b..d03f81b249b83 100644 --- a/ext/curl/interface.c +++ b/ext/curl/interface.c @@ -1803,6 +1803,14 @@ static void curl_free_post(void **post) } /* }}} */ +/* {{{ curl_free_stream + */ +static void curl_free_stream(void **post) +{ + php_stream_close((php_stream *)*post); +} +/* }}} */ + /* {{{ curl_free_slist */ static void curl_free_slist(zval *el) @@ -1894,6 +1902,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); ch->to_free->slist = emalloc(sizeof(HashTable)); zend_hash_init(ch->to_free->slist, 4, NULL, curl_free_slist, 0); @@ -2145,14 +2154,6 @@ static int seek_cb(void *arg, curl_off_t offset, int origin) /* {{{ */ return CURL_SEEKFUNC_OK; } /* }}} */ - -static void free_cb(void *arg) /* {{{ */ -{ - php_stream *stream = (php_stream *) arg; - - php_stream_close(stream); -} -/* }}} */ #endif static int _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue) /* {{{ */ @@ -2820,15 +2821,18 @@ static int _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue) /* {{{ } 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, free_cb, stream)) != CURLE_OK + || (form_error = curl_mime_data_cb(part, -1, read_cb, seek_cb, NULL, stream)) != 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); #else form_error = curl_formadd(&first, &last, CURLFORM_COPYNAME, ZSTR_VAL(string_key), @@ -3558,6 +3562,7 @@ static void _php_curl_close_ex(php_curl *ch) if (--(*ch->clone) == 0) { zend_llist_clean(&ch->to_free->str); zend_llist_clean(&ch->to_free->post); + zend_llist_clean(&ch->to_free->stream); zend_hash_destroy(ch->to_free->slist); efree(ch->to_free->slist); efree(ch->to_free); diff --git a/ext/curl/php_curl.h b/ext/curl/php_curl.h index f289c31645024..359d6058efde8 100644 --- a/ext/curl/php_curl.h +++ b/ext/curl/php_curl.h @@ -167,6 +167,7 @@ struct _php_curl_send_headers { struct _php_curl_free { zend_llist str; zend_llist post; + zend_llist stream; HashTable *slist; }; diff --git a/ext/curl/tests/curl_copy_handle_variation3.phpt b/ext/curl/tests/curl_copy_handle_variation3.phpt new file mode 100644 index 0000000000000..18f35f71b1535 --- /dev/null +++ b/ext/curl/tests/curl_copy_handle_variation3.phpt @@ -0,0 +1,38 @@ +--TEST-- +curl_copy_handle() allows to post CURLFile multiple times +--SKIPIF-- + +--FILE-- + $file); +var_dump(curl_setopt($ch1, CURLOPT_POSTFIELDS, $params)); + +$ch2 = curl_copy_handle($ch1); + +var_dump(curl_exec($ch1)); +curl_close($ch1); + +var_dump(curl_exec($ch2)); +curl_close($ch2); +?> +===DONE=== +--EXPECTF-- +bool(true) +string(%d) "АБВ.txt|application/octet-stream" +string(%d) "АБВ.txt|application/octet-stream" +===DONE=== +--CLEAN-- +