WordPress.org

Make WordPress Core

Opened 23 months ago

Last modified 7 months ago

#37276 new defect (bug)

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

Reported by: smerriman Owned by:
Milestone: Awaiting Review 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 23 months ago.
Adds wp_unslash to mirror what we do when inserting the term

Download all attachments as: .zip

Change History (10)

@elrae
23 months ago

Adds wp_unslash to mirror what we do when inserting the term

#1 @elrae
23 months 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
23 months 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
23 months 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
22 months 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
22 months 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.


22 months ago

#7 @boonebgorges
21 months 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
20 months 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
7 months 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?

Note: See TracTickets for help on using tickets.