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

Will Norris will at willnorris.com
Tue Feb 7 02:28:43 UTC 2012 

yeah sorry, meant to mention that.  #68 only has one *real* change... AND
it's a subset of #44.  I have no doubt that at least SOME of the changes in
#44 are necessary, but it's not clear to me if ALL of them are.  That's
what I would love to settle on.

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

> #68 is a subset of #44
>
>
> On Mon, Feb 6, 2012 at 8:29 PM, Will Norris <will at willnorris.com> wrote:
>
>> +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/a65c5033/attachment-0001.html>

More information about the Code mailing list