Critical error when updating xProfile fields through REST API using PHP 8+

[niftythree]

Hello,

When attempting to update an xProfile field through the REST API with value(s) that have an apostrophe in them, the REST API succeeds at making the change but then returns "There has been a critical error on this website." when using PHP 8 versions. Upon downgrading to PHP 7.4, the are no issues. We're using the latest WordPress and BuddyPress versions with the Legacy template.

Output in error log:

PHP Fatal error:  Uncaught TypeError: implode(): Argument #2 ($array) must be of type ?array, bool given in /wp-content/plugins/buddypress/bp-xprofile/bp-xprofile-template.php:986
Stack trace:
#0 /wp-content/plugins/buddypress/bp-xprofile/bp-xprofile-template.php(986): implode()
#1 /wp-content/plugins/buddypress/bp-xprofile/classes/class-bp-rest-xprofile-fields-endpoint.php(860): bp_unserialize_profile_field()
#2 /wp-content/plugins/buddypress/bp-xprofile/classes/class-bp-rest-xprofile-data-endpoint.php(437): BP_REST_XProfile_Fields_Endpoint->get_profile_field_rendered_value()
#3 /wp-content/plugins/buddypress/bp-xprofile/classes/class-bp-rest-xprofile-data-endpoint.php(253): BP_REST_XProfile_Data_Endpoint->prepare_item_for_response()
#4 /wp-includes/rest-api/class-wp-rest-server.php(1193): BP_REST_XProfile_Data_Endpoint->update_item()
#5 /wp-includes/rest-api/class-wp-rest-server.php(1041): WP_REST_Server->respond_to_request()
#6 /wp-includes/rest-api/class-wp-rest-server.php(431): WP_REST_Server->dispatch()
#7 /wp-includes/rest-api.php(424): WP_REST_Server->serve_request()
#8 /wp-includes/class-wp-hook.php(324): rest_api_loaded()
#9 /wp-includes/class-wp-hook.php(348): WP_Hook->apply_filters()
#10 /wp-includes/plugin.php(565): WP_Hook->do_action()
#11 /wp-includes/class-wp.php(418): do_action_ref_array()
#12 /wp-includes/class-wp.php(813): WP->parse_request()
#13 /wp-includes/functions.php(1336): WP->main()
#14 /wp-blog-header.php(16): wp()
#15 /index.php(17): require('/website/12345.we...')
#16 {main} thrown in /wp-content/plugins/buddypress/bp-xprofile/bp-xprofile-template.php on line 986

Also getting: PHP Warning: Attempt to read property "display_name" on bool in /wp-content/plugins/buddypress/bp-messages/classes/class-bp-rest-messages-endpoint.php on line 879.

Thanks.

[espellcaste]

Could you share the REST request performed here? Might be helpful to debug this, although I think I know what it is.

[niftythree]

Hi @espellcaste,

Thanks for your reply. We are performing the following request in Postman:

POST /wp-json/buddypress/v1/xprofile/<fieldid>/data/<userid>

{
    "context" : "edit",
    "value" : "I don\\'t travel often"
}

[espellcaste]

@niftythree Sorry it took me this long to test this. But so far, I'm unable to replicate the issue.

I tested passing the value as such: "I don't travel often" and as suggested by your example: "I don
't travel often". Tested on PHP 8.1 and 8.2.

I don't get the error logged or the PHP warning.

Also getting: PHP Warning: Attempt to read property "display_name" on bool in /wp-content/plugins/buddypress/bp-messages/classes/class-bp-rest-messages-endpoint.php on line 879

The fact that you are getting this warning from the messages endpoint indicates to me there is something else going on in your application that might be affecting your test, but not sure what is based on the information provided.

Can you share more information that might help? Maybe the BuddyPress version, if you are using other plugins, etc. Did you test with the endpoint only and can confirm if it works correctly?

For context, I created a unit test to confirm this is working properly here: ​https://github.com/buddypress/BP-REST/pull/497

[niftythree]

Hi @espellcaste,

No problem, thanks for your reply. Sorry, we should have added we're trying to update a checkbox, but this also happens for the multi-select box too, for example. Updating a text box is OK.

PHP version: 8.1
Theme: Twenty Twenty-Four
BuddyPress Template: Legacy
Plugins: The only plugins we had running were BuddyPress and JSON Basic Authentication (​https://github.com/WP-API/Basic-Auth)

Here's some extra information that could assist with the critical error:
We tested this in a clean environment, with a new server, new application and with the latest WordPress (6.5) and BuddyPress (12.4.0). We created a new group and a new field within this group with "Checkboxes" and options ordered in Custom, one being "I don't travel often".

Thanks.

[espellcaste]

Thanks! That helped! Confirmed that unserialize doesn't work well when there is quoted strings inside it.

The issue is coming from our bp_unserialize_profile_field ​function.

This does not only happen when updating a field, but by seeing a field too.

The root of the issue seems to be ​this line.

It is removing the slash of the quoted string and the PHP unserialize function can't handle the new string properly.

@imath I'd love your take on a possible solution here.

[espellcaste]

I added some unit tests to confirm this is a real issue: ​https://github.com/buddypress/BP-REST/pull/497

[imath]

Hi @espellcaste

Here's my opinion about it!

This very early stripslashes() is there since ​1.0. I checked history if this wasn't made on purpose to fix a bug.

We have a lot of filters stripping slashes, so we are probably doing this twice. I'd see what happens if we remove this very early stripslashes() <- probably the nicest solution ;)

The other possibilities I see would be:

• to do this before saving is_string( $value ) ? wptexturize( $value ) : $value; but I don't think we should do this.
• to avoid doing this very early stripslashes()

eg: $this->value = is_serialized( $profiledata->value ) ? $profiledata->value : stripslashes( $profiledata->value ); <- probably the less risky!

Trac ticket: ​https://buddypress.trac.wordpress.org/ticket/9127

[espellcaste]

So, while testing this, the last suggestion is the better, and safer, one. Removing stripslashes all together was affecting non-serializable fields, like ​this example.

[espellcaste]

• The fix in BP core: ​https://github.com/buddypress/buddypress/pull/268
• Here is the pr with the fix applied and working in the BP REST API: ​https://github.com/buddypress/BP-REST/pull/497

I tested this locally too. Both via the BP REST API and a local site (browser/UI) too.

[imath]

Awesome work @espellcaste on the ticket! Let's go with the safer approach 👍

[espellcaste]

In 13791:

Strip slashes from a BP_XProfile_ProfileData field value only if the field is serialized.

This change makes sure that XProfile fields with serialized data correctly keeps the string with apostrophes.

Props imath and niftythree
See ​https://github.com/buddypress/BP-REST/pull/497
Closes ​https://github.com/buddypress/buddypress/pull/268
Fixes #9127

[espellcaste]

The PHP warning mentioned was fixed at #9133. :)