Req #61759 [Com]: class_alias() should accept classes with leading backslashes

2013-08-27 Thread jpa...@php.net
Edit report at https://bugs.php.net/bug.php?id=61759edit=1

 ID: 61759
 Comment by: jpa...@php.net
 Reported by:ahar...@php.net
 Summary:class_alias() should accept classes with leading
 backslashes
 Status: Open
 Type:   Feature/Change Request
 Package:Class/Object related
 Operating System:   Irrelevant
 PHP Version:master-Git-2012-04-18 (Git)
 Block user comment: N
 Private report: N

 New Comment:

Johannes: I agree, but we could start by patching this bug report right?
I got a patch here : https://github.com/jpauli/php-
src/compare/class_alias_registration_fix


Previous Comments:

[2013-08-26 18:32:26] johan...@php.net

Note: The bug report is too restrictive. A proper patch would have to work on 
all places where classnames come from string context. This at first means 
verifying that all places go via zend_lookup_class() and related functions, not 
EG(class_table) / CG(class_table)


[2013-08-26 18:13:08] contact at jubianchi dot fr

I experienced the exact same issue on PHP 5.4.17 on OS X 10.9 (Mavericks DP6).
I wrote a simple test case, here it is :

Test script:
---
namespace jubianchi\Alias {
class A {}
 
var_dump(class_alias('\\jubianchi\\Alias\\A', 'C'));
$reflector = new \ReflectionClass('C');
var_dump($reflector-getName());  

var_dump(class_alias('\\jubianchi\\Alias\\A', '\\jubianchi\\Alias\\B'));
try {
$reflector = new \ReflectionClass('\\jubianchi\\Alias\\B');
var_dump($reflector-getName());
} catch(\Exception $e) {
var_dump(get_class($e) . ': ' . $e-getMessage());
}

var_dump(class_alias('\\jubianchi\\Alias\\A', 'jubianchi\\Alias\\B'));
$reflector = new \ReflectionClass('\\jubianchi\\Alias\\B');
var_dump($reflector-getName());
}

Expected result:

bool(true)
string(17) jubianchi\Alias\A
bool(true)
string(17) jubianchi\Alias\A
bool(true)
string(17) jubianchi\Alias\A

Or:

bool(true)
string(17) jubianchi\Alias\A
bool(false)
string(60) ReflectionException: Class \jubianchi\Alias\B does not exist
bool(true)
string(17) jubianchi\Alias\A

Actual result:

bool(true)
string(1) A
bool(true)
string(17) jubianchi\Alias\A
bool(true)
string(60) ReflectionException: Class \jubianchi\Alias\B does not exist
bool(true)
string(17) jubianchi\Alias\A

As you can see, class_alias returns bool(true) as if everything went fine, so 
we 
expect the alias to be available but a reflection on the latter throws an 
exception.
I think class_alias should be able to handle the leading backslashes or return 
bool(false) if it can't.


[2012-04-18 06:11:21] ahar...@php.net

Description:

Aliasing namespaced classes currently expects that class names will be given in 
the same form as the ZE uses internally; ie without a leading backslash. Since 
that's inconsistent with the absolute form in PHP, it would be good if 
class_alias() could also ignore a leading backslash.

Test script:
---
?php
namespace A;
class C { function foo() { echo 42\n; } }

namespace B;
class_alias('\A\C', '\B\C');
$c = new C;
$c-foo();

Expected result:

42

Actual result:
--
Fatal error: Class 'B\C' not found in /private/tmp/test.php on line 7






-- 
Edit this bug report at https://bugs.php.net/bug.php?id=61759edit=1


Req #61759 [Com]: class_alias() should accept classes with leading backslashes

2013-08-27 Thread contact at jubianchi dot fr
Edit report at https://bugs.php.net/bug.php?id=61759edit=1

 ID: 61759
 Comment by: contact at jubianchi dot fr
 Reported by:ahar...@php.net
 Summary:class_alias() should accept classes with leading
 backslashes
 Status: Open
 Type:   Feature/Change Request
 Package:Class/Object related
 Operating System:   Irrelevant
 PHP Version:master-Git-2012-04-18 (Git)
 Block user comment: N
 Private report: N

 New Comment:

I agree with Johannes about consistency.

The severity is not really is not very high and this use case can easily be 
handled at a useland level.

As long as this behavior is not fixed I think a warning on the doc shoudl be 
enough, even if I'd like to see it fixed (but as I said, it's not a big deal at 
the moment).

BTW, thanks for you work Julien :)


Previous Comments:

[2013-08-27 10:08:00] johan...@php.net

Technically we could, but it adds some inconsistency if one place allows this 
but others not and that should be avoided.


[2013-08-27 09:46:53] jpa...@php.net

The following patch has been added/updated:

Patch Name: fix-class_alias
Revision:   1377596813
URL:
https://bugs.php.net/patch-display.php?bug=61759patch=fix-class_aliasrevision=1377596813


[2013-08-27 09:45:12] jpa...@php.net

Johannes: I agree, but we could start by patching this bug report right?
I got a patch here : https://github.com/jpauli/php-
src/compare/class_alias_registration_fix


[2013-08-26 18:32:26] johan...@php.net

Note: The bug report is too restrictive. A proper patch would have to work on 
all places where classnames come from string context. This at first means 
verifying that all places go via zend_lookup_class() and related functions, not 
EG(class_table) / CG(class_table)


[2013-08-26 18:13:08] contact at jubianchi dot fr

I experienced the exact same issue on PHP 5.4.17 on OS X 10.9 (Mavericks DP6).
I wrote a simple test case, here it is :

Test script:
---
namespace jubianchi\Alias {
class A {}
 
var_dump(class_alias('\\jubianchi\\Alias\\A', 'C'));
$reflector = new \ReflectionClass('C');
var_dump($reflector-getName());  

var_dump(class_alias('\\jubianchi\\Alias\\A', '\\jubianchi\\Alias\\B'));
try {
$reflector = new \ReflectionClass('\\jubianchi\\Alias\\B');
var_dump($reflector-getName());
} catch(\Exception $e) {
var_dump(get_class($e) . ': ' . $e-getMessage());
}

var_dump(class_alias('\\jubianchi\\Alias\\A', 'jubianchi\\Alias\\B'));
$reflector = new \ReflectionClass('\\jubianchi\\Alias\\B');
var_dump($reflector-getName());
}

Expected result:

bool(true)
string(17) jubianchi\Alias\A
bool(true)
string(17) jubianchi\Alias\A
bool(true)
string(17) jubianchi\Alias\A

Or:

bool(true)
string(17) jubianchi\Alias\A
bool(false)
string(60) ReflectionException: Class \jubianchi\Alias\B does not exist
bool(true)
string(17) jubianchi\Alias\A

Actual result:

bool(true)
string(1) A
bool(true)
string(17) jubianchi\Alias\A
bool(true)
string(60) ReflectionException: Class \jubianchi\Alias\B does not exist
bool(true)
string(17) jubianchi\Alias\A

As you can see, class_alias returns bool(true) as if everything went fine, so 
we 
expect the alias to be available but a reflection on the latter throws an 
exception.
I think class_alias should be able to handle the leading backslashes or return 
bool(false) if it can't.




The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at

https://bugs.php.net/bug.php?id=61759


-- 
Edit this bug report at https://bugs.php.net/bug.php?id=61759edit=1


Req #61759 [Com]: class_alias() should accept classes with leading backslashes

2013-08-27 Thread jpa...@php.net
Edit report at https://bugs.php.net/bug.php?id=61759edit=1

 ID: 61759
 Comment by: jpa...@php.net
 Reported by:ahar...@php.net
 Summary:class_alias() should accept classes with leading
 backslashes
 Status: Open
 Type:   Feature/Change Request
 Package:Class/Object related
 Operating System:   Irrelevant
 PHP Version:master-Git-2012-04-18 (Git)
 Block user comment: N
 Private report: N

 New Comment:

Yep, let's start finding all places where classes as strings can be used, and 
patch them all to use zend_lookup_class().
There shouldn't be tons of them AFAIR.


Previous Comments:

[2013-08-27 10:19:53] contact at jubianchi dot fr

I agree with Johannes about consistency.

The severity is not really is not very high and this use case can easily be 
handled at a useland level.

As long as this behavior is not fixed I think a warning on the doc shoudl be 
enough, even if I'd like to see it fixed (but as I said, it's not a big deal at 
the moment).

BTW, thanks for you work Julien :)


[2013-08-27 10:08:00] johan...@php.net

Technically we could, but it adds some inconsistency if one place allows this 
but others not and that should be avoided.


[2013-08-27 09:46:53] jpa...@php.net

The following patch has been added/updated:

Patch Name: fix-class_alias
Revision:   1377596813
URL:
https://bugs.php.net/patch-display.php?bug=61759patch=fix-class_aliasrevision=1377596813


[2013-08-27 09:45:12] jpa...@php.net

Johannes: I agree, but we could start by patching this bug report right?
I got a patch here : https://github.com/jpauli/php-
src/compare/class_alias_registration_fix


[2013-08-26 18:32:26] johan...@php.net

Note: The bug report is too restrictive. A proper patch would have to work on 
all places where classnames come from string context. This at first means 
verifying that all places go via zend_lookup_class() and related functions, not 
EG(class_table) / CG(class_table)




The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at

https://bugs.php.net/bug.php?id=61759


-- 
Edit this bug report at https://bugs.php.net/bug.php?id=61759edit=1


Req #61759 [Com]: class_alias() should accept classes with leading backslashes

2013-08-27 Thread ni...@php.net
Edit report at https://bugs.php.net/bug.php?id=61759edit=1

 ID: 61759
 Comment by: ni...@php.net
 Reported by:ahar...@php.net
 Summary:class_alias() should accept classes with leading
 backslashes
 Status: Open
 Type:   Feature/Change Request
 Package:Class/Object related
 Operating System:   Irrelevant
 PHP Version:master-Git-2012-04-18 (Git)
 Block user comment: N
 Private report: N

 New Comment:

I'm not convinced that allowing a leading \ is something we should strive 
towards. The \ is unnecessary and redundant (as string names are always fully 
qualified). I'd rather allow only the canonical form.


Previous Comments:

[2013-08-27 12:04:43] jpa...@php.net

Yep, let's start finding all places where classes as strings can be used, and 
patch them all to use zend_lookup_class().
There shouldn't be tons of them AFAIR.


[2013-08-27 10:19:53] contact at jubianchi dot fr

I agree with Johannes about consistency.

The severity is not really is not very high and this use case can easily be 
handled at a useland level.

As long as this behavior is not fixed I think a warning on the doc shoudl be 
enough, even if I'd like to see it fixed (but as I said, it's not a big deal at 
the moment).

BTW, thanks for you work Julien :)


[2013-08-27 10:08:00] johan...@php.net

Technically we could, but it adds some inconsistency if one place allows this 
but others not and that should be avoided.


[2013-08-27 09:46:53] jpa...@php.net

The following patch has been added/updated:

Patch Name: fix-class_alias
Revision:   1377596813
URL:
https://bugs.php.net/patch-display.php?bug=61759patch=fix-class_aliasrevision=1377596813


[2013-08-27 09:45:12] jpa...@php.net

Johannes: I agree, but we could start by patching this bug report right?
I got a patch here : https://github.com/jpauli/php-
src/compare/class_alias_registration_fix




The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at

https://bugs.php.net/bug.php?id=61759


-- 
Edit this bug report at https://bugs.php.net/bug.php?id=61759edit=1


Req #61759 [Com]: class_alias() should accept classes with leading backslashes

2013-08-27 Thread contact at jubianchi dot fr
Edit report at https://bugs.php.net/bug.php?id=61759edit=1

 ID: 61759
 Comment by: contact at jubianchi dot fr
 Reported by:ahar...@php.net
 Summary:class_alias() should accept classes with leading
 backslashes
 Status: Open
 Type:   Feature/Change Request
 Package:Class/Object related
 Operating System:   Irrelevant
 PHP Version:master-Git-2012-04-18 (Git)
 Block user comment: N
 Private report: N

 New Comment:

Also agree with the fact that the leading backslashes are redundant but the 
point 
is that class_alias returns a value saying all went fine (bool(true)) when the 
alias is not reachable after the call.


Previous Comments:

[2013-08-27 12:10:10] ni...@php.net

I'm not convinced that allowing a leading \ is something we should strive 
towards. The \ is unnecessary and redundant (as string names are always fully 
qualified). I'd rather allow only the canonical form.


[2013-08-27 12:04:43] jpa...@php.net

Yep, let's start finding all places where classes as strings can be used, and 
patch them all to use zend_lookup_class().
There shouldn't be tons of them AFAIR.


[2013-08-27 10:19:53] contact at jubianchi dot fr

I agree with Johannes about consistency.

The severity is not really is not very high and this use case can easily be 
handled at a useland level.

As long as this behavior is not fixed I think a warning on the doc shoudl be 
enough, even if I'd like to see it fixed (but as I said, it's not a big deal at 
the moment).

BTW, thanks for you work Julien :)


[2013-08-27 10:08:00] johan...@php.net

Technically we could, but it adds some inconsistency if one place allows this 
but others not and that should be avoided.


[2013-08-27 09:46:53] jpa...@php.net

The following patch has been added/updated:

Patch Name: fix-class_alias
Revision:   1377596813
URL:
https://bugs.php.net/patch-display.php?bug=61759patch=fix-class_aliasrevision=1377596813




The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at

https://bugs.php.net/bug.php?id=61759


-- 
Edit this bug report at https://bugs.php.net/bug.php?id=61759edit=1


Req #61759 [Com]: class_alias() should accept classes with leading backslashes

2013-08-26 Thread contact at jubianchi dot fr
Edit report at https://bugs.php.net/bug.php?id=61759edit=1

 ID: 61759
 Comment by: contact at jubianchi dot fr
 Reported by:ahar...@php.net
 Summary:class_alias() should accept classes with leading
 backslashes
 Status: Open
 Type:   Feature/Change Request
 Package:Class/Object related
 Operating System:   Irrelevant
 PHP Version:master-Git-2012-04-18 (Git)
 Block user comment: N
 Private report: N

 New Comment:

I experienced the exact same issue on PHP 5.4.17 on OS X 10.9 (Mavericks DP6).
I wrote a simple test case, here it is :

Test script:
---
namespace jubianchi\Alias {
class A {}
 
var_dump(class_alias('\\jubianchi\\Alias\\A', 'C'));
$reflector = new \ReflectionClass('C');
var_dump($reflector-getName());  

var_dump(class_alias('\\jubianchi\\Alias\\A', '\\jubianchi\\Alias\\B'));
try {
$reflector = new \ReflectionClass('\\jubianchi\\Alias\\B');
var_dump($reflector-getName());
} catch(\Exception $e) {
var_dump(get_class($e) . ': ' . $e-getMessage());
}

var_dump(class_alias('\\jubianchi\\Alias\\A', 'jubianchi\\Alias\\B'));
$reflector = new \ReflectionClass('\\jubianchi\\Alias\\B');
var_dump($reflector-getName());
}

Expected result:

bool(true)
string(17) jubianchi\Alias\A
bool(true)
string(17) jubianchi\Alias\A
bool(true)
string(17) jubianchi\Alias\A

Or:

bool(true)
string(17) jubianchi\Alias\A
bool(false)
string(60) ReflectionException: Class \jubianchi\Alias\B does not exist
bool(true)
string(17) jubianchi\Alias\A

Actual result:

bool(true)
string(1) A
bool(true)
string(17) jubianchi\Alias\A
bool(true)
string(60) ReflectionException: Class \jubianchi\Alias\B does not exist
bool(true)
string(17) jubianchi\Alias\A

As you can see, class_alias returns bool(true) as if everything went fine, so 
we 
expect the alias to be available but a reflection on the latter throws an 
exception.
I think class_alias should be able to handle the leading backslashes or return 
bool(false) if it can't.


Previous Comments:

[2012-04-18 06:11:21] ahar...@php.net

Description:

Aliasing namespaced classes currently expects that class names will be given in 
the same form as the ZE uses internally; ie without a leading backslash. Since 
that's inconsistent with the absolute form in PHP, it would be good if 
class_alias() could also ignore a leading backslash.

Test script:
---
?php
namespace A;
class C { function foo() { echo 42\n; } }

namespace B;
class_alias('\A\C', '\B\C');
$c = new C;
$c-foo();

Expected result:

42

Actual result:
--
Fatal error: Class 'B\C' not found in /private/tmp/test.php on line 7






-- 
Edit this bug report at https://bugs.php.net/bug.php?id=61759edit=1