WordPress.org

Make WordPress Core

Opened 3 months ago

Last modified 3 days ago

#51986 new defect (bug)

PHP Warning: array_intersect_key(): Expected parameter 1 to be an array, string given in class-wp-rest-server.php

Reported by: slaFFik Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 5.6
Component: Editor Keywords: php8 has-patch needs-unit-tests
Focuses: rest-api Cc:

Description (last modified by desrosj)

When editing a page in the Gutenberg editor (that is built-in, not the plugin), I see this Warning on a page:

<b>Warning</b>:  array_intersect_key(): Expected parameter 1 to be an array, string given in <b>/home/cosydale/public_html/ovirium.com/wp-includes/rest-api/class-wp-rest-server.php</b> on line <b>1402</b><br />

Disable REST plugin is not installed.

Change History (26)

#1 @desrosj
3 months ago

  • Component changed from REST API to Editor
  • Description modified (diff)
  • Focuses rest-api added
  • Keywords reporter-feedback added

Hi @slaFFik!

Do you have any plugins registering custom REST API routes? Any plugins that are registering custom block types?

#2 @apermo
3 months ago

I have the same issue.

I've already posted some information here: https://wordpress.org/support/topic/wordpress-5-6-upgrading-issue/#post-13765662

I have the same issue, among my active plugins is redux, and quite a long list of custom plugins.

Warning: array_intersect_key(): Expected parameter 1 to be an array, string given in /wp-includes/rest-api/class-wp-rest-server.php on line 1402

Call Stack
#	Time	Memory	Function	Location
1	0.0109	488824	{main}( )	.../post.php:0
2	3.6107	69597144	require( '/wp-admin/edit-form-blocks.php' )	.../post.php:187
3	3.6108	69600920	array_reduce ( )	.../edit-form-blocks.php:82
4	3.6108	69600920	rest_preload_api_request( )	.../edit-form-blocks.php:82
5	3.6108	69601520	rest_do_request( )	.../rest-api.php:2520
6	3.6820	71674264	WP_REST_Server-&gt;dispatch( )	.../rest-api.php:479
7	3.6858	71732896	WP_REST_Server-&gt;respond_to_request( )	.../class-wp-rest-server.php:1007
8	3.6858	71732896	WP_REST_Server-&gt;get_index( )	.../class-wp-rest-server.php:1160
9	3.6900	72029472	WP_REST_Server-&gt;get_data_for_routes( )	.../class-wp-rest-server.php:1243
10	3.6909	72042136	WP_REST_Server-&gt;get_data_for_route( )	.../class-wp-rest-server.php:1324
11	3.7373	72049584	array_intersect_key ( )	.../class-wp-rest-server.php:1402

My research me to redux-framework/redux-templates/classes.class-api.php and the class method register_api_hooks in line 785.

Last edited 2 months ago by jrf (previous) (diff)

#3 @apermo
3 months ago

Index: public_html/wp-includes/rest-api/class-wp-rest-server.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/public_html/wp-includes/rest-api/class-wp-rest-server.php b/public_html/wp-includes/rest-api/class-wp-rest-server.php
--- a/public_html/wp-includes/rest-api/class-wp-rest-server.php	(date 1607525283294)
+++ b/public_html/wp-includes/rest-api/class-wp-rest-server.php	(date 1607525283294)
@@ -1399,7 +1399,7 @@
 				$endpoint_data['args'] = array();
 
 				foreach ( $callback['args'] as $key => $opts ) {
-					$arg_data             = array_intersect_key( $opts, $allowed_schema_keywords );
+					$arg_data             = array_intersect_key( (array) $opts, $allowed_schema_keywords );
 					$arg_data['required'] = ! empty( $opts['required'] );
 
 					$endpoint_data['args'][ $key ] = $arg_data;

This patch solves the issue for me.

#4 @slaFFik
3 months ago

@desrosj

Yes, and with the help of the developers of one of the active plugins on my site, we identified the plugin that is causing that issue. It will be patched, but I think that WordPress itself should also make sure that it's working with the array in the array_intersect_key() function.

Last edited 3 months ago by slaFFik (previous) (diff)

#5 @desrosj
3 months ago

  • Keywords needs-testing added

@apermo thanks for those details! I am also able to reproduce with the redux-framework plugin and looking into the issue some more.

@slaFFik I don't disagree, but it's important to understand what the patterns are causing the issue and confirm that type casting will not cause unintended consequences or backwards compatibility concerns before deciding on the correct change.

Are you able to share which plugin is causing the issue for you? If it's a custom plugin not available on the wordpress.org/plugin directory, dropping the offending code would be very helpful.

#6 follow-up: @slaFFik
3 months ago

@desrosj

The plugin is not hosted on wp.org.
Here is the code of that plugin that registers the endpoints:

public function register_api_endpoints() {

    register_rest_route( 'example/v1', '/some-content/woof/(?P<type>[a-zA-Z0-9-]+)', array(
        'methods'             => 'GET',
        'callback'            => array( $this, 'get_gutenberg_themes' ),
        'permission_callback' => function () {
            return current_user_can( 'edit_posts' );
        },
        'args'                => array(
            'type',
        ),
    ) );

    register_rest_route( 'example/v1', '/terms/(?P<slug>[a-zA-Z0-9-_]+)', array(
        'methods'             => 'GET',
        'callback'            => array( $this, 'get_taxonomy_terms' ),
        'permission_callback' => function () {
            return current_user_can( 'edit_posts' );
        },
        'args'                => array(
            'slug',
        ),
    ) );

    register_rest_route( 'example/v1', '/taxonomy/(?P<slug>[a-zA-Z0-9-_]+)', array(
        'methods'             => 'GET',
        'callback'            => array( $this, 'get_taxonomy' ),
        'permission_callback' => function () {
            return current_user_can( 'edit_posts' );
        },
        'args'                => array(
            'slug',
        ),
    ) );
}

It looks like it should be 'args' => [ 'type' => [] ] instead of 'args' => [ 'type' ] (same for slug).

Last edited 3 months ago by slaFFik (previous) (diff)

#7 in reply to: ↑ 6 @desrosj
3 months ago

  • Keywords needs-patch php8 added; needs-testing removed
  • Milestone changed from Awaiting Review to 5.6.1

It looks like it should be 'args' => [ 'type' => [] ] instead of 'args' => [ 'type' ] (same for slug).

Correct!

One additional note here, this pattern will cause a fatal error in PHP 8 due to strict typing now being applied to all internal PHP functionality.

Looks like this is a result of the efforts on #51020 (see the diff). Non-arrays were never truly supported, but the changes in #51020 seem to have surfaced warnings when being defined incorrectly.

I've been working through this with @TimothyBlynJacobs a bit in Slack.

The args is used primarily for validation, but with those strings set, it wouldn't be able to validate parameters. The get_data_for_route outputs the index. So the endpoint would "work" but wouldn't be doing validation properly and wouldn't be outputting correct schemas, but the route should actually be able to process the request and the response.

The type casting suggested above does not appear to be the correct solution here. While it would silence the warning, it would not force the correct behavior. A _doing_it_wrong() to notify developers of incorrect usage paired with skipping the item in the array when not the correct type seems like the safest way forward.

This ticket was mentioned in Slack in #core by desrosj. View the logs.


3 months ago

This ticket was mentioned in Slack in #forums by timothybjacobs. View the logs.


3 months ago

#10 @AndrewNZ
3 months ago

I have just created a new ticket, #52008 when I perhaps should have made a comment on this ticket.

This ticket was mentioned in PR #802 on WordPress/wordpress-develop by aristath.


3 months ago

  • Keywords has-patch added; needs-patch removed

#12 @aristath
3 months ago

  • Keywords needs-patch added; has-patch removed

I added a simple patch in https://github.com/WordPress/wordpress-develop/pull/802
The patch checks if the 1st argument is a string, and if it is then converts it to an array prior to array_intersect_key().

#13 @poena
3 months ago

I did a basic test of the patch by:
Installing the Redux plugin and confirming that the PHP warning was reproduced.
Applying the patch and confirming that the new doing it wrong message is displayed.
(PHP version: 7.4.11)

Last edited 3 months ago by poena (previous) (diff)

#14 @desrosj
3 months ago

#52008 was marked as a duplicate.

#15 @prbot
3 months ago

TimothyBJacobs commented on PR #802:

@aristath thanks for the patch! I think I would prefer if we moved this to register_rest_route like we do for our other _doing_it_wrong notices. It should hopefully make it more clear about where the error is coming from and we can make sure it is only printed once per route. Something like

if ( count( array_filter( $args, 'is_array' ) ) !== count( $args ) {

}

#16 follow-up: @dovyp
3 months ago

The Redux issue has been resolved in version 4.1.24. A huge help to @TimothyBlynJacobs for his guidance. :)

Last edited 3 months ago by dovyp (previous) (diff)

#17 @prbot
3 months ago

aristath commented on PR #802:

Thank you @TimothyBJacobs,
I moved the _doing_it_wrong call to register_rest_route. I did however leave the extra check in get_data_for_route as that will act as a safeguard in case a REST route is registered via rest_get_server()->register_route or other methods and not using the register_rest_route function.

#18 in reply to: ↑ 16 @apermo
2 months ago

Replying to dovyp:

The Redux issue has been resolved in version 4.1.24. A huge help to @TimothyBlynJacobs for his guidance. :)

Can confirm, it works, just updated my system. Thanks for your effort :)

#19 @hellofromTonya
2 months ago

  • Keywords has-patch added; reporter-feedback needs-patch removed

#20 @prbot
8 weeks ago

TimothyBJacobs commented on PR #802:

Thanks for updating this @aristath! We should be checking $arg_group['args'] not $args though. Let's also make sure to include the namespace and route in the doing it wrong message so it will be cleared to the developers exactly which route registration is failing.

We can also make the check in get_data_for_route use is_array as well since any non-array value will cause PHP warnings, not just strings.

Lastly, would you be interested in writing unit tests for this? You can see \Tests_REST_API::test_register_route_with_missing_permission_callback_top_level_route for some examples of how to test register_rest_route with an invalid definition.

#21 @prbot
6 weeks ago

TimothyBJacobs commented on PR #802:

Hey @aristath,

Just checking in about unit tests.

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


4 weeks ago

#23 @whyisjake
4 weeks ago

As we prep for the 5.6.1 release, we are going to punt this to the 5.6.2 milestone as we still need some work and tests.

#24 @audrasjb
4 weeks ago

  • Milestone changed from 5.6.1 to 5.6.2

Given 5.6.1, let's move this ticket to 5.6.2 to give it some more time.

#25 @desrosj
10 days ago

  • Keywords needs-unit-tests added
  • Milestone changed from 5.6.2 to Future Release

5.6.2 RC is going to be packaged in a few hours. It seems that the PR has an open request for some unit tests, so I'm going to punt to Future Release.

#26 @prbot
3 days ago

aristath commented on PR #802:

@TimothyBJacobs I tried but I'm afraid I don't understand how to write unit tests for that.
If you want I can just close this PR so you can create another one with tests included.

Note: See TracTickets for help on using tickets.