Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:45028
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain sci.fi from 204.13.248.71 cause and error)
Message-ID: <4A6189AF.6040001@sci.fi>
Date: Sat, 18 Jul 2009 11:37:03 +0300
Reply-To: jani.taskinen@iki.fi
User-Agent: Thunderbird 2.0.0.22 (Windows/20090605)
MIME-Version: 1.0
To: Sriram Natarajan <sriram.natarajan@gmail.com>
CC: PHP internals <internals@lists.php.net>
References: <4A616EFE.7090107@GMail.COM>
In-Reply-To: <4A616EFE.7090107@GMail.COM>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] PATCH for bug 48774
From: jani.taskinen@sci.fi (Jani Taskinen)

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
>