Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:28185
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain zend.com designates 212.25.124.162 as permitted sender)
Message-ID: <45E5F067.3020709@zend.com>
Date: Thu, 01 Mar 2007 00:13:11 +0300
User-Agent: Thunderbird 2.0b2 (X11/20070116)
MIME-Version: 1.0
To:  cardoe@gentoo.org
CC:  internals@lists.php.net
References: <40059.216.155.111.10.1172694790.squirrel@webmail.cardoe.com>    <45E5EA50.3070800@zend.com> <33449.216.155.111.10.1172696679.squirrel@webmail.cardoe.com>
In-Reply-To: <33449.216.155.111.10.1172696679.squirrel@webmail.cardoe.com>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] LDAP functions implemented poorly
From: antony@zend.com (Antony Dovgal)

On 03/01/2007 12:04 AM, Doug Goldstein wrote:
>> Wait, I thought the DEPRECATED thing fixed it (I can't test it myself as I
>> don't use LDAP).
>> If not, then what was it all about?
> 
> The test case still failed for me. It also does not address the fact that
> the usage of the PHP ldap functions is unsafe because they could return a
> char * with a non-NULL terminated string and your code will eventually
> call strlen() on it. With the changes I have made it knows the length from
> the LDAP server and doesn't need to use strlen(). Since
> add_index_stringl() doesn't call strlen() since the length is provided.

Did you really test it with non-NULL terminated strings?
Don't you need to add '\0' manually?

>> Some comments regarding the patch:
>> First of all, why do you add a new function and comment the old one
>> instead of
>> changing/fixing the old function?
>> We definitely don't need dead/commented out code in the sources =)
> 
> I didn't add a new function. I replaced a function with an alias since the
> function is pointlessly there. It's duplicated code and the net result is
> just an alias.

No, replace means "add something and delete the original", you added new function and 
commented out the original one, which made me wonder why you didn't change the original function instead.
What was the point in creating new function adding an alias?
There is nothing wrong, I'm just curious.

>> Then, it's a bit too late to change 5.1.6, when 5.2.2 is on it's way, so
>> surely
>> we would like to see the patches for 5.2.2 instead.
> 
> This patch is against 5.1.6 and the 5.2 branch. The code in question has
> not been touched since before 5.1.0 was released. I tested it against both
> 5.2 branch and 5.1.6 prior to uploading it.

Ok then.

>> You can get its sources from CVS using PHP_5_2 branch:
>> cvs -d :pserver:anonymous@cvs.php.net/repository co -r PHP_5_2 php-src
> 
> You might want to update your instructions to people since the user is
> "cvsread" and not "anonymous".

Oh, that's just my memory.

-- 
Wbr, 
Antony Dovgal