# [RFC] [Vote] var_info - Thread: https://externals.io/message/94434 - Original archive: https://news-web.php.net/php.internals/94434 - Started by Fleshgrinder on 2016-07-08T19:49:05+00:00 - Messages: 7 - RFC: var_info() (Declined) — https://externals.io/rfc/var_info.md - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · RFCs at https://externals.io/rfc.md · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #94434 — Fleshgrinder — 2016-07-08T19:49:05+00:00 URL: https://externals.io/message/94434#94434 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 --- ## #94436 — Nikita Popov — 2016-07-08T21:54:29+00:00 URL: https://externals.io/message/94434#94436 · In reply to: #94434 > Hi! > > Voting on var_info has opened and will stay open until 2016-07-22: > > https://wiki.php.net/rfc/var_info > 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 --- ## #94437 — Stanislav Malyshev — 2016-07-08T22:19:58+00:00 URL: https://externals.io/message/94434#94437 · In reply to: #94436 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 --- ## #94462 — Stanislav Malyshev — 2016-07-11T00:01:20+00:00 URL: https://externals.io/message/94434#94462 · In reply to: #94437 Hi! > > […] > > 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 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 --- ## #94618 — Fleshgrinder — 2016-07-21T17:21:54+00:00 URL: https://externals.io/message/94434#94618 · In reply to: #94436 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 --- ## #94640 — Nikita Popov — 2016-07-22T20:21:19+00:00 URL: https://externals.io/message/94434#94640 · In reply to: #94437 On Sat, Jul 9, 2016 at 12:19 AM, Stanislav Malyshev wrote: > Hi! > > > […] > actually > > […] > > 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. > > > […] > > 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 > > […] > actually > > […] > need > > […] > > 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 --- ## #95212 — Nikita Popov — 2016-08-15T21:17:02+00:00 URL: https://externals.io/message/94434#95212 · In reply to: #94640 > On Sat, Jul 9, 2016 at 12:19 AM, Stanislav Malyshev > wrote: > > > […] > > 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 > 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