Footnotes: checking type before using count()

[ramonjd]

What?

The PR attempts to guard against uncountable values in the footnotes block.

Reported in https://core.trac.wordpress.org/ticket/59103

Why?

I couldn't reproduce the error in PHP 7.4 or PHP 8.0, but decided it wouldn't hurt to first check if $footnotes was an array before we attempt to pass it through count().

I tested on WordPress 6.4-alpha and WordPress 6.3.

How?

Adding an is_array() check before using count() in case $footnotes is not countable.

Testing Instructions

If you're using wp-env, in order to test in PHP 8.0 add the following to .wp-env.json.

{
	"core": "WordPress/WordPress",
	"phpVersion": "8.0",
...
}

And run npm run wp-env start --update

Ensure that still footnotes work in posts/pages and in the site editor.

As you work, check the logs for warnings using npm run wp-env logs --watch.

[andrewserong]

I couldn't reproduce the error in PHP 7.4 or PHP 8.0, but decided it wouldn't hurt to first check if $footnotes was an array before we attempt to pass it through count().

I couldn't reproduce the issue with PHP 7.4 or PHP 8, either. I wonder what shape $footnotes was in that resulted in it being null in the reported issue? I guess somehow the result from get_post_meta would need to be truthy, but when passed through json_decode, it winds up being null? The PHP docs mention that null is returned if the JSON cannot be decoded, so I wonder if there's a condition somewhere that can happen where the post meta value for Footnotes is not valid JSON 🤔

In any case, the ! is_array() check looks like a good way to guard against this error for now 👍

[ramonjd]

I wonder what shape $footnotes was in that resulted in it being null in the reported issue? I guess somehow the result from get_post_meta would need to be truthy, but when passed through json_decode, it winds up being null? The PHP docs mention that null is returned if the JSON cannot be decoded, so I wonder if there's a condition somewhere that can happen where the post meta value for Footnotes is not valid JSON 🤔

Yeah, I feel like we're puttying over what could be a real bug. I also tried with special characters but no dice.

[tellthemachines]

I can't reproduce the issue either, but this change it doesn't seem to break anything.

[andrewserong]

Yeah, I feel like we're puttying over what could be a real bug. I also tried with special characters but no dice.

Yeah, totally. Still, this PR shouldn't hurt, and ideally if for some reason the Footnotes meta isn't valid JSON, I think silently returning nothing at render time is probably the best bet rather than throwing errors.

[ramonjd]

Okay, I can reproduce I think. It seems to only happen on autosave.

The footnotes are not being escaped. So we add a few " and ' it creates invalid JSON.

Here's what json_decode is trying to parse:

[{
	"content": "sdasdfas!!@#$%^&*()": ; ? > & lt;,
	. / {}
	"''/']{></{}dfasdf",
	"id" : "b06691e4-f584-4c51-9a4b-a49c87a6abb8"
}, {
	"content": ""
	sdfsdf 'd '
	a "",
	"id": "cc13626b-a993-49e7-a092-9581ecbdb0b0"
}, {
	"content": "adsfdsfasdf",
	"id": "5b4d0fe6-f2bb-4051-b398-f671a908e995"
}]

[andrewserong]

It seems to only happen on autosave.
The footnotes are not being escaped. So we I add a few " and ' it creates invalid JSON.

Great catch, that's consistent for me, too, and only on auto-save.

[ramonjd]

Closing this as it doesn't address the core issue. This PR does 🤞🏻

• Footnotes: autosave is not slashing JSON #53664

[ellatrix]

This is probably still valuable.

[mcsf]

Yes, let's definitely prevent count from throwing.

[ramonjd]

Okay! I'll restore 😄 Thanks, folks