WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#23208 closed defect (bug) (fixed)

Incorrect variable name for comment meta queries

Reported by: danielbachhuber Owned by: nacin
Milestone: 3.5.1 Priority: normal
Severity: normal Version: 3.5
Component: Comments Keywords:
Focuses: Cc:

Description

An error notice I saw using the Liveblog plugin:

Undefined variable: q in /Users/danielbachhuber/wp/wp-includes/comment.php on line 299

traces back to r22074 where the following is added:

// Parse meta query
$this->meta_query = new WP_Meta_Query();
$this->meta_query->parse_query_vars( $this->query_vars );

but then is used later as:

if ( ! empty( $this->query_vars['meta_key'] ) ) {
	$allowed_keys[] = $q['meta_key'];
		$allowed_keys[] = 'meta_value';
		$allowed_keys[] = 'meta_value_num';
	}
	$ordersby = array_intersect( $ordersby, $allowed_keys );
	foreach ( $ordersby as $key => $value ) {
		if ( $value == $q['meta_key'] || $value == 'meta_value' ) {
			$ordersby[ $key ] = "$wpdb->commentmeta.meta_value";
		} elseif ( $value == 'meta_value_num' ) {
			$ordersby[ $key ] = "$wpdb->commentmeta.meta_value+0";
		}
	}
}

Something like the patch attached resolves the error, but this would probably benefit from unit tests too.

Attachments (1)

comment-notice.diff (863 bytes) - added by danielbachhuber 3 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 @danielbachhuber3 years ago

Actually, it looks like there are already unit tests for this. I'll see if I can track down further why they don't fail when I have a moment.

comment:2 @ocean903 years ago

  • Milestone changed from Awaiting Review to 3.5.1

comment:3 @nacin3 years ago

Patch is incorrect; this is a case of taking $q and replacing it with $this->query_vars. $this->query_vars['meta_key'] is correct there, not 'meta_key'.

Last edited 3 years ago by nacin (previous) (diff)

comment:4 @nacin3 years ago

In 23325:

Comment Query: Use $this->query_vars instead of the nonexistent shorthand $q. see #23208.

comment:5 @nacin3 years ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from new to closed

In 23332:

Comment Query: Use $this->query_vars instead of the nonexistent shorthand $q.

Merges [23325] to the 3.5 branch.
fixes #23208.

comment:7 @SergeyBiryukov3 years ago

In 1191/tests:

Comment meta query tests for ordering by meta key. see #23208.

comment:8 @SergeyBiryukov3 years ago

  • Keywords needs-patch removed

Replying to danielbachhuber:

Actually, it looks like there are already unit tests for this. I'll see if I can track down further why they don't fail when I have a moment.

The tests only had assertions for ordering by meta value. To trigger the bug, you need to order by meta key, which is functionally the same.

Note: See TracTickets for help on using tickets.