Return of _wp_json_prepare_data() in wp_json_encode() should be used.

[jrf]

I was looking at the wp_json_encode() function for unrelated reasons and noticed a code oddity where the return of a function call to _wp_json_prepare_data() which was intentionally introduced was being disregarded.

The code was introduced in [34926] by @rmccue.

I'm including three different patches which each solve the issue in a slightly different way.

• Patch 1 presumes that the call should only influence the $data for the first json_encode() call.

• Patch 2 presumes that the call should influence the $data for both json_encode() calls.

• Patch 3 presumes that the call should only influence the $data for the potential second json_encode() call.

As I'm not 100% sure about the intended influence reach of the code, I'd like to ask @rmccue to review the patches and decide which one should be merged.

[rachelbaker]

@rmccue @swissspidy or @pento Can you take a look? I cannot tell from [34926] if the intent was for the result of _wp_json_prepare_data to be passed to json_encode always OR only if json_encode() fails.

[rmccue]

Sorry, I had meant to comment here! 36358-patch1-_wp_json_prepare_data.patch​ is good-to-go, but needs to wait until we've branched to 4.6.

[jrf]

@rmccue @rachelbaker Thank you both for your response. Glad to see this will make it in soon.

Just putting it out there: should this also be ported to 4.5.1 (and maybe 4.4.3) ?

[swissspidy]

Not (yet) familiar with _wp_json_prepare_data(), so maybe anyone else can write a test highlighting the bug and how the patch(es) fixes it?

[jrf]

@swissspidy Re: unit tests - while I totally agree it would be good to have unit tests for these functions, IMHO that's outside of the scope of this issue.

This is simply about fixing a 'typo' in a previous commit.

[swissspidy]

@jrf Yeah, but there's no test at all for wp_json_encode() showing the usage of _wp_json_prepare_data(). That's why I was wondering.

Since _wp_json_prepare_data() was introduced in 4.4 as part of the REST API infrastructure to support the JsonSerializable interface for PHP 5.2 and 5.3, this is unlikely to affect the majority of users.

That's why it doesn't really feel like 4.5.1 material to me, unless it would also be fixed in 4.4.x. What about putting this in 4.6 only?

[jrf]

That's why it doesn't really feel like 4.5.1 material to me, unless it would also be fixed in 4.4.x. What about putting this in 4.6 only?

@rmccue would be the best person to judge this IMHO.

[rmccue]

I am +1 on 4.6-only. I don't think it's a major bug, it's more like an enhancement (i.e. enabling JsonSerializable on PHP <5.4).

[ocean90]

Replying to rmccue:

Sorry, I had meant to comment here! 36358-patch1-_wp_json_prepare_data.patch​ is good-to-go, but needs to wait until we've branched to 4.6.

@rmccue 4.6 was branched a few weeks ago. What is needed to get this in?

[rmccue]

In 37444:

REST API: Use prepared JSON data correctly.

This was modifying a variable that was never used. Oops.

Fixes #36358.
Props jrf.