Make WordPress Core

Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#35902 closed defect (bug) (fixed)

WP_Query::is_page() can return incorrect results when post title includes a slash

Reported by: mikejolley's profile mikejolley Owned by: boonebgorges's profile boonebgorges
Milestone: 4.5 Priority: normal
Severity: normal Version:
Component: General Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

In WP_Query::is_page(), the following evaluations are made to determine if the queried object matches the supplied page property:

if ( in_array( (string) $page_obj->ID, $page ) ) {
  return true;
} elseif ( in_array( $page_obj->post_title, $page ) ) {
  return true;
} elseif ( in_array( $page_obj->post_name, $page ) ) {
  return true;
} else {

If you have a page with a slash in the name, for example, 4/6 Sample Page, WP_Query::is_page(4) returns true, even if the actual page ID is something else.
I traced this back to the line:

} elseif ( in_array( $page_obj->post_title, $page ) ) {

In my test case, $page is:

array(1) { [0]=> int(4) } 

If you cast 4/6 Sample Page to int, it becomes int(4) causing the match. This comparison should be strict...

There was some discussion around this in ticket https://core.trac.wordpress.org/ticket/24674, however, I don't see a fix for the case I posted above. Only IDs were fixed.

When comparing string based post_titles, ideally the $page should be cast to string to prevent the above case returning true.

To fix this I believe we can do some more intelligent casting.
The first rule:

if ( in_array( (string) $page_obj->ID, $page ) ) {

ID is cast to string and compared against $page with type array, contents unknown.
The second rule we already know title is string:

} elseif ( in_array( $page_obj->post_title, $page ) ) {

Same for the postname rule:

} elseif ( in_array( $page_obj->post_name, $page ) ) {

Since all comparisons use string, we should be able to safely cast the array values to a string too:

$page = array_map( 'strval', (array) $page );

This fixes all comparisons and prevents the bug occurring.

I found this bug when investigating https://github.com/woothemes/woocommerce/issues/10404. Page name was 4/6 Sample Page, and in my case my shop page was ID 4. Because of this, checks for page ID 4 (shop) returned true on 4/6 Sample Page incorrectly.

Patch to follow...

Attachments (2)

fix-35902.diff (372 bytes) - added by mikejolley 9 years ago.
Fix for 35902
35902.diff (1.3 KB) - added by swissspidy 9 years ago.

Download all attachments as: .zip

Change History (9)

@mikejolley
9 years ago

Fix for 35902

#1 @mikejolley
9 years ago

#35901 was marked as a duplicate.

#2 @mikejolley
9 years ago

  • Keywords has-patch added

#3 @swissspidy
9 years ago

  • Keywords needs-unit-tests added

@swissspidy
9 years ago

#4 @swissspidy
9 years ago

  • Keywords has-unit-tests added; needs-unit-tests removed
  • Milestone changed from Awaiting Review to 4.5

#5 @boonebgorges
9 years ago

  • Owner set to boonebgorges
  • Status changed from new to assigned

This needs treatment for the other conditional functions, and separate tests for the various ways the checks could fail. I'll look at it.

#6 @boonebgorges
9 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 36625:

Query: is_*( $int ) should not falsely match strings starting with "$int".

Another chapter in the Storied Annals of Weird in_array() Behavior:
in_array( 4, array( "4-cool-dudes" ) ); resolves to true, such that
is_page( 4 ) was returning true for posts with the name '4-cool-dudes'.

We work around this behavior by ensuring that values passed to the is_
methods are cast to strings before the in_array() checks. ID checks still
work as expected; see #24674.

Props mikejolley, swissspidy, boonebgorges.
Fixes #35902.

#7 @johnbillion
9 years ago

  • Version trunk deleted
Note: See TracTickets for help on using tickets.