Inconsistent Variable Usage

[ericmann]

Someone in the forums alerted me to a potential issue with the code in wp_version_check(). It seems we're using two different functions to get a user count depending on whether we're in Multisite or a standalone installation.

The Multisite function (get_user_count()) returns a number. The standalone function (count_users()) returns an array. We then assume the array when we use the variable later to build the version check query.

In some situations, this can result in an "illegal string offset" error.

In other situations, it will generate inaccurate data. Assume the following situation:

• get_user_count() returns "100" because there are 100 users.
• $user_count['total_users'] will return "1"

When you reference a string as an array with an offset that doesn't exist, it only returns the first digit. So our count, in this case, is only accurate for Multisite installations with 1-9 users.

Not a huge end-user issue ... but confusing for WP.org stats in the long run if it remains unpatched.

I have worked out a patch that will fix the stat reporting issue for us and silence any "illegal string offset" issues other developers might be experiencing.

[ericmann]

Discussion/questions about this originated:

• In the forums(​http://wordpress.org/support/topic/illegal-string-offset-total_users?replies=17)
• On the WP Stack Exchange (​http://wordpress.stackexchange.com/questions/55175/when-does-output-of-get-user-count-turns-into-an-array)
• And I've seen at least one conversation about it on WP Hackers (​http://lists.automattic.com/pipermail/wp-hackers/2012-June/043295.html)

[SergeyBiryukov]

Introduced in [16604]. I don't see the "Illegal string offset" warning on PHP 5.2.14, but can reproduce the inaccurate data (after calling wp_update_network_counts() to make sure the count is updated).

[cogmios]

I have php 5.4.4 see the warning.
I assume it has something todo with wp_cache_get where site-options does not contain the user-count yet since it reads then the value from the meta_value. Maybe the php doc from get_user_count could also be updated since the return value should NOT be an integer if i follow correctly.

[nacin]

I am interested to see how "illegal string offset" can occur. Also, is it a notice, or a warning? Big difference as to whether we plan to include a fix for this in 3.4.1. Otherwise, we don't currently even aggregate the user stats, so I have no qualms about waiting until 3.5 for this.

[cogmios]

it's a warning

It happens on ​http://(url)/wp-admin/network/update-core.php where the version check takes place or every site admin page if the check hasnt runt.

[ericmann]

Replying to nacin:

Big difference as to whether we plan to include a fix for this in 3.4.1. Otherwise, we don't currently even aggregate the user stats, so I have no qualms about waiting until 3.5 for this.

Since the code's been in there since 3.1 and the issue is just now coming up (and only for a handful of users) I don't see this as a critical issue. Particularly if you can work around it by lowering your PHP warning settings. I'm leaning towards waiting until 3.5.

[cogmios]

I found out... that if I run in a PHP version < 5.4

$expected_array_got_string = 'somestring';
echo $expected_array_got_string['some_key'];

that it returns "s"

However starting from PHP version 5.4. it will throw: Warning: Illegal string offset 'some_key'

:)

I think that's the reason for the low amount of users seeing it...

(see also (related) example 2 here: ​http://php.net/manual/en/function.empty.php )

So... to see the error... simply install php 5.4 or later and hit the check updates page a few times to get it NOT from the cache)

[ericmann]

I still definitely see this as an important fix moving forward, but since it only seems to affect those running PHP >= 5.4, I wanted to do a quick sanity check to see just how many users this bug effects.

According to ​http://wordpress.org/about/stats/, only 0.2% of self-hosted WordPress installs are running PHP 5.4. About 98% of installs are running either PHP 5.3 or 5.2, which explains why we haven't noticed this issue before.

But, moving forward, I do think this will become more of an issue.

[nacin]

In [21105]:

Fix string offset PHP 5.4 error by normalizing $user_count to always be an integer. props ericmann. see #20966 for trunk.

[nacin]

In [21121]:

Fix string offset PHP 5.4 error by normalizing to always be an integer. props ericmann. fixes #20966 for 3.4