Searchfrom value is double escaped #122

Closed
wants to merge 1 commit into from

6 participants

@raldenhoven

There is no need to esc_attr(get_search_query) since the default behavior of the function is already escaped.

/* See wp-includes/general-template.php, line 1827 */
function get_search_query( $escaped = true ) {
    $query = apply_filters( 'get_search_query', get_query_var( 's' ) );
    if ( $escaped )
        $query = esc_attr( $query );
    return $query;
}
@kovshenin

I disagree. I believe it's best practice to escape as late as possible, so in this example, I don't have to think about whether get_search_query escapes the output or not, because I know I am and that's all that matters. I think there can never be too much escaping :)

@ashfame

+1 to keep it, as that educate developers too.

@philiparthurmoore

I strongly agree with @kovshenin. Not only does this reduce the stress required to figure out if the attribute escaping happens in core or not, but it also encourages proper practices in the community.

Voting to close this request.

@raldenhoven

In my opinion there should be the least ammount of coding should be done in the template.
Other solution could be: get_search_query($escaped = true); so you keep educating but not double running a ascape on a string.

And @philiparthurmoore a wordpress theme should be build on the wordpress core not teaching people how to proper use php.

@kovshenin

In my opinion there should be the least ammount of coding

Shorter doesn't always mean better, besides get_search_query( $escaped = true ); is 5 bytes longer than the original esc_attr way, and doesn't really educate developers about how they should escape attributes in their code :)

@mfields

I totally agree with @kovshenin on this and suggest that we close this as a duplicate of #52

@ianstewart ianstewart closed this Dec 14, 2012
@ianstewart

Votes and suggestion taken in, ticket closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment