Hi!
Voting on var_info has opened and will stay open until 2016-07-22:
https://wiki.php.net/rfc/var_info
--
Richard "Fleshgrinder" Fussenegger
Hi!
Voting on var_info has opened and will stay open until 2016-07-22:
Not a big fan of this in general (imho belongs in userland), but I'd like
to comment on one specific issue:
You are doing a callability check for all arrays and strings. I wouldn't be
comfortable passing any data that potentially derives from external sources
(aka all data) to that function. E.g. this could trigger autoloading on
user-provided values -- while we have some protections in place to avoid
the most egregious security vulnerabilities, I wouldn't be comfortable with
that. There probably other funky things that can happen here.
Lastly, I'll just leave this list of type-related improvements I'd actually
like to see:
- Make
is_object()return true for IncompleteClass. This is just
ridiculous. I care zero about the theoretical BC impact. - Make
gettype()return 'resource (unknown type)' instead of just
'unknown type' for closed resources. This makes the output clear while
still leaving the closed resource distinction. - Normalize error message. IIRC currently our type error messages are
really weird, in that they use terms like "integer" which are not actually
valid type hints (or have different meaning). Fixing this doesn't even need
a proposal, just a PR.
Regards,
Nikita
Hi!
Lastly, I'll just leave this list of type-related improvements I'd actually
like to see:
- Make
is_object()return true for IncompleteClass. This is just
ridiculous. I care zero about the theoretical BC impact.
I have no idea why it doesn't do so. I'll try to dig up when it was made
so and see if there's some explanation.
- Make
gettype()return 'resource (unknown type)' instead of just
'unknown type' for closed resources. This makes the output clear while
still leaving the closed resource distinction.
That makes total sense. And BC impact is rather minimal given that
closed resources are useless.
- Normalize error message. IIRC currently our type error messages are
really weird, in that they use terms like "integer" which are not actually
valid type hints (or have different meaning). Fixing this doesn't even need
a proposal, just a PR.
Right. But ensure tests are fixed, that's probably where one has to
spend some time on cleaning it up.
--
Stas Malyshev
smalyshev@gmail.com
Hi!
- Make
is_object()return true for IncompleteClass. This is just
ridiculous. I care zero about the theoretical BC impact.I have no idea why it doesn't do so. I'll try to dig up when it was made
so and see if there's some explanation.
This is the change that caused it:
commit 7b3fb771e4b022a8bf74b1a881dbec67d3ad460b
Author: Yasuo Ohgaki yohgaki@php.net
Date: Wed Jul 24 09:55:11 2002 +0000
`is_object()` returns `FALSE` if object is a "incomplete object".
Raise E_NOTICE, instead of E_ERROR, for setting/getting properties
to/from a "incomplete object".
There's some discussion on this patch:
http://grokbase.com/t/php/php-dev/027rwwmc8k/php-cvs-cvs-php4-ext-standard-incomplete-class-c-php-incomplete-class-h-type-c
There are references to previous discussions but I couldn't find it.
Yasuo, I realize it was a long time ago, but maybe you remember why it
was done and can weight in on this?
--
Stas Malyshev
smalyshev@gmail.com
On Sat, Jul 9, 2016 at 12:19 AM, Stanislav Malyshev smalyshev@gmail.com
wrote:
Hi!
Lastly, I'll just leave this list of type-related improvements I'd
actually
like to see:
- Make
is_object()return true for IncompleteClass. This is just
ridiculous. I care zero about the theoretical BC impact.I have no idea why it doesn't do so. I'll try to dig up when it was made
so and see if there's some explanation.
- Make
gettype()return 'resource (unknown type)' instead of just
'unknown type' for closed resources. This makes the output clear while
still leaving the closed resource distinction.That makes total sense. And BC impact is rather minimal given that
closed resources are useless.
I've created PRs for both of these:
gettype() on closed resources: https://github.com/php/php-src/pull/2022
is_object() on Incomplete_Class: https://github.com/php/php-src/pull/2023
Unless there are objections from core devs I'll apply both in a few days.
Nikita
- Normalize error message. IIRC currently our type error messages are
really weird, in that they use terms like "integer" which are not
actually
valid type hints (or have different meaning). Fixing this doesn't even
need
a proposal, just a PR.Right. But ensure tests are fixed, that's probably where one has to
spend some time on cleaning it up.--
Stas Malyshev
smalyshev@gmail.com
On Sat, Jul 9, 2016 at 12:19 AM, Stanislav Malyshev smalyshev@gmail.com
wrote:Hi!
Lastly, I'll just leave this list of type-related improvements I'd
actually
like to see:
- Make
is_object()return true for IncompleteClass. This is just
ridiculous. I care zero about the theoretical BC impact.I have no idea why it doesn't do so. I'll try to dig up when it was made
so and see if there's some explanation.
- Make
gettype()return 'resource (unknown type)' instead of just
'unknown type' for closed resources. This makes the output clear while
still leaving the closed resource distinction.That makes total sense. And BC impact is rather minimal given that
closed resources are useless.I've created PRs for both of these:
gettype()on closed resources: https://github.com/php/php-src/pull/2022
is_object()on Incomplete_Class: https://github.com/php/php-src/pull/2023Unless there are objections from core devs I'll apply both in a few days.
Nikita
As there were no objections, I've now merged both changes:
https://github.com/php/php-src/commit/34824b70f8e72b200d77957145bb61883b03322d
https://github.com/php/php-src/commit/89f63779711f34d87c2464ae13965df868d6231f
As I forgot to handle this earlier, I went with master-only.
Nikita
Many thanks for the feedback to both of you. :)
You are doing a callability check for all arrays and strings. I wouldn't be
comfortable passing any data that potentially derives from external sources
(aka all data) to that function. E.g. this could trigger autoloading on
user-provided values -- while we have some protections in place to avoid
the most egregious security vulnerabilities, I wouldn't be comfortable with
that. There probably other funky things that can happen here.
That is true for anything that uses is_callable anywhere.
Lastly, I'll just leave this list of type-related improvements I'd actually
like to see:
- Make
is_object()return true for IncompleteClass. This is just
ridiculous. I care zero about the theoretical BC impact.
I figured that this situation is on purpose and I don't think that it is
ridiculous per se. It makes sense if is_object() is defined as only
identifying valid objects (and incomplete class is by definition invalid).
- Make
gettype()return 'resource (unknown type)' instead of just
'unknown type' for closed resources. This makes the output clear while
still leaving the closed resource distinction.
Will prepare a PR. 8)
- Normalize error message. IIRC currently our type error messages are
really weird, in that they use terms like "integer" which are not actually
valid type hints (or have different meaning). Fixing this doesn't even need
a proposal, just a PR.
Definitely on my roadmap anyways and yes stas, this requires going
through a sh**load of existing tests but it is definitely worth it.
I will most probably start work on all of this this weekend after being
on vacation for the last days.
--
Richard "Fleshgrinder" Fussenegger