Re: PATCH for bug 48774

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-- +<?php if (!extension_loaded("curl") || false === getenv('PHP_CURL_HTTP_REMOTE_SERVER')) print "skip"; ?> +--FILE-- +<?php + $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