# PATCH for bug 48774 - Thread: https://externals.io/message/45027 - Original archive: https://news-web.php.net/php.internals/45027 - Started by Sriram Natarajan on 2009-07-18T06:43:10+00:00 - Messages: 4 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #45027 — Sriram Natarajan — 2009-07-18T06:43:10+00:00 URL: https://externals.io/message/45027#45027 Hi I was looking into CR: 48774(http://bugs.php.net/bug.php?id=48774&thanks=3) and was hoping if some one can kindly review this patch. This patch does address the SEGV issue reported in the bug. Can this be applied to PHP 5.3 and PHP 5.2 as well ? Index: ext/curl/interface.c =================================================================== --- ext/curl/interface.c (revision 284171) +++ ext/curl/interface.c (working copy) @@ -1328,6 +1328,7 @@ { php_curl *ch; CURL *cp; + zval *clone; char *url = NULL; int url_len = 0; @@ -1353,6 +1354,9 @@ ch->uses = 0; + MAKE_STD_ZVAL(clone); + ch->clone = clone; + curl_easy_setopt(ch->cp, CURLOPT_NOPROGRESS, 1); curl_easy_setopt(ch->cp, CURLOPT_VERBOSE, 0); curl_easy_setopt(ch->cp, CURLOPT_ERRORBUFFER, ch->err.str); @@ -1448,6 +1452,10 @@ zend_llist_copy(&dupch->to_free.slist, &ch->to_free.slist); zend_llist_copy(&dupch->to_free.post, &ch->to_free.post); + /* Keep track of cloned copies to avoid invoking curl destructors for every clone */ + Z_ADDREF_P(ch->clone); + dupch->clone = ch->clone; + ZEND_REGISTER_RESOURCE(return_value, dupch, le_curl); dupch->id = Z_LVAL_P(return_value); } @@ -2298,9 +2306,20 @@ #if LIBCURL_VERSION_NUM < 0x071101 zend_llist_clean(&ch->to_free.str); #endif - zend_llist_clean(&ch->to_free.slist); - zend_llist_clean(&ch->to_free.post); + /* cURL destructors should be invoked only by last curl handle */ + if (Z_REFCOUNT_P(ch->clone) <= 1) { + zend_llist_clean(&ch->to_free.slist); + zend_llist_clean(&ch->to_free.post); + zval_ptr_dtor(&ch->clone); + } else { + Z_DELREF_P(ch->clone); + ch->to_free.slist.dtor = NULL; + ch->to_free.post.dtor = NULL; + zend_llist_clean(&ch->to_free.slist); + zend_llist_clean(&ch->to_free.post); + } + if (ch->handlers->write->buf.len > 0) { smart_str_free(&ch->handlers->write->buf); } Index: ext/curl/tests/curl_copy_handle_basic_007.phpt =================================================================== --- ext/curl/tests/curl_copy_handle_basic_007.phpt (revision 0) +++ ext/curl/tests/curl_copy_handle_basic_007.phpt (revision 0) @@ -0,0 +1,44 @@ +--TEST-- +Test `curl_copy_handle()` with simple POST +--SKIPIF-- + +--FILE-- + "World", "Foo" => "Bar", "Person" => "John Doe")); + curl_setopt($ch, CURLOPT_URL, $url); //set the url we want to use + + $copy = curl_copy_handle($ch); + curl_close($ch); + + $curl_content = curl_exec($copy); + curl_close($copy); + + var_dump( $curl_content ); +?> +===DONE=== +--EXPECTF-- +*** Testing curl copy handle with simple POST using array as arguments *** +string(163) "array(1) { + ["test"]=> + string(7) "getpost" +} +array(3) { + ["Hello"]=> + string(5) "World" + ["Foo"]=> + string(3) "Bar" + ["Person"]=> + string(8) "John Doe" +} +" +===DONE=== Index: ext/curl/php_curl.h =================================================================== --- ext/curl/php_curl.h (revision 284171) +++ ext/curl/php_curl.h (working copy) @@ -139,6 +139,7 @@ long id; unsigned int uses; zend_bool in_callback; + zval *clone; } php_curl; typedef struct { Index: NEWS =================================================================== --- NEWS (revision 284171) +++ NEWS (working copy) @@ -32,6 +32,8 @@ (markril at hotmail dot com, Pierre) - Fixed bug #38091 (Mail() does not use FQDN when sending SMTP helo). (Kalle, Rick Yorgason) +- Fixed bug #48774 (SIGSEGVs when using `curl_copy_handle()`). + (Sriram Natarajan) 30 Jun 2009, PHP 5.3.0 - Upgraded bundled PCRE to version 7.9. (Nuno) thanks sriram --- ## #45028 — Jani Taskinen — 2009-07-18T08:37:03+00:00 URL: https://externals.io/message/45027#45028 · In reply to: #45027 Sriram Natarajan wrote: > Hi > I was looking into CR: > 48774(http://bugs.php.net/bug.php?id=48774&thanks=3) and was hoping if > some one can kindly review this patch. This patch does address the SEGV > issue reported in the bug. Can this be applied to PHP 5.3 and PHP 5.2 as > well ? > > > Index: ext/curl/interface.c > =================================================================== > --- ext/curl/interface.c (revision 284171) > +++ ext/curl/interface.c (working copy) > @@ -1328,6 +1328,7 @@ > { > php_curl *ch; > CURL *cp; > + zval *clone; Fix your whitespace first. We use tabs only in .c files.. --Jani > char *url = NULL; > int url_len = 0; > > @@ -1353,6 +1354,9 @@ > > ch->uses = 0; > > + MAKE_STD_ZVAL(clone); > + ch->clone = clone; > + > curl_easy_setopt(ch->cp, CURLOPT_NOPROGRESS, 1); > curl_easy_setopt(ch->cp, CURLOPT_VERBOSE, 0); > curl_easy_setopt(ch->cp, CURLOPT_ERRORBUFFER, ch->err.str); > @@ -1448,6 +1452,10 @@ > zend_llist_copy(&dupch->to_free.slist, &ch->to_free.slist); > zend_llist_copy(&dupch->to_free.post, &ch->to_free.post); > > + /* Keep track of cloned copies to avoid invoking curl > destructors for every clone */ > + Z_ADDREF_P(ch->clone); > + dupch->clone = ch->clone; > + > ZEND_REGISTER_RESOURCE(return_value, dupch, le_curl); > dupch->id = Z_LVAL_P(return_value); > } > @@ -2298,9 +2306,20 @@ > #if LIBCURL_VERSION_NUM < 0x071101 > zend_llist_clean(&ch->to_free.str); > #endif > - zend_llist_clean(&ch->to_free.slist); > - zend_llist_clean(&ch->to_free.post); > > + /* cURL destructors should be invoked only by last curl handle */ > + if (Z_REFCOUNT_P(ch->clone) <= 1) { > + zend_llist_clean(&ch->to_free.slist); > + zend_llist_clean(&ch->to_free.post); > + zval_ptr_dtor(&ch->clone); > + } else { > + Z_DELREF_P(ch->clone); > + ch->to_free.slist.dtor = NULL; > + ch->to_free.post.dtor = NULL; > + zend_llist_clean(&ch->to_free.slist); > + zend_llist_clean(&ch->to_free.post); > + } > + > if (ch->handlers->write->buf.len > 0) { > smart_str_free(&ch->handlers->write->buf); > } > Index: ext/curl/tests/curl_copy_handle_basic_007.phpt > =================================================================== > --- ext/curl/tests/curl_copy_handle_basic_007.phpt (revision 0) > +++ ext/curl/tests/curl_copy_handle_basic_007.phpt (revision 0) > @@ -0,0 +1,44 @@ > +--TEST-- > +Test `curl_copy_handle()` with simple POST > +--SKIPIF-- > + getenv('PHP_CURL_HTTP_REMOTE_SERVER')) print "skip"; ?> > +--FILE-- > + + $host = getenv('PHP_CURL_HTTP_REMOTE_SERVER'); > + > + echo '*** Testing curl copy handle with simple POST using array as > arguments ***' . "\n"; > + > + $url = "{$host}/get.php?test=getpost"; > + $ch = `curl_init()`; > + > + `ob_start()`; // start output buffering > + curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1); > + curl_setopt($ch, CURLOPT_POST, 1); > + curl_setopt($ch, CURLOPT_POSTFIELDS, array("Hello" => "World", "Foo" > => "Bar", "Person" => "John Doe")); > + curl_setopt($ch, CURLOPT_URL, $url); //set the url we want to use > + + $copy = curl_copy_handle($ch); > + curl_close($ch); > + > + $curl_content = curl_exec($copy); > + curl_close($copy); > + > + var_dump( $curl_content ); > +?> > +===DONE=== > +--EXPECTF-- > +*** Testing curl copy handle with simple POST using array as arguments *** > +string(163) "array(1) { > + ["test"]=> > + string(7) "getpost" > +} > +array(3) { > + ["Hello"]=> > + string(5) "World" > + ["Foo"]=> > + string(3) "Bar" > + ["Person"]=> > + string(8) "John Doe" > +} > +" > +===DONE=== > Index: ext/curl/php_curl.h > =================================================================== > --- ext/curl/php_curl.h (revision 284171) > +++ ext/curl/php_curl.h (working copy) > @@ -139,6 +139,7 @@ > long id; > unsigned int uses; > zend_bool in_callback; > + zval *clone; > } php_curl; > > typedef struct { > Index: NEWS > =================================================================== > --- NEWS (revision 284171) > +++ NEWS (working copy) > @@ -32,6 +32,8 @@ > (markril at hotmail dot com, Pierre) > - Fixed bug #38091 (Mail() does not use FQDN when sending SMTP helo). > (Kalle, Rick Yorgason) > +- Fixed bug #48774 (SIGSEGVs when using `curl_copy_handle()`). > + (Sriram Natarajan) > > 30 Jun 2009, PHP 5.3.0 > - Upgraded bundled PCRE to version 7.9. (Nuno) > > > thanks > sriram --- ## #45029 — Sriram Natarajan — 2009-07-18T16:03:15+00:00 URL: https://externals.io/message/45027#45029 · In reply to: #45028 Jani Taskinen wrote: > Sriram Natarajan wrote: > > […] > Fix your whitespace first. We use tabs only in .c files.. > Please find the updated patch below --- ## #45045 — Jani Taskinen — 2009-07-20T10:43:50+00:00 URL: https://externals.io/message/45027#45045 · In reply to: #45029 > Jani Taskinen wrote: > > […] > Please find the updated patch below As you seem to have your own svn account, I guess you can commit it yourself now? :) --Jani