Issue with the ampersand character in members search

[dontdream]

In the Members Directory page, entering in the "Search Members..." search box a string containing an ampersand returns incorrect results.

It looks like the ampersand and any following text are ignored by the search, e.g. if you search for "M&M" you get search results for "M", or if you search for "Wiley & Sons" you get search results for "Wiley ".

[imath]

Hello dontdream,

Thanks for your feedback. I confirm the issue. Suggesting 5694.patch to fix it.

If you can test it and tell me if it did fix the problem for you, it would be great.

[dontdream]

Thank you imath, I tested the patch and it works fine.

Actually it works for me even in a simpler form, patching buddypress.js only, with

if ( search_terms.length ) {
    search_terms = search_terms.split( '&' ).join( '&' );

[DJPaul]

JS' escape has been deprecated for a while.

​https://stackoverflow.com/questions/75980/best-practice-escape-or-encodeuri-encodeuricomponent

[DJPaul]

Also, not sure why you've added esc_html. It doesn't seem to affect the reported issue directly. I'd use something like sanitize_text_field and commit it separately.

[imath]

Replying to DJPaul:

Also, not sure why you've added esc_html

In Activity table '&' are stored '&', so we need a function to encode & to & esc_html() does it, htmlentities2() is also doing, i've updated the patch this way in 5694.02.patch.

[imath]

OOps i made a confusion with another ticket, this one is about the member search issue with '&'. Sorry. Well the problem also exist in activity search ;)

Anyway, after looking at it. The member search is a bit more complex as in bp_legacy_theme_ajax_querystring() a querystring is built from the posted data. And the problem is that when a search is containing '&', it's parsed like if there were a new query var.

For instance (if javascript is enabled) searching for '&mp' will generate a query string like this :
type=alphabetical&action=alphabetical&page=1&search_terms=&mp
Then once parsed in bp_has_members(), the search terms argument is empty and there is a new 'mp' var

So i suggest 5694.03.patch to try to fix this issue.

[DJPaul]

Is this another case where we should change htmlentities2 for addslashes? (I assume the problem with that is that our search parameters expect data with slashes, because we incorrectly store slashed data in the DB).

[dontdream]

No, this time the problem is that we (incorrectly?) store '&' as '&' so we need something like

search_terms = search_terms.split( '&' ).join( '&' );

see comment:3

[imath]

Replying to dontdream:

No, this time the problem is that we (incorrectly?) store '&' as '&' so we need something like

htmlentities2() is doing this conversion, but yes "&" are stored in db "&amp", reason of the use of this function.

[imath]

Ok i've explored it a bit more.

When a profile field is saved, the filter xprofile_sanitize_data_value_before_save enters in action.

It uses xprofile_filter_kses which is using wp_kses which is using wp_kses_normalize_entities() which is replacing & by & see in ​kses.php at line 1150

So i'm suggesting this new version of the patch : 5694.04.patch, where i use wp_kses_normalize_entities() directly in bp_xprofile_bp_user_query_search() and BP_User_Query. This way the search in group members will also benefit of it.

I've tested the patch with javascript enabled and not enabled, it's working.
I've built unit tests and tested it and it's also working.

[DJPaul]

Good job. Also, nice work spotting that urlencode correction.

[imath]

In 8928:

Make sure BP_User_Query returns correct results when search term contains the Ampersand character

1. Search terms containing this character was problematic as the "&" is a query var delimiter used in bp_legacy_theme_ajax_querystring() to build the ajax querystring

2. When a xProfile field is saved, the value is sanitized using the xprofile_filter_kses() filter which is converting "&" to "&" before inserting the value in database.

Urlencoding the search terms in bp_legacy_theme_ajax_querystring() and applying wp_kses_normalize_entities() to search terms in BP_User_Query & in bp_xprofile_bp_user_query_search() make sure the correct results are returned to the user.

Fixes #5694