jetpack icon indicating copy to clipboard operation
jetpack copied to clipboard

WPCOM endpoint wpcom/v2/admin-menu not setting is_admin() = true -> causing issues

Open Ninodevo opened this issue 3 years ago • 3 comments

Impacted plugin

Jetpack

Steps to Reproduce

The used plugin here is only an example that causes the issue it could also be other plugins with similar implementations.

Example 1:

  1. Install Meta Slider plugin on a WPCOM site and Activate the plugin
  2. Call the route -> https://public-api.wordpress.com/wpcom/v2/sites/{blog_id}/admin-menu (Authorization: Bearer {token})

Example 2:

  1. Install Meta Slider plugin on a WPCOM site and Activate the plugin
  2. Go to Calypso admin of the site
  3. View the error log of the site in Grafana

A clear and concise description of what you expected to happen.

Example 1: I expect to get response 200 and an admin menu item.

Example 2: I expect to not see any fatal errors related to the MetaSlider plugin.

What actually happened

Example 1:

I received error 500:

Screenshot 2022-11-16 at 14 56 52

Example 2:

I see a fatal error: PHP Fatal error: Uncaught Error: Call to a member function add_page() on null... 2e365-pb

Browser

Google Chrome/Chromium

Other information

I believe that the issue is not in the plugin but in how our Jetpack WPCOM endpoint wpcom/v2/admin-menu is implemented -> wpcom-rest-api-v2-endpoint-admin-menu.php

Here is the code 2e366-pb from the MetaSlider plugin (how the plugin is written is not important only the code I will highlight). On line 116 is_admin() check is used to init a class property and on line 282 add_action('admin_menu', array($this, 'register_admin_pages'), 9553); a hook is added to admin_menu whose callback uses the initialized class property in is_admin. The hook admin_menu is only called in the admin so that should be ok.

In the Jetpack WPCOM endpoint wpcom/v2/admin-menu the get_item function includes the wp-admin/menu file on line 91 which subsequently includes the wp-admin/includes/menu.php file and that triggers the admin_menu hook but is_admin returns false.


The endpoint wpcom/v2/admin-menu -> triggers admin_menu hook but is_admin is false which shouldn't happen

I believe is_admin should be true if we are using the wpcom/v2/admin-menu endpoint or the implementation of the route should be in some other way (not including the WP admin files).

The plugin author should be able to use the is_admin function in regard to the admin_menu hook (which should only be fired in the admin). This plugin is only an example if there are similar implementations of using is_admin in combination of the admin menu hooks those will fail.

Platform (Simple, Atomic, or both?)

Atomic

Reproducibility

Consistent

Severity

Some (< 50%)

Available workarounds?

No response

Workaround details

No response

Ninodevo avatar Nov 16 '22 14:11 Ninodevo

Hey @jeherve, I'm wondering if you can share any insight for this issue being looked at?

xpurichan avatar Jun 19 '24 17:06 xpurichan

@xpurichan I don't believe anyone is looking into this issue at the moment.

@davemart-in Do you think that's something you could look at as part of the Untangling Calypso project?

jeherve avatar Jun 20 '24 07:06 jeherve

Yup! @jeherve I just moved it to our board. Thanks for the ping!

davemart-in avatar Jun 20 '24 18:06 davemart-in

Removing this from the logical flows board. It's no longer a great fit.

davemart-in avatar Aug 23 '24 13:08 davemart-in

@fushar not urgent, please take a look at this when you have capacity. Thanks!

taipeicoder avatar Nov 07 '24 01:11 taipeicoder

Sure, I can start looking into this tomorrow.

fushar avatar Nov 07 '24 09:11 fushar

Sorry for the delay. I tried to reproduce the issue without success. Is this still happening? I installed the MetaSlider plugin, but I don't see any error logs. The admin-menu endpoint also returns 200 correctly.

Image

Image

fushar avatar Dec 04 '24 08:12 fushar

Hey @fushar!

I'm sorry, but I've noticed that the steps I outlined above are not 100% correct.

The issue occurs if your site is not hosted on WordPress.com but on another host, such as Pressable, and you have an active Jetpack connection with a plan (e.g., Complete). When you open the site on WP.com (for example, https://wordpress.com/posts/{site_url}) and attempt to click on a post, nothing happens, and you encounter an error in Grafana.

PHP Fatal error: Uncaught Error: Call to a member function add_page() on null in /srv/htdocs/wp-content/plugins/ml-slider/ml-slider.php:489 Stack trace: #0 /wordpress/core/6.7.1/wp-includes/class-wp-hook.php(324): MetaSliderPlugin->register_admin_pages('') #1 /wordpress/core/6.7.1/wp-includes/class-wp-hook.php(348): WP_Hook->apply_filters(NULL, Array) #2 /wordpress/core/6.7.1/wp-includes/plugin.php(517): WP_Hook->do_action(Array) #3 /wordpress/core/6.7.1/wp-admin/includes/menu.php(161): do_action('admin_menu', '') #4 /wordpress/core/6.7.1/wp-admin/menu.php(412): require_once('/wordpress/core...') #5 /wordpress/plugins/jetpack/14.1-beta/_inc/lib/core-api/wpcom-endpoints/class-wpcom-rest-api-v2-endpoint-admin-menu.php(97): require_once('/wordpress/core...') #6 /wordpress/core/6.7.1/wp-includes/rest-api/class-wp-rest-server.php(1292): WPCOM_REST_API_V2_Endpoint_Admin_Menu->get_item(Object(WP_REST_Request)) #7 /wordpress/core/6.7.1/wp-includes/rest-api/class-wp-rest-server.php(1125): WP_REST_Server->respond_to_request(Object(WP_REST_Request), '/wpcom/v2/admin...', Array, NULL) #8 /wordpress/core/6.7.1/wp-includes/rest-api/class-wp-rest-server.php(439): WP_REST_Server->dispatch(Object(WP_REST_Request)) #9 /wordpress/core/6.7.1/wp-includes/rest-api.php(449): WP_REST_Server->serve_request('/wpcom/v2/admin...') #10 /wordpress/core/6.7.1/wp-includes/class-wp-hook.php(324): rest_api_loaded(Object(WP)) #11 /wordpress/core/6.7.1/wp-includes/class-wp-hook.php(348): WP_Hook->apply_filters(NULL, Array) #12 /wordpress/core/6.7.1/wp-includes/plugin.php(565): WP_Hook->do_action(Array) #13 /wordpress/core/6.7.1/wp-includes/class-wp.php(418): do_action_ref_array('parse_request', Array) #14 /wordpress/core/6.7.1/wp-includes/class-wp.php(813): WP->parse_request('') #15 /wordpress/core/6.7.1/wp-includes/functions.php(1336): WP->main('') #16 /wordpress/core/6.7.1/wp-blog-header.php(16): wp() #17 /wordpress/core/6.7.1/index.php(17): require('/wordpress/core...') #18 {main} thrown in /srv/htdocs/wp-content/plugins/ml-slider/ml-slider.php on line 489

Ninodevo avatar Dec 04 '24 11:12 Ninodevo

I see, I can reproduce it using your steps, thanks!

However, is it still a real issue for users? For self-hosted Jetpack sites, we (almost) don't have entry point to Calypso screens anymore, and not in the future. For example, there's no way to go the /posts/:slug unless the user types that URL manually. See: pekYwv-4QE-p2#comment-4367

Asking to gauge the priority.

fushar avatar Dec 05 '24 07:12 fushar

Support References

This comment is automatically generated. Please do not edit it.

  • [ ] pekYwv-4QE-p2#comment-4367

github-actions[bot] avatar Dec 05 '24 07:12 github-actions[bot]

The issue was reported two times on our Team51 Partner Sites [Issue1] & [Issue2]. I will ping on those issues with your message and the P2 comment to see if we should close them, especially if we are moving away from Calypso for Jetpack sites.

Ninodevo avatar Dec 05 '24 11:12 Ninodevo

Hey @fushar sorry for the late answer, but I just got back from the partner issues and our TAMs are happy to close out this issue since we are moving away from Calypso for Jetpack sites so there is no need to fix this.

Ninodevo avatar Jan 20 '25 09:01 Ninodevo

Thanks for the follow-up @Ninodevo! Closing this issue.

taipeicoder avatar Jan 23 '25 06:01 taipeicoder