Twenty Twelve: twentytwelve_content_nav $nav_id is not validated.

[ounziw]

In functions.php of TwentyTwelve Theme, function "twentytwelve_content_nav" is defined.

twentytwelve_content_nav takes a parameter called $nav_id. $nav_id is echoed without validated nor escaped.

When careless people write a code like twentytwelve_content_nav( 'nav below' ), it breaks HTML's rule.

function twentytwelve_content_nav( $nav_id ) {
	global $wp_query;

	if ( $wp_query->max_num_pages > 1 ) : ?>
		<nav id="<?php echo $nav_id; ?>" class="navigation" role="navigation">
...

I propose to add

$nav_id = esc_attr( str_replace(' ','',$nav_id ) );

at the beginning of the function definition.

Would something like sanitize_key() make sense? Since you can't have spaces/multiple IDs anyway.

Or sanitize_html_class()?

22690.2.patch​ uses sanitize_html_class().

Looks like the only difference between sanitize_html_class() and sanitize_key() is that the former A) allows for a fallback value, B) has a filter, C) strips octets. They use the same sanitization.

It's possible that in the future, sanitize_html_class() is expanded to all characters possible in a class, which is slightly different than what is allowed in an ID.

sanitize_key() seems fine here. But, either function could break a hypothetically valid ID already in use. "nav below" is not a valid ID. Perhaps we rename the argument from $nav_id to $html_id and then just drop esc_attr() in. There is only so much we should do to prevent someone from shooting themselves in the foot. Eventually they're just going to do it.

Note: Twenty Eleven needs the change, also.

In 22999:

Twenty Twelve: escape navigation ID output, props nacin. See #22690.

In 23000:

Twenty Eleven: escape navigation ID output, closes #22690.