#19131 closed defect (bug) (fixed)

setting $taxnow in POSTS

Reported by: haayman Owned by: ryan
Priority: normal Milestone: 3.3
Component: Administration Version: 3.2.1
Severity: normal Keywords: has-patch
Cc:

Description

line 96 and 97 in wp-admin/admin.php:

if ( isset($_GET['taxonomy']) )
	$taxnow = sanitize_key($_GET['taxonomy']);
else
	$taxnow = '';

should be

if ( isset($_REQUEST['taxonomy']) )
	$taxnow = sanitize_key($_REQUEST['taxonomy']);
else
	$taxnow = '';

because in wp-admin/edit-tags.php lines 10, 11:

require_once('./admin.php');
$tax = get_taxonomy( $taxnow );
if ( !current_user_can( $tax->cap->manage_terms ) )
  wp_die( __( 'Cheatin’ uh?' ) );

doesn't work when you edit a tag and POST the changes. The current_user_can() is checked against 'tag_post' instead against the actual taxonomy.

Attachments (5)

19131.diff (672 bytes) - added by nacin 19 months ago.
19131.edit-php.patch (834 bytes) - added by ocean90 18 months ago.
19131.2.diff (1.7 KB) - added by ryan 18 months ago.
'_invalid' post type
19131.3.diff (2.2 KB) - added by nacin 18 months ago.
19131.4.diff (4.8 KB) - added by nacin 18 months ago.

Download all attachments as: .zip

Change History (20)

  • Milestone Awaiting Review deleted
  • Resolution set to worksforme
  • Status changed from new to closed

Done in 3.3, see r19097.

  • Milestone set to 3.3
  • Resolution worksforme deleted
  • Status changed from closed to reopened

Yes, but code changes in edit-tags.php has broken this, as it now only checks $_GET.

Attached a patch as an idea.

nacin19 months ago

Yes, but code changes in edit-tags.php has broken this, as it now only checks $_GET.

Same for edit.php.

#18716, #18983

This code has changed so much over the last year or two, mainly due to reshuffling from the list tables. [16253], [18966].

We need to make this standardized; it's a mess.

  • Owner set to ryan
  • Resolution set to fixed
  • Status changed from reopened to closed

In [19253]:

Use taxnow to determine the taxonomy for GET and POST requests in edit-tags.php. Props nacin. fixes #19131

  • Resolution fixed deleted
  • Status changed from closed to reopened

edit.php needs this too.

post.php seems fine. post-new.php uses $_GET but I don't think you ever POST to post-new.php.

There are also post type checks in WP_Posts_List_Table::construct().

The screen code treats a request for ?post_type=foo as a request for post_type = post. This is not good behavior. We should fallback to the default post type only when a particular post type was not requested. Same for taxonomy.

ryan18 months ago

'_invalid' post type

19131.3.diff takes a slightly different approach. Rather than overloading with _invalid, we just keep it as an empty string if we can't ascertain the post type. We'll die anyway, so it'd probably be better to keep the screen object clean.

nacin18 months ago

nacin18 months ago

  • Keywords has-patch added

In [19321]:

Don't fallback to default post type or taxonomy if given an invalid post type or taxonomy. Use typenow as the canonical post type. Props nacin. see #19131

comment:11 follow-up: ↓ 12   ryan18 months ago

This no longer sets _GET[ 'post_type'] to the derived post type. I don't think the lack hurts anything.

Instead of $typenow, standardize on get_current_screen()->post_type?

comment:12 in reply to: ↑ 11   nacin18 months ago

Replying to ryan:

This no longer sets _GET[ 'post_type'] to the derived post type. I don't think the lack hurts anything.

Agreed. Our penchant to hack stuff into $_GET and $_REQUEST bugs the heck out of me.

Instead of $typenow, standardize on get_current_screen()->post_type?

Definitely. I started using it here and looked at killing off usage. It's why there's an otherwise unnecessary $post_type = $typenow;, that way it wouldn't appear in my greps.

typenow:

./wp-admin/admin.php:92:	$typenow = $_REQUEST['post_type'];
./wp-admin/admin.php:94:	$typenow = '';
./wp-admin/admin.php:114:	if ( !empty($typenow) )
./wp-admin/admin.php:115:		$the_parent = $pagenow . '?post_type=' . $typenow;
./wp-admin/admin.php:217:	if ( $typenow == 'page' ) {
./wp-admin/edit.php:12:if ( ! $typenow )
./wp-admin/edit.php:15:$post_type = $typenow;
./wp-admin/includes/plugin.php:1338:	global $typenow;
./wp-admin/includes/plugin.php:1387:			if ( !empty($typenow) && ($submenu_array[2] == "$pagenow?post_type=$typenow") ) {
./wp-admin/includes/plugin.php:1390:			} elseif ( $submenu_array[2] == $pagenow && empty($typenow) && ( empty($parent_file) || false === strpos($parent_file, '?') ) ) {
./wp-admin/includes/plugin.php:1412:	global $typenow;
./wp-admin/includes/plugin.php:1447:						( !empty($typenow) && $parent == $pagenow . '?post_type=' . $typenow)
./wp-admin/includes/screen.php:531:		global $current_screen, $taxnow, $typenow;
./wp-admin/includes/screen.php:534:		$typenow = $this->post_type;
./wp-admin/menu-header.php:37:	global $self, $parent_file, $submenu_file, $plugin_page, $pagenow, $typenow;
./wp-admin/menu-header.php:56:		if ( ( $parent_file && $item[2] == $parent_file ) || ( empty($typenow) && $self == $item[2] ) )
./wp-admin/menu-header.php:123:				$self_type = ! empty( $typenow ) ? $self . '?post_type=' . $typenow : 'nothing';

taxnow:

./wp-admin/admin.php:97:	$taxnow = $_REQUEST['taxonomy'];
./wp-admin/admin.php:99:	$taxnow = '';
./wp-admin/admin.php:223:		if ( $taxnow == 'category' )
./wp-admin/admin.php:225:		elseif ( $taxnow == 'link_category' )
./wp-admin/edit-tags.php:12:if ( ! $taxnow )
./wp-admin/edit-tags.php:15:$tax = get_taxonomy( $taxnow );
./wp-admin/includes/screen.php:531:		global $current_screen, $taxnow, $typenow;
./wp-admin/includes/screen.php:533:		$taxnow = $this->taxonomy;

Most of these don't really need to be touched for now. Let's just stop propagating it.

In [19324]:

WP_Screen->post_type is always set. see #19131.

  • Resolution set to fixed
  • Status changed from reopened to closed
Note: See TracTickets for help on using tickets.