From 3685f95eb6b64cddf935044edf92c6216e430753 Mon Sep 17 00:00:00 2001 From: Jb Audras Date: Wed, 12 Jan 2022 23:23:41 +0000 Subject: [PATCH] Administration: Ensure an integer is used for menu priority in `add_menu_page()`. This change adds a verification of the `$position` parameter in `add_menu_page()` to ensure an integer is used. If not, the function informs developers of the wrong parameter type via a `_doing_it_wrong` message. This brings consistency with a similar check used in `add_submenu_page()`. This change also typecasts any floating number to string to ensure that in case a float value was passed, at least it doesn't override existing menus. Follow-up to [46570]. Props kirtan95. Fixes #54798. See #48249. git-svn-id: https://develop.svn.wordpress.org/trunk@52569 602fd350-edb4-49c9-b593-d223f7449a82 --- src/wp-admin/includes/plugin.php | 15 +++++++++ tests/phpunit/tests/admin/includesPlugin.php | 35 ++++++++++++++++++-- 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/src/wp-admin/includes/plugin.php b/src/wp-admin/includes/plugin.php index c0ac17a881..bf2bc8a5ec 100644 --- a/src/wp-admin/includes/plugin.php +++ b/src/wp-admin/includes/plugin.php @@ -1329,6 +1329,21 @@ function add_menu_page( $page_title, $menu_title, $capability, $menu_slug, $func $position = $position + substr( base_convert( md5( $menu_slug . $menu_title ), 16, 10 ), -5 ) * 0.00001; $menu[ "$position" ] = $new_menu; } else { + if ( ! is_int( $position ) ) { + _doing_it_wrong( + __FUNCTION__, + sprintf( + /* translators: %s: add_submenu_page() */ + __( 'The seventh parameter passed to %s should be an integer representing menu position.' ), + 'add_menu_page()' + ), + '6.0.0' + ); + // If the position is not a string (i.e. float), convert it to string. + if ( ! is_string( $position ) ) { + $position = (string) $position; + } + } $menu[ $position ] = $new_menu; } diff --git a/tests/phpunit/tests/admin/includesPlugin.php b/tests/phpunit/tests/admin/includesPlugin.php index 2fde9195fd..52a4b509e9 100644 --- a/tests/phpunit/tests/admin/includesPlugin.php +++ b/tests/phpunit/tests/admin/includesPlugin.php @@ -296,11 +296,11 @@ class Tests_Admin_IncludesPlugin extends WP_UnitTestCase { } /** - * Passing a string as position will fail. + * Passing a string as position will fail in submenu. * * @ticket 48599 */ - public function test_passing_string_as_position_fires_doing_it_wrong() { + public function test_passing_string_as_position_fires_doing_it_wrong_submenu() { $this->setExpectedIncorrectUsage( 'add_submenu_page' ); global $submenu, $menu; @@ -324,6 +324,37 @@ class Tests_Admin_IncludesPlugin extends WP_UnitTestCase { $this->assertSame( 'submenu_page_1', $submenu['main_slug'][1][2] ); } + /** + * Passing a string as position will fail in menu. + * + * @ticket 48599 + */ + public function test_passing_string_as_position_fires_doing_it_wrong_menu() { + $this->setExpectedIncorrectUsage( 'add_menu_page' ); + global $submenu, $menu; + + // Reset menus. + $submenu = array(); + $menu = array(); + $current_user = get_current_user_id(); + $admin_user = self::factory()->user->create( array( 'role' => 'administrator' ) ); + wp_set_current_user( $admin_user ); + set_current_screen( 'dashboard' ); + + // Setup a menu with some items. + add_menu_page( 'Main Menu', 'Main Menu', 'manage_options', 'main_slug', 'main_page_callback', 'icon_url', '1' ); + add_menu_page( 'Main Menu 1', 'Main Menu 1', 'manage_options', 'main1_slug', 'main1_page_callback', 'icon_url1', 1.5 ); + + // Clean up the temporary user. + wp_set_current_user( $current_user ); + wp_delete_user( $admin_user ); + + // Verify the menu was inserted. + $this->assertSame( 'main_slug', $menu[1][2] ); + // Verify the menu was inserted correctly on passing float as position. + $this->assertSame( 'main1_slug', $menu['1.5'][2] ); + } + public function test_is_plugin_active_true() { activate_plugin( 'hello.php' ); $test = is_plugin_active( 'hello.php' );