Skip to content

Commit

Permalink
Merge pull request #951 from Automattic/update/sanitization-reporting
Browse files Browse the repository at this point in the history 
Update sanitization reporting
  • Loading branch information
@westonruter
westonruter authored Feb 15, 2018
2 parents 1ab37c6 + 18a877e commit acf84e6
Show file tree
Hide file tree
Showing 6 changed files with 206 additions and 281 deletions.
23 changes: 16 additions & 7 deletions includes/sanitizers/class-amp-base-sanitizer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -314,12 +314,12 @@ public function maybe_enforce_https_src( $src, $force_https = false ) {
* @since 0.7
*
* @param DOMElement $child The node to remove.
* @return void.
* @return void
*/
public function remove_invalid_child( $child ) {
$child->parentNode->removeChild( $child );
if ( isset( $this->args['remove_invalid_callback'] ) ) {
call_user_func( $this->args['remove_invalid_callback'], $child, AMP_Validation_Utils::NODE_REMOVED );
call_user_func( $this->args['remove_invalid_callback'], $child );
}
}

Expand All @@ -331,14 +331,23 @@ public function remove_invalid_child( $child ) {
*
* @since 0.7
*
* @param DOMElement $element The node for which to remove the attribute.
* @param string $attribute The attribute to remove from the node.
* @return void.
* @param DOMElement $element The node for which to remove the attribute.
* @param DOMAttr|string $attribute The attribute to remove from the element.
* @return void
*/
public function remove_invalid_attribute( $element, $attribute ) {
$element->removeAttribute( $attribute );
if ( isset( $this->args['remove_invalid_callback'] ) ) {
call_user_func( $this->args['remove_invalid_callback'], $element, AMP_Validation_Utils::ATTRIBUTE_REMOVED, $attribute );
if ( is_string( $attribute ) ) {
$attribute = $element->getAttributeNode( $attribute );
}
if ( $attribute ) {
$element->removeAttributeNode( $attribute );
call_user_func( $this->args['remove_invalid_callback'], $attribute );
}
} elseif ( is_string( $attribute ) ) {
$element->removeAttribute( $attribute );
} else {
$element->removeAttributeNode( $attribute );
}
}

Expand Down
5 changes: 1 addition & 4 deletions includes/sanitizers/class-amp-tag-and-attribute-sanitizer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -729,10 +729,7 @@ private function sanitize_disallowed_attributes_in_node( $node, $attr_spec_list
}

foreach ( $attrs_to_remove as $attr ) {
$node->removeAttributeNode( $attr );
if ( isset( $this->args['remove_invalid_callback'], $attr->name ) ) {
call_user_func( $this->args['remove_invalid_callback'], $node, AMP_Validation_Utils::ATTRIBUTE_REMOVED, $attr->name );
}
$this->remove_invalid_attribute( $node, $attr );
}
}

Expand Down
258 changes: 89 additions & 169 deletions includes/utils/class-amp-validation-utils.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,6 @@
*/
class AMP_Validation_Utils {

/**
* The argument if an attribute was removed.
*
* @var array.
*/
const ATTRIBUTE_REMOVED = 'removed_attr';

/**
* The argument if a node was removed.
*
* @var array.
*/
const NODE_REMOVED = 'removed';

/**
* Key for the markup value in the REST API endpoint.
*
Expand All @@ -40,86 +26,31 @@ class AMP_Validation_Utils {
*/
const ERROR_KEY = 'has_error';

/**
* Key of the AMP error query var.
*
* @var string.
*/
const ERROR_QUERY_KEY = 'amp_error';

/**
* Query arg value if there is an AMP error in the post content.
*
* @var string.
*/
const ERROR_QUERY_VALUE = '1';

/**
* Nonce name for the editor error message.
*
* @var string.
*/
const ERROR_NONCE = 'amp_nonce';

/**
* Nonce action for displaying the invalid AMP message.
*
* @var string.
*/
const ERROR_NONCE_ACTION = 'invalid_amp_message';

/**
* The attributes that the sanitizer removed.
*
* @var array.
*/
public static $removed_attributes;

/**
* The nodes that the sanitizer removed.
*
* @var array.
* @var DOMNode[]
*/
public static $removed_nodes;
public static $removed_nodes = array();

/**
* Add the actions.
*
* @return void.
* @return void
*/
public static function init() {
add_action( 'rest_api_init', array( __CLASS__, 'amp_rest_validation' ) );
add_action( 'save_post', array( __CLASS__, 'validate_content' ), 10, 2 );
add_action( 'edit_form_top', array( __CLASS__, 'display_error' ) );
}

/**
* Tracks when a sanitizer removes an attribute or node.
*
* @param DOMNode|DOMElement $node The node in which there was a removal.
* @param string $removal_type The removal: 'removed_attr' for an attribute, or 'removed' for a node or element.
* @param string $attr_name The name of the attribute removed (optional).
* @return void.
*/
public static function track_removed( $node, $removal_type, $attr_name = null ) {
if ( ( self::ATTRIBUTE_REMOVED === $removal_type ) && isset( $attr_name ) ) { // phpcs:ignore WordPress.NamingConventions.ValidVariableName.NotSnakeCaseMemberVar
self::$removed_attributes = self::increment_count( self::$removed_attributes, $attr_name );
} elseif ( ( self::NODE_REMOVED === $removal_type ) && isset( $node->nodeName ) ) {
self::$removed_nodes = self::increment_count( self::$removed_nodes, $node->nodeName ); // phpcs:ignore WordPress.NamingConventions.ValidVariableName.NotSnakeCaseMemberVar
}
add_action( 'edit_form_top', array( __CLASS__, 'validate_content' ), 10, 2 );
}

/**
* Tracks when a sanitizer removes an attribute or node.
* Tracks when a sanitizer removes an node (element or attribute).
*
* @param array $histogram The count of attributes or nodes removed.
* @param string $key The attribute or node name removed.
* @return array $histogram The incremented histogram.
* @param DOMNode $node The node which was removed.
* @return void
*/
public static function increment_count( $histogram, $key ) {
$current_value = isset( $histogram[ $key ] ) ? $histogram[ $key ] : 0;
$histogram[ $key ] = $current_value + 1;
return $histogram;
public static function track_removed( $node ) {
self::$removed_nodes[] = $node;
}

/**
Expand All @@ -141,12 +72,19 @@ public static function was_node_removed() {
* @return void.
*/
public static function process_markup( $markup ) {
$args = array(
'content_max_width' => ! empty( $content_width ) ? $content_width : AMP_Post_Template::CONTENT_MAX_WIDTH,
);
if ( self::is_authorized() ) {
$args['remove_invalid_callback'] = 'AMP_Validation_Utils::track_removed';
if ( ! self::has_cap() ) {
return;
}

AMP_Theme_Support::register_content_embed_handlers();
remove_filter( 'the_content', 'wpautop' );

/** This filter is documented in wp-includes/post-template.php */
$markup = apply_filters( 'the_content', $markup );
$args = array(
'content_max_width' => ! empty( $content_width ) ? $content_width : AMP_Post_Template::CONTENT_MAX_WIDTH,
'remove_invalid_callback' => 'AMP_Validation_Utils::track_removed',
);
AMP_Content_Sanitizer::sanitize( $markup, amp_get_content_sanitizers(), $args );
}

Expand Down Expand Up @@ -212,11 +150,34 @@ public static function get_response( $markup = null ) {
self::process_markup( $markup );
$response['processed_markup'] = $markup;
}
$response = array_merge( array(
self::ERROR_KEY => self::was_node_removed(),
'removed_nodes' => self::$removed_nodes,
'removed_attributes' => self::$removed_attributes,
), $response );

$removed_elements = array();
$removed_attributes = array();
foreach ( self::$removed_nodes as $node ) {
if ( $node instanceof DOMAttr ) {
if ( ! isset( $removed_attributes[ $node->nodeName ] ) ) {
$removed_attributes[ $node->nodeName ] = 1;
} else {
$removed_attributes[ $node->nodeName ]++;
}
} elseif ( $node instanceof DOMElement ) {
if ( ! isset( $removed_elements[ $node->nodeName ] ) ) {
$removed_elements[ $node->nodeName ] = 1;
} else {
$removed_elements[ $node->nodeName ]++;
}
}
}

$response = array_merge(
array(
self::ERROR_KEY => self::was_node_removed(),
),
compact(
'removed_elements',
'removed_attributes'
),
$response );
self::reset_removed();

return $response;
Expand All @@ -232,8 +193,7 @@ public static function get_response( $markup = null ) {
* @return void.
*/
public static function reset_removed() {
self::$removed_nodes = null;
self::$removed_attributes = null;
self::$removed_nodes = array();
}

/**
Expand All @@ -250,106 +210,66 @@ public static function validate_arg( $arg ) {
}

/**
* On updating a post, this checks the AMP validity of the content.
* Checks the AMP validity of the post content.
*
* If it's not valid AMP, it adds a query arg to the redirect URL.
* This will cause an error message to appear above the 'Classic' editor.
* If it's not valid AMP,
* it displays an error message above the 'Classic' editor.
*
* @param integer $post_id The ID of the updated post.
* @param WP_Post $post The updated post.
* @param WP_Post $post The updated post.
* @return void.
*/
public static function validate_content( $post_id, $post ) {
unset( $post_id );
if ( ! post_supports_amp( $post ) || ! self::is_authorized() ) {
public static function validate_content( $post ) {
if ( ! post_supports_amp( $post ) || ! self::has_cap() ) {
return;
}
AMP_Theme_Support::register_content_embed_handlers();
/** This filter is documented in wp-includes/post-template.php */
$filtered_content = apply_filters( 'the_content', $post->post_content, $post->ID );
$response = self::get_response( $filtered_content );
if ( isset( $response[ self::ERROR_KEY ] ) && ( true === $response[ self::ERROR_KEY ] ) ) {
add_filter( 'redirect_post_location', function( $location ) use ( $response ) {
$location = AMP_Validation_Utils::error_message( $location );
$location = add_query_arg(
array(
'removed_elements' => array_keys( $response['removed_nodes'] ),
'removed_attributes' => array_keys( $response['removed_attributes'] ),
),
$location
);
return $location;
} );
self::display_error( $response );
}
}

/**
* Whether the current user is authorized.
* Displays an error message on /wp-admin/post.php.
*
* This checks that the user has a certain capability and the nonce is valid.
* It will only return true when updating the post on:
* wp-admin/post.php
* Avoids using check_admin_referer().
* This function might be called in different places,
* and it can't cause it to die() if the nonce is invalid.
*
* @return boolean $is_valid True if the nonce is valid.
*/
public static function is_authorized() {
$nonce = isset( $_REQUEST['_wpnonce'] ) ? sanitize_text_field( wp_unslash( $_REQUEST['_wpnonce'] ) ) : ''; // WPCS: CSRF ok.
return ( self::has_cap() && ( false !== wp_verify_nonce( $nonce, 'update-post_' . get_the_ID() ) ) );
}

/**
* Adds an error message to the URL if it's not valid AMP.
*
* When redirecting after saving a post, the content was validated for AMP compliance.
* If it wasn't valid AMP, this will add a query arg to the URL.
* And an error message will display on /wp-admin/post.php.
*
* @param string $url The URL of the redirect.
* @return string $url The filtered URL, including the AMP error message query var.
*/
public static function error_message( $url ) {
$args = array(
self::ERROR_QUERY_KEY => self::ERROR_QUERY_VALUE,
self::ERROR_NONCE => wp_create_nonce( self::ERROR_NONCE_ACTION ),
);
return add_query_arg( $args, $url );
}

/**
* Displays an error message on /wp-admin/post.php if the saved content is not valid AMP.
*
* Use $_GET, as get_query_var won't return the value.
* This displays at the top of the 'Classic' editor.
* Located at the top of the 'Classic' editor.
* States that the content is not valid AMP.
*
* @param array $response The validation response, an associative array.
* @return void.
*/
public static function display_error() {
if ( ! isset( $_GET[ self::ERROR_QUERY_KEY ] ) ) {
return;
public static function display_error( $response ) {
echo '<div class="notice notice-warning">';
printf( '<p>%s</p>', esc_html__( 'Warning: There is content which fails AMP validation; it will be stripped when served as AMP.', 'amp' ) );
$removed_sets = array();
if ( ! empty( $response['removed_elements'] ) && is_array( $response['removed_elements'] ) ) {
$removed_sets[] = array(
'label' => __( 'Invalid elements:', 'amp' ),
'names' => array_map( 'sanitize_key', $response['removed_elements'] ),
);
}
check_admin_referer( self::ERROR_NONCE_ACTION, self::ERROR_NONCE );
$error = isset( $_GET[ self::ERROR_QUERY_KEY ] ) ? sanitize_text_field( wp_unslash( $_GET[ self::ERROR_QUERY_KEY ] ) ) : ''; // WPCS: CSRF ok.
if ( self::ERROR_QUERY_VALUE === $error ) {
echo '<div class="notice notice-warning">';
printf( '<p>%s</p>', esc_html__( 'Warning: There is content which fails AMP validation; it will be stripped when served as AMP.', 'amp' ) );
if ( ! empty( $_GET['removed_elements'] ) && is_array( $_GET['removed_elements'] ) ) {
printf(
'<p>%s %s</p>',
esc_html__( 'Invalid elements:', 'amp' ),
'<code>' . implode( '</code>, <code>', array_map( 'esc_html', $_GET['removed_elements'] ) ) . '</code>'
);
}
if ( ! empty( $_GET['removed_attributes'] ) ) {
printf(
'<p>%s %s</p>',
esc_html__( 'Invalid attributes:', 'amp' ),
'<code>' . implode( '</code>, <code>', array_map( 'esc_html', $_GET['removed_attributes'] ) ) . '</code>'
);
if ( ! empty( $response['removed_attributes'] ) && is_array( $response['removed_attributes'] ) ) {
$removed_sets[] = array(
'label' => __( 'Invalid attributes:', 'amp' ),
'names' => array_map( 'sanitize_key', $response['removed_attributes'] ),
);
}
foreach ( $removed_sets as $removed_set ) {
printf( '<p>%s ', esc_html( $removed_set['label'] ) );
$items = array();
foreach ( $removed_set['names'] as $name => $count ) {
if ( 1 === intval( $count ) ) {
$items[] = sprintf( '<code>%s</code>', esc_html( $name ) );
} else {
$items[] = sprintf( '<code>%s</code> (%d)', esc_html( $name ), $count );
}
}
echo '</div>';
echo implode( ', ', $items ); // WPCS: XSS OK.
echo '</p>';
}
echo '</div>';
}

}
Loading

0 comments on commit acf84e6

Please sign in to comment.