From: Hannes Magnusson Date: Mon, 12 Sep 2011 09:16:04 +0000 (+0000) Subject: Fixed issues when streams were closed before curl read/write from them, or cleaning X-Git-Tag: php-5.3.9RC1~150 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=e43c21e53afcbc09639f16100d342215b7ec19dd;p=php Fixed issues when streams were closed before curl read/write from them, or cleaning Closing a original handle after copying it now no longer cleans up all resources (fixes missing CURLOPT_POSTFIELDS values among others) --- diff --git a/ext/curl/interface.c b/ext/curl/interface.c index f2c185f4d2..935f153acf 100644 --- a/ext/curl/interface.c +++ b/ext/curl/interface.c @@ -150,6 +150,7 @@ static struct gcry_thread_cbs php_curl_gnutls_tsl = { static void _php_curl_close_ex(php_curl *ch TSRMLS_DC); static void _php_curl_close(zend_rsrc_list_entry *rsrc TSRMLS_DC); + #define SAVE_CURL_ERROR(__handle, __err) (__handle)->err.no = (int) __err; #define CAAL(s, v) add_assoc_long_ex(return_value, s, sizeof(s), (long) v); @@ -204,6 +205,72 @@ static int php_curl_option_url(php_curl *ch, const char *url, const int len) /* } /* }}} */ +int _php_curl_verify_handlers(php_curl *ch, int reporterror TSRMLS_DC) /* {{{ */ +{ + php_stream *stream; + if (!ch || !ch->handlers) { + return 0; + } + + if (ch->handlers->std_err) { + stream = (php_stream *) zend_fetch_resource(&ch->handlers->std_err TSRMLS_CC, -1, NULL, NULL, 2, php_file_le_stream(), php_file_le_pstream()); + if (stream == NULL) { + if (reporterror) { + php_error_docref(NULL TSRMLS_CC, E_WARNING, "CURLOPT_STDERR resource has gone away, resetting to stderr"); + } + zval_ptr_dtor(&ch->handlers->std_err); + ch->handlers->std_err = NULL; + + curl_easy_setopt(ch->cp, CURLOPT_STDERR, stderr); + } + } + if (ch->handlers->read && ch->handlers->read->stream) { + stream = (php_stream *) zend_fetch_resource(&ch->handlers->read->stream TSRMLS_CC, -1, NULL, NULL, 2, php_file_le_stream(), php_file_le_pstream()); + if (stream == NULL) { + if (reporterror) { + php_error_docref(NULL TSRMLS_CC, E_WARNING, "CURLOPT_INFILE resource has gone away, resetting to default"); + } + zval_ptr_dtor(&ch->handlers->read->stream); + ch->handlers->read->fd = 0; + ch->handlers->read->fp = 0; + ch->handlers->read->stream = NULL; + + curl_easy_setopt(ch->cp, CURLOPT_INFILE, (void *) ch); + } + } + if (ch->handlers->write_header && ch->handlers->write_header->stream) { + stream = (php_stream *) zend_fetch_resource(&ch->handlers->write_header->stream TSRMLS_CC, -1, NULL, NULL, 2, php_file_le_stream(), php_file_le_pstream()); + if (stream == NULL) { + if (reporterror) { + php_error_docref(NULL TSRMLS_CC, E_WARNING, "CURLOPT_WRITEHEADER resource has gone away, resetting to default"); + } + zval_ptr_dtor(&ch->handlers->write_header->stream); + ch->handlers->write_header->fp = 0; + ch->handlers->write_header->stream = NULL; + + ch->handlers->write_header->method = PHP_CURL_IGNORE; + curl_easy_setopt(ch->cp, CURLOPT_WRITEHEADER, (void *) ch); + } + } + if (ch->handlers->write && ch->handlers->write->stream) { + stream = (php_stream *) zend_fetch_resource(&ch->handlers->write->stream TSRMLS_CC, -1, NULL, NULL, 2, php_file_le_stream(), php_file_le_pstream()); + if (stream == NULL) { + if (reporterror) { + php_error_docref(NULL TSRMLS_CC, E_WARNING, "CURLOPT_FILE resource has gone away, resetting to default"); + } + zval_ptr_dtor(&ch->handlers->write->stream); + ch->handlers->write->fp = 0; + ch->handlers->write->stream = NULL; + + ch->handlers->write->method = PHP_CURL_STDOUT; + ch->handlers->write->type = PHP_CURL_ASCII; + curl_easy_setopt(ch->cp, CURLOPT_FILE, (void *) ch); + } + } + return 1; +} +/* }}} */ + /* {{{ arginfo */ ZEND_BEGIN_ARG_INFO_EX(arginfo_curl_version, 0, 0, 0) ZEND_ARG_INFO(0, version) @@ -337,7 +404,6 @@ PHP_INI_BEGIN() PHP_INI_END() /* }}} */ -/* }}} */ /* {{{ PHP_MINFO_FUNCTION */ PHP_MINFO_FUNCTION(curl) @@ -1394,6 +1460,7 @@ static void split_certinfo(char *string, zval *hash) efree(org); } } +/* }}} */ /* {{{ create_certinfo */ @@ -1537,6 +1604,7 @@ PHP_FUNCTION(curl_copy_handle) dupch->cp = cp; dupch->uses = 0; + ch->uses++; if (ch->handlers->write->stream) { Z_ADDREF_P(dupch->handlers->write->stream); dupch->handlers->write->stream = ch->handlers->write->stream; @@ -2211,28 +2279,10 @@ PHP_FUNCTION(curl_exec) ZEND_FETCH_RESOURCE(ch, php_curl *, &zid, -1, le_curl_name, le_curl); + _php_curl_verify_handlers(ch, 1 TSRMLS_CC); + _php_curl_cleanup_handle(ch); - if (ch->handlers->std_err) { - php_stream *stream; - stream = (php_stream*)zend_fetch_resource(&ch->handlers->std_err TSRMLS_CC, -1, NULL, NULL, 2, php_file_le_stream(), php_file_le_pstream()); - if (stream == NULL) { - php_error_docref(NULL TSRMLS_CC, E_WARNING, "CURLOPT_STDERR resource has gone away, resetting to stderr"); - zval_ptr_dtor(&ch->handlers->std_err); - curl_easy_setopt(ch->cp, CURLOPT_STDERR, stderr); - } - } - if (ch->handlers->read && ch->handlers->read->stream) { - php_stream *stream; - stream = (php_stream*)zend_fetch_resource(&ch->handlers->read->stream TSRMLS_CC, -1, NULL, NULL, 2, php_file_le_stream(), php_file_le_pstream()); - if (stream == NULL) { - php_error_docref(NULL TSRMLS_CC, E_WARNING, "CURLOPT_INFILE resource has gone away, resetting to default"); - zval_ptr_dtor(&ch->handlers->read->stream); - ch->handlers->read->fd = 0; - ch->handlers->read->fp = 0; - curl_easy_setopt(ch->cp, CURLOPT_INFILE, (void *) ch); - } - } error = curl_easy_perform(ch->cp); SAVE_CURL_ERROR(ch, error); /* CURLE_PARTIAL_FILE is returned by HEAD requests */ @@ -2243,6 +2293,14 @@ PHP_FUNCTION(curl_exec) RETURN_FALSE; } + if (ch->handlers->std_err) { + php_stream *stream; + stream = (php_stream*)zend_fetch_resource(&ch->handlers->std_err TSRMLS_CC, -1, NULL, NULL, 2, php_file_le_stream(), php_file_le_pstream()); + if (stream) { + php_stream_flush(stream); + } + } + if (ch->handlers->write->method == PHP_CURL_RETURN && ch->handlers->write->buf.len > 0) { smart_str_0(&ch->handlers->write->buf); RETURN_STRINGL(ch->handlers->write->buf.c, ch->handlers->write->buf.len, 1); @@ -2520,11 +2578,7 @@ static void _php_curl_close_ex(php_curl *ch TSRMLS_DC) fprintf(stderr, "DTOR CALLED, ch = %x\n", ch); #endif - /* Prevent crash inside cURL if passed file has already been closed */ - if (ch->handlers->std_err && Z_REFCOUNT_P(ch->handlers->std_err) <= 0) { - curl_easy_setopt(ch->cp, CURLOPT_STDERR, stderr); - } - + _php_curl_verify_handlers(ch, 0 TSRMLS_CC); curl_easy_cleanup(ch->cp); zend_llist_clean(&ch->to_free.str); diff --git a/ext/curl/multi.c b/ext/curl/multi.c index edcf013974..1c498e7fb1 100644 --- a/ext/curl/multi.c +++ b/ext/curl/multi.c @@ -211,6 +211,19 @@ PHP_FUNCTION(curl_multi_exec) ZEND_FETCH_RESOURCE(mh, php_curlm *, &z_mh, -1, le_curl_multi_handle_name, le_curl_multi_handle); + { + zend_llist_position pos; + php_curl *ch; + zval *pz_ch; + + for(pz_ch = (zval *)zend_llist_get_first_ex(&mh->easyh, &pos); pz_ch; + pz_ch = (zval *)zend_llist_get_next_ex(&mh->easyh, &pos)) { + + ZEND_FETCH_RESOURCE(ch, php_curl *, &pz_ch, -1, le_curl_name, le_curl); + _php_curl_verify_handlers(ch, 1 TSRMLS_CC); + } + } + convert_to_long_ex(&z_still_running); still_running = Z_LVAL_P(z_still_running); result = curl_multi_perform(mh->multi, &still_running); @@ -324,6 +337,17 @@ void _php_curl_multi_close(zend_rsrc_list_entry *rsrc TSRMLS_DC) /* {{{ */ { php_curlm *mh = (php_curlm *) rsrc->ptr; if (mh) { + zend_llist_position pos; + php_curl *ch; + zval *pz_ch; + + for(pz_ch = (zval *)zend_llist_get_first_ex(&mh->easyh, &pos); pz_ch; + pz_ch = (zval *)zend_llist_get_next_ex(&mh->easyh, &pos)) { + + ch = (php_curl *) zend_fetch_resource(&pz_ch TSRMLS_CC, -1, le_curl_name, NULL, 1, le_curl); + _php_curl_verify_handlers(ch, 0 TSRMLS_CC); + } + curl_multi_cleanup(mh->multi); zend_llist_clean(&mh->easyh); efree(mh); diff --git a/ext/curl/php_curl.h b/ext/curl/php_curl.h index d7cbb4051a..a50a2b8270 100644 --- a/ext/curl/php_curl.h +++ b/ext/curl/php_curl.h @@ -150,6 +150,7 @@ typedef struct { void _php_curl_cleanup_handle(php_curl *); void _php_curl_multi_cleanup_list(void *data); +int _php_curl_verify_handlers(php_curl *ch, int reporterror TSRMLS_DC); /* streams support */ diff --git a/ext/curl/tests/bug48203_multi.phpt b/ext/curl/tests/bug48203_multi.phpt index 0be5135948..01dc3cdd27 100644 --- a/ext/curl/tests/bug48203_multi.phpt +++ b/ext/curl/tests/bug48203_multi.phpt @@ -67,40 +67,24 @@ foreach($options_to_check as $option) { --CLEAN-- --EXPECTF-- -Warning: curl_multi_exec(): %d is not a valid stream resource in %s on line %d +Warning: curl_multi_exec(): CURLOPT_STDERR resource has gone away, resetting to stderr in %sbug48203_multi.php on line 36 -Warning: curl_multi_exec(): CURLOPT_STDERR handle is incorrect in %s on line %d - -Warning: curl_multi_exec(): %d is not a valid stream resource in %s on line %d - -Warning: curl_multi_exec(): CURLOPT_STDERR handle is incorrect in %s on line %d -%a +Warning: curl_multi_exec(): CURLOPT_STDERR resource has gone away, resetting to stderr in %sbug48203_multi.php on line 36 +%A Ok for CURLOPT_STDERR -Warning: curl_multi_exec(): %d is not a valid stream resource in %s on line %d - -Warning: curl_multi_exec(): CURLOPT_WRITEHEADER handle is incorrect in %s on line %d - -Warning: curl_multi_exec(): %d is not a valid stream resource in %s on line %d +Warning: curl_multi_exec(): CURLOPT_WRITEHEADER resource has gone away, resetting to default in %sbug48203_multi.php on line 36 -Warning: curl_multi_exec(): CURLOPT_WRITEHEADER handle is incorrect in %s on line %d -%AOk for CURLOPT_WRITEHEADER +Warning: curl_multi_exec(): CURLOPT_WRITEHEADER resource has gone away, resetting to default in %sbug48203_multi.php on line 36 +Ok for CURLOPT_WRITEHEADER -Warning: curl_multi_exec(): %d is not a valid stream resource in %s on line %d +Warning: curl_multi_exec(): CURLOPT_FILE resource has gone away, resetting to default in %sbug48203_multi.php on line 36 -Warning: curl_multi_exec(): CURLOPT_FILE handle is incorrect in %s on line %d - -Warning: curl_multi_exec(): %d is not a valid stream resource in %s on line %d - -Warning: curl_multi_exec(): CURLOPT_FILE handle is incorrect in %s on line %d -%a +Warning: curl_multi_exec(): CURLOPT_FILE resource has gone away, resetting to default in %sbug48203_multi.php on line 36 +%A Ok for CURLOPT_FILE -Warning: curl_multi_exec(): %d is not a valid stream resource in %s on line %d - -Warning: curl_multi_exec(): CURLOPT_INFILE handle is incorrect in %s on line %d - -Warning: curl_multi_exec(): %d is not a valid stream resource in %s on line %d +Warning: curl_multi_exec(): CURLOPT_INFILE resource has gone away, resetting to default in %sbug48203_multi.php on line 36 -Warning: curl_multi_exec(): CURLOPT_INFILE handle is incorrect in %s on line %d +Warning: curl_multi_exec(): CURLOPT_INFILE resource has gone away, resetting to default in %sbug48203_multi.php on line 36 Ok for CURLOPT_INFILE diff --git a/ext/curl/tests/bug54798.phpt b/ext/curl/tests/bug54798.phpt index 6fd85131d3..390c2eebb1 100644 --- a/ext/curl/tests/bug54798.phpt +++ b/ext/curl/tests/bug54798.phpt @@ -59,7 +59,12 @@ Warning: curl_exec(): CURLOPT_STDERR resource has gone away, resetting to stderr * About to connect() %a * Closing connection #%d Ok for CURLOPT_STDERR + +Warning: curl_exec(): CURLOPT_WRITEHEADER resource has gone away, resetting to default in %sbug54798.php on line 24 Ok for CURLOPT_WRITEHEADER + +Warning: curl_exec(): CURLOPT_FILE resource has gone away, resetting to default in %sbug54798.php on line 24 +%a Ok for CURLOPT_FILE Warning: curl_exec(): CURLOPT_INFILE resource has gone away, resetting to default in %sbug54798.php on line %d diff --git a/ext/curl/tests/curl_copy_handle_basic_002.phpt b/ext/curl/tests/curl_copy_handle_basic_002.phpt index 0ec58436ee..9ab33635fb 100644 --- a/ext/curl/tests/curl_copy_handle_basic_002.phpt +++ b/ext/curl/tests/curl_copy_handle_basic_002.phpt @@ -29,8 +29,6 @@ Rick Buitenman var_dump( $curl_content ); ?> ===DONE=== ---XFAIL-- -This test fails, the copy seems to be missing the CURLOPT_POSTFIELDS after the original is closed --EXPECTF-- *** Testing curl copy handle with simple POST *** string(163) "array(1) { diff --git a/ext/curl/tests/curl_copy_handle_basic_005.phpt b/ext/curl/tests/curl_copy_handle_basic_005.phpt index 9444961e8b..aa9e2fa998 100644 --- a/ext/curl/tests/curl_copy_handle_basic_005.phpt +++ b/ext/curl/tests/curl_copy_handle_basic_005.phpt @@ -32,8 +32,6 @@ Rick Buitenman var_dump( $curl_content_copy ); ?> ===DONE=== ---XFAIL-- -This test fails, the output of the copy seems to be corrupted if the original is closed after exec() --EXPECTF-- *** Test curl_copy_handle() after exec() with POST *** string(163) "array(1) { diff --git a/ext/curl/tests/curl_file_deleted_before_curl_close.phpt b/ext/curl/tests/curl_file_deleted_before_curl_close.phpt index 680ae547ce..592f110fbe 100644 --- a/ext/curl/tests/curl_file_deleted_before_curl_close.phpt +++ b/ext/curl/tests/curl_file_deleted_before_curl_close.phpt @@ -34,4 +34,5 @@ echo "Closed correctly\n"; unlink(dirname(__FILE__) . '/curl_file_deleted_before_curl_close.tmp'); ?> --EXPECTF-- +* Closing connection #%d Closed correctly diff --git a/ext/curl/tests/curl_setopt_basic002.phpt b/ext/curl/tests/curl_setopt_basic002.phpt index 328af0d906..d90ecb7bd2 100644 --- a/ext/curl/tests/curl_setopt_basic002.phpt +++ b/ext/curl/tests/curl_setopt_basic002.phpt @@ -48,3 +48,5 @@ curl_close($ch); *** Testing curl_setopt with CURLOPT_STDERR string(%d) "%S" string(%d) "%S" +* Closing connection #%d +