Make WordPress Core

Opened 8 years ago

Closed 5 years ago

#37276 closed defect (bug) (duplicate)

tax_query with field=name doesn't work if the term contains an apostrophe

Reported by: smerriman's profile smerriman Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Taxonomy Keywords: has-patch needs-unit-tests close
Focuses: Cc:

Description

Related to the bug in #27810.

If you create a tax_query with field=name and term containing an apostrophe, eg:

new WP_Query(array(
    'tax_query'=>array(
        array(
        	'taxonomy'=>'sample-tax',
        	'field'=>'name',
        	'terms'=>"Test's"
        )
    )
));

the query built in the transform_query function in class-wp-tax-query.php comes back with:

... AND wp_terms.name IN ('Test\\\'s')

and thus generates no results.

It looks like the apostrophe is getting double escaped by both the esc_sql and sanitize_term_field calls in @boonebgorges patch.

Stab in the dark - in wp_insert_term, wp_unslash is called on the name field after sanitizing it - does the same need to apply here?

Attachments (1)

37276.diff (674 bytes) - added by elrae 8 years ago.
Adds wp_unslash to mirror what we do when inserting the term

Download all attachments as: .zip

Change History (11)

@elrae
8 years ago

Adds wp_unslash to mirror what we do when inserting the term

#1 @elrae
8 years ago

  • Keywords has-patch added

Patch uploaded that runs the wp_unslash to mirror what happens when inserting the term. This solved the issue and lets you query with an apostrophe. This bug also prohibited terms with quotes ( " ) from being pulled in. This patch fixes that as well.

#2 @smerriman
8 years ago

Is there any danger calling wp_unslash when 'slug' is handled by the same code? (Or should it apply to name only)?

#3 @elrae
8 years ago

There is no danger with slugs, the function just runs stripslashes and removes them. When dealing with slugs, you aren't allowed to use characters that would require a slash, and you aren't allowed to use special chars anyways.

#4 @ocean90
8 years ago

  • Keywords needs-unit-tests added
  • Version trunk deleted

@elrae Thanks for your patch! Can you also add a unit test for this?

#5 @elrae
8 years ago

I'm not sure how to write / add unit tests, is there a guide for it somewhere I can reference? Happy to help once I'm up to speed on it.

This ticket was mentioned in Slack in #core by elrae. View the logs.


8 years ago

#7 @boonebgorges
8 years ago

Thanks for the patch, @elrae. Here's an existing test you can copy for the purposes of testing with apostrophes and other slashed characters: https://core.trac.wordpress.org/browser/trunk/tests/phpunit/tests/term/taxQuery.php#L170

#8 @elrae
7 years ago

Thanks for the reference boone. Unfortunately my schedule has been crazy packed the last few months. If I get some time in the next few weeks to add this test I definitely will, but if anyone else comes across this ticket and wants to hop in feel free.

#9 @birgire
6 years ago

  • Keywords close added

This seems to have been solved in #39315. The changelog there looks very similar to the suggestion here:

https://core.trac.wordpress.org/changeset/39662
https://core.trac.wordpress.org/attachment/ticket/37276/37276.diff

Then it looks like transform_query() was later changed further.

So I think we can mark this as fixed and close it?

#10 @boonebgorges
5 years ago

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

Duplicate of #39315.

Yup, it looks like [39662] fixed it.

Note: See TracTickets for help on using tickets.