[Code] php-openid: Bugs and new version?

Tue Feb 7 01:29:43 UTC 2012

+openid-code list

we currently have two pull requests in the queue pertaining to fixing
references in PHP5:

https://github.com/openid/php-openid/pull/44
https://github.com/openid/php-openid/pull/68

#68 only has one actual substantive change, plus a lot of whitespace
changes.  To folks on the code list: has anyone actually sat down and
verified that the changes in #44 are all correct (see my comment earlier in
this thread below)?  It may be totally fine, I just haven't had time to
trace through each individual change.

Cash: you mention that one of them is definitely fine.  Could you look
through the others in #44 as well to verify, and reply on the pull request.

-will

On Mon, Feb 6, 2012 at 4:07 PM, Cash Costello <cash.costello at gmail.com>wrote:

> Interestingly enough, the MediaWiki OpenID plugin distributes a patch for
> this issue (see note under requisite 1):
> http://www.mediawiki.org/wiki/Extension:OpenID
>
>
> On Mon, Feb 6, 2012 at 7:05 PM, Cash Costello <cash.costello at gmail.com>wrote:
>
>> These look like cases of code not being updated for PHP 5.
>>
>> Take Auth_OpenID_GenericConsumer::complete() as an example:
>>
>>     function complete($message, $endpoint, $return_to)
>>
>>
>>     {
>>         $mode = $message->getArg(Auth_OpenID_OPENID_NS, 'mode',
>>
>>
>>                                  '<no mode set>');
>>
>>         $mode_methods = array(
>>                               'cancel' => '_complete_cancel',
>>
>>                               'error' => '_complete_error',
>>                               'setup_needed' => '_complete_setup_needed',
>>
>>
>>                               'id_res' => '_complete_id_res',
>>
>>                               );
>>
>>         $method = Auth_OpenID::arrayGet($mode_methods, $mode,
>>
>>
>>                                         '_completeInvalid');
>>
>>         return call_user_func_array(array($this, $method),
>>
>>
>>                                     array($message, &$endpoint, $return_to));
>>     }
>>
>>
>> $endpoint is an object so it passed by reference to the complete()
>> method. It will also be passed by reference to whatever method
>> call_user_func_array() is calling. It does not need to be explicit in PHP 5.
>>
>> Cash
>>
>>
>> On Mon, Feb 6, 2012 at 6:22 PM, Jan Hauke Rahm <jhr at debian.org> wrote:
>>
>>> On Mon, Feb 06, 2012 at 02:58:35PM -0800, Will Norris wrote:
>>> > +Cash, who was looking at merging in some of the existing pending pull
>>> > requests.
>>>
>>> Cool.
>>>
>>> > One of the reasons I haven't merged in the reference pull requests is
>>> that I
>>> > think they did exactly what you're describing... wholesale dropping
>>> all '&',
>>> > which I don't think is the right thing.  References are still
>>> supported in
>>> > PHP5, just not in the same way as PHP4.  While simply dropping all
>>> references
>>> > does of course get rid of the warnings and errors from more recent
>>> versions of
>>> > PHP, it also potentially has significant changes in what the library
>>> is doing.
>>>
>>> Well, as I understand the actual issue here: call-by-reference is not
>>> really going away but the syntax changed. A function can be declared to
>>> take references but it (formerlly should, now) must not be called with
>>> references. That means, the behavior won't change unless the function
>>> was declared wrong.
>>>
>>> The issues here are regarding call_user_func and call_user_func_array
>>> which are defined to take multiple arguments. [1] Wether they internally
>>> make references or not, I don't know. According to [1], one can pass the
>>> arguments without problem and prior to php 5.3 the referencing syntax
>>> was also valid.
>>>
>>> [1] http://de.php.net/call_user_func
>>>
>>> > I simply haven't had the time to sit down and work through the code to
>>> see if
>>> > switching from passing references to passing values is okay (not sure
>>> if the
>>> > methods are actually modifying the objects in any way that the
>>> external caller
>>> > is expecting to see the result of... and there's a lot of indirection
>>> in there
>>> > to trace through to make sure).  I'm also not convinced that person who
>>> > submitted the pull request has actually gone through this exercise
>>> either, and
>>> > unfortunately, we don't have good working unit tests to verify that
>>> the changes
>>> > don't break things... but that's a whole 'nother problem.  If someone
>>> is
>>> > willing to actually go through and verify that it *really* is doing
>>> the right
>>> > thing, I'd be happy to merge the request.
>>>
>>> Well, I'm afraid I'll have to test php-openid against php 5.4 a bit
>>> anyways. Be sure, if I encounter any issues, I'll contact you, with
>>> patches if possible.
>>>
>>> Thanks much for your fast response!
>>>
>>> Hauke
>>>
>>> > On Mon, Feb 6, 2012 at 2:46 PM, Jan Hauke Rahm <jhr at debian.org> wrote:
>>> >
>>> >     Hi Will,
>>> >
>>> >     I'm the Debian developer maintaining php-openid in Debian. We're
>>> >     currently planning to transition to php 5.4 for Debian's next
>>> release
>>> >     and this will break php-openid as it currently is.
>>> >
>>> >     The Debian bugs (duplicates as it seems) [1] refer to the
>>> >     call-by-reference stuff that is going away in 5.4 as well as the
>>> dl()
>>> >     stuff. The latter seems to be resolved by your recent merge on
>>> github,
>>> >     the former is still open.
>>> >
>>> >     I'm curious, are you going to fix the call-by-reference stuff
>>> soon? I
>>> >     could provide a patch, though it's realls just a s/&// in a
>>> handful of
>>> >     cases if I'm not too wrong. And, incidently, are you going to
>>> release a
>>> >     new version of php-openid? A 2.2.3 or something?
>>> >
>>> >     I'd appreciate a short answer just so I can plan ahead. I'd hate
>>> to have
>>> >     Debian differ from your code too much. Thanks anyways for keeping
>>> the
>>> >     code alive!
>>> >
>>> >     Hauke
>>> >
>>> >     [1] http://bugs.debian.org/php-openid
>>> >
>>> >     --
>>> >      .''`.   Jan Hauke Rahm <jhr at debian.org>
>>> www.jhr-online.de
>>> >     : :'  :  Debian Developer
>>> www.debian.org
>>> >     `. `'`   Member of the Linux Foundation
>>> www.linux.com
>>> >      `-     Fellow of the Free Software Foundation Europe
>>> www.fsfe.org
>>> >
>>> >
>>>
>>> --
>>>  .''`.   Jan Hauke Rahm <jhr at debian.org>               www.jhr-online.de
>>> : :'  :  Debian Developer                                 www.debian.org
>>> `. `'`   Member of the Linux Foundation                    www.linux.com
>>>  `-     Fellow of the Free Software Foundation Europe      www.fsfe.org
>>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openid.net/pipermail/openid-code/attachments/20120206/55bbd851/attachment.html>