S2, Ampersand problem

Resolved ccgjosh
(@ccgjosh)

12 years, 1 month ago

We’re using S2 member with buddypress to handle user accounts and showing them with AMR Users. Our members are categorized based on industry which we previously setup using ampersands. The filtering on the front end does not work with any categories with &, probably because it is stored in the database as & while the filtering form uses esc_attr. How can I fix this? http://www.towsonchamber.com/members/

Also we were having a problem with companies in multiple categories showing up as extra filters in the select menu. This was resolved by adding a space after the comma in the explode function of amr-users-plus.php —

$colarr = explode(', ',$user[$col]); // amr

https://wordpress.org/plugins/amr-users/

Viewing 6 replies - 1 through 6 (of 6 total)

Plugin Author anmari
(@anmari)

12 years ago
Hi there,
just letting you know I’m not ignoring you
First on the ‘spaces’. there is a correction, although I have not yet published the update. (pending a few more tweaks)

Code looks like this:
```
/* 20140305 - add logic to cope ith problem that some csv strings
			have comma space and some not.  we are using that ONLY fields
			requested for filtering will end up in this piece of code and NOT
			descriptive text fields that may have commas in them. */
			if (stristr($user[$col],', ') ){
				$colarr = explode(', ',$user[$col]);
				// for values created by normal build from normal user meta
				// that probably have been imploded with comma space for pretty printing
			}
			else 	{
				$colarr = explode(',',$user[$col]);
				// for values NOT imploded with comma space

			}
```
Plugin Author anmari
(@anmari)

12 years ago

Secondly on the ampersands. When deciding how to treat things, I look at what wordpress does and so far have found:

The wordpress update_user_meta stores ampersands as ‘& a m p ;’ in the database. Try this on a field like last name and you’ll see. Try creating two tags & and ‘& a m p ;’ – both get stored as &

BUT Strangely posts and post titles don’t and post_meta doesn’t….

Looking at the plugin the problem arise with the esc_attr. As the wp codes notes https://codex.wordpress.org/Function_Reference/esc_attr,

“Always use when escaping HTML attributes (especially form values) “

Esc_attr works fine for filtering wordpress user meta values in my list’s (yes I actually did a filter on last name, luckly not too many in my test db!)

But as you have pointed out, esc_attr doesn’t help filter on data that has been inserted without the ampersand escaped. Escaping an ampersand in a mysql db is not strictly necessary – as noted above, sometimes wp does, sometimes wp doesn’t.

sanitize_text works on unescaped ampersand, BUT then NOT on the wp escaped ampersands.

Should the plugin be ready to handle either? probably and if so how? and should an ampersand match an escaped ampersand?

I’m pondering this further.
I have to be careful what/how I change it as I do not want to ‘break’ filtering for other people.

I should have a decision soon.
In the meantime, you could setup S2 so that the values (ie not the lables) do NOT have ampersands.
(Of course then you’d have some add-on coding to do to get those lables to show instead of the values. My plugin does not natively read the S2 options.

Related posts:
http://wpusersplugin.com/2940/filters/ (see last 2 filters)
http://wpusersplugin.com/related-plugins/amr-user-plugin-add-ons/amr-users-plus-s2-member/

You may also want to read
http://wpusersplugin.com/3086/user-lists-and-filtering-with-s2member-fields/
Plugin Author anmari
(@anmari)

12 years ago
Ok, doing little code tests and digging into wp code highlights that normal add/update_user_meta does NOT encode the &,
but certain fields like last_name have kses filter applied:
```
foreach ( array( 'pre_term_name', 'pre_comment_author_name', 'pre_link_name', 'pre_link_target', 'pre_link_rel', 'pre_user_display_name', 'pre_user_first_name', 'pre_user_last_name', 'pre_user_nickname' ) as $filter ) {
	add_filter( $filter, 'sanitize_text_field'  );
	add_filter( $filter, 'wp_filter_kses'       );
	add_filter( $filter, '_wp_specialchars', 30 );
}
```
This (kses) has been flagged as an issue
https://core.trac.wordpress.org/ticket/11311#comment:9
and there are a number of related trac posts.

So possibly esc_attr for the form value display is fine, but one should unencode when processing the values ? (and my dubious last name filter would then fail…),

More later…
Plugin Author anmari
(@anmari)

12 years ago

Update:
so having decided that in this case possibly wordpress is wrong – & should not be stored as & a m p ; in the database. Ampersands being special to html output and urls and not to stored data.

some thoughts:
http://stackoverflow.com/questions/11704233/the-best-way-to-store-ampersand-in-mysql-database

However either way, in wp either may end up in the DB, so the plugin needs to evaluate whether/how to cope. The problem with filtering on values with ampersands is that esc_attr does not double encode and both & and ‘& a m p ;’ end up as ‘& a m p ;’ . So the resulting filter which has to work generically can be seto handle one or the other, but not both with way too much setup complexity (OK for dedicated scripts who can decide which way a particular field will be)

Since the meta plugins I looked at were NOT encoding the ampersand in the DB I decided to change the code to work for the values with unescaped ampersands in DB. (Generally I’d suggest AVOID having a & in your code values – descriptions ok, but not in the code values.)

So the esc_attr was moved closer to the output thus avoiding filter matching issues. This did cause a ripple effect of code changes so I am running series of tests.

This led to the discovery that values with ampersands will break pagination of results after filtering. add_query_arg breaks unescaped ampersand into just the first half before the &. If I escape the value with ampersand then it changes for eg: Family & Friends to Family & Friends&Friends&Friends…. weird…

I found this trac reports:
https://core.trac.wordpress.org/ticket/20771
and logged this
https://core.trac.wordpress.org/ticket/11311#comment:18

In the interim I need to put this update up soon. I will be doing abit more investigation but it seems that I may have to leave that
IF you have ampersands in your code values (not codes can be different from descriptions/lables)
AND you then want to filter on those values
AND you have more than a page of results, the pagination may not work.

In the amr-users plugin you can set the rows per page. So you could try setting it to your max subset to avoid this.
OR Simply be safe and DO not use ampersands in your codes.

Thread Starter ccgjosh
(@ccgjosh)

12 years ago

Thank you for the replies and plugin updates. I agree that the best bet would be to not use ampersands. Thankfully our members in categories which have an ampersand are few so I shouldn’t be running into the pagination problem anytime soon.

Plugin Author anmari
(@anmari)

12 years ago

actually that got fixed too
but still left me with the impression that it’s probably safe when pulling together a site with multiple third party plugins to avoid such things.

Viewing 6 replies - 1 through 6 (of 6 total)

The topic ‘S2, Ampersand problem’ is closed to new replies.