get_search_query() can be confusing as it doesn't sanitize

[Viper007Bond]

the_search_query() is the recommended way to display what a user searched for. But what if you need the_search_query()'s output for use in PHP, i.e. the value returned? get_search_query() seems like the correct function to use, but they differ in one very important way -- get_search_query() does not escape it's output at all.

It's an easy mistake as most get_ functions are identical to their echo'ing counterparts and most users don't realize the difference. This can easily result in a XSS attack.

I'm not sure what the solution to this is, but there should be an easier way to get a safe search query than having the user call esc_attr(), get_search_query(), etc.

Perhaps deprecated get_search_query() and introduce get_the_search_query or something.

[Viper007Bond]

As proof, this common mistake was even made in the Twenty Ten theme: #12781

[nacin]

Deprecating it for get_the_search_query() doesn't do much good. We can't even get plugin authors to obey the deprecated API.

I suggest we break back compat here and escape it. The Codex is wrong, Twenty Ten is wrong, many many themes are inviting XSS.

If anyone wants the unescaped value, they can call the query var themselves.

[Viper007Bond]

Replying to nacin:

Deprecating it for get_the_search_query() doesn't do much good. We can't even get plugin authors to obey the deprecated API.

I suggest we break back compat here and escape it. The Codex is wrong, Twenty Ten is wrong, many many themes are inviting XSS.

If anyone wants the unescaped value, they can call the query var themselves.

That sounds like an acceptable solution to me.

[nacin]

Making sure we're at least back compat with filters, and never passing already-escaped values to a filter. Also modify instances in core.

[nacin]

(In [13978]) Have get_search_query() escape by default, like it's echoing counterpart the_search_query(). see #12780

[nacin]

Checking this in and leaving open for now. Since esc_attr and attribute_escape don't double-escape, this won't have any adverse effects when the API was used. We're only breaking htmlspecialchars here.