Skip to content

Commit

Permalink
Issue #842: Display warning on plugin activation.
Browse files Browse the repository at this point in the history 
Display a notice on /wp-admin/plugins.php,
when a plugin has invalide markup on the home page.
@todos include improving validate_home().
There's now an error locally,
with a self-signed SSL certificate.
  • Loading branch information
Ryan Kienstra committed Feb 28, 2018
1 parent 0263e58 commit 4d0569f
Show file tree
Hide file tree
Showing 5 changed files with 241 additions and 76 deletions.
2 changes: 1 addition & 1 deletion amp.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,9 @@ function amp_init() {

add_rewrite_endpoint( AMP_QUERY_VAR, EP_PERMALINK );

AMP_Validation_Utils::init();
AMP_Theme_Support::init();
AMP_Post_Type_Support::add_post_type_support();
AMP_Validation_Utils::init();
add_filter( 'request', 'amp_force_query_var_value' );
add_action( 'admin_init', 'AMP_Options_Manager::register_settings' );
add_action( 'wp_loaded', 'amp_post_meta_box' );
Expand Down
4 changes: 2 additions & 2 deletions includes/sanitizers/class-amp-base-sanitizer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -393,9 +393,9 @@ public function capture_current_source( $node ) {
return;
}
if ( ! empty( $matches['closing'] ) ) {
array_pop( $this->current_sources );
} else {
$this->current_sources[] = wp_array_slice_assoc( $matches, array( 'type', 'name' ) );
} else {
array_pop( $this->current_sources );
}
}
}
150 changes: 114 additions & 36 deletions includes/utils/class-amp-validation-utils.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,11 @@ class AMP_Validation_Utils {
const POST_TYPE_SLUG = 'amp_validation_error';

/**
* The key in the response for the plugins that have invalid output.
* The key in the response for the sources that have invalid output.
*
* @var string
*/
const PLUGINS_INVALID_OUTPUT = 'plugins_with_invalid_output';
const SOURCES_INVALID_OUTPUT = 'sources_with_invalid_output';

/**
* The meta key for the primary AMP URL where the error occurred.
Expand All @@ -76,11 +76,14 @@ class AMP_Validation_Utils {
public static $removed_nodes = array();

/**
* The plugins that have had nodes removed.
* The sources that have had nodes removed.
*
* @var array
* @var array[][] {
* @type string $name The name of the source.
* @type string $type The type of the source.
* }
*/
public static $plugins_removed_nodes = array();
public static $sources_removed_nodes = array();

/**
* Add the actions.
Expand All @@ -92,22 +95,41 @@ public static function init() {
add_action( 'edit_form_top', array( __CLASS__, 'validate_content' ), 10, 2 );
add_action( 'wp', array( __CLASS__, 'callback_wrappers' ) );
add_filter( 'amp_content_sanitizers', array( __CLASS__, 'add_validation_callback' ) );
add_filter( 'init', array( __CLASS__, 'register_post_type' ) );
add_action( 'init', array( __CLASS__, 'register_post_type' ) );
add_action( 'activate_plugin', array( __CLASS__, 'validate_home' ) );
add_action( 'all_admin_notices', array( __CLASS__, 'plugin_notice' ) );
}

/**
* Tracks when a sanitizer removes a node (element or attribute).
*
* @param DOMNode $node The node which was removed.
* @param array $sources The plugins and/or themes of the removed elements.
* @param array $removed {
* The data of the removed node.
*
* @type DOMElement|DOMNode $node The removed node.
* @type DOMElement|DOMNode $parent The parent of the removed node.
* @type array[][] $sources {
* The data of the removed sources (theme, plugin, or mu-plugin).
*
* @type string $name The name of the source.
* @type string $type The type of the source.
* }
* }
* @return void
*/
public static function track_removed( $node, $sources = array() ) {
self::$removed_nodes[] = $node;
$source = array_pop( $sources );
if ( isset( $source ) && ! in_array( $source, self::$plugins_removed_nodes, true ) ) {
self::$plugins_removed_nodes[] = $source;
public static function track_removed( $removed ) {
$source = ! empty( $removed['sources'] ) ? array_pop( $removed['sources'] ) : null;
if ( isset( $source['name'], $source['type'] ) ) {
$removed['sources'] = $source;
$name = $source['name'];
$type = $source['type'];
if ( ! isset( self::$sources_removed_nodes[ $type ] ) ) {
self::$sources_removed_nodes[ $type ] = array( $name );
} elseif ( is_array( self::$sources_removed_nodes[ $type ] ) && ( ! in_array( $name, self::$sources_removed_nodes[ $type ], true ) ) ) {
self::$sources_removed_nodes[ $type ][] = $name;
}
}
self::$removed_nodes[] = $removed;
}

/**
Expand Down Expand Up @@ -202,13 +224,16 @@ public static function validate_markup( WP_REST_Request $request ) {
* @return array $response The AMP validity of the markup.
*/
public static function get_response( $markup = null ) {
global $wp;
$response = array();
if ( isset( $markup ) ) {
self::process_markup( $markup );
$response['processed_markup'] = $markup;
} elseif ( isset( $wp ) ) {
$response['url'] = add_query_arg( $wp->query_string, '', home_url( $wp->request ) );
}
if ( self::should_validate_front_end() ) {
$response[ self::PLUGINS_INVALID_OUTPUT ] = self::$plugins_removed_nodes;
$response[ self::SOURCES_INVALID_OUTPUT ] = self::$sources_removed_nodes;
}

$removed_elements = array();
Expand All @@ -233,7 +258,6 @@ public static function get_response( $markup = null ) {
$response = array_merge(
array(
self::ERROR_KEY => self::was_node_removed(),
'url' => get_permalink(),
),
compact(
'removed_elements',
Expand All @@ -256,7 +280,7 @@ public static function get_response( $markup = null ) {
*/
public static function reset_removed() {
self::$removed_nodes = array();
self::$plugins_removed_nodes = array();
self::$sources_removed_nodes = array();
}

/**
Expand Down Expand Up @@ -367,15 +391,15 @@ public static function get_source( $callback ) {
return null;
}


$file = isset( $reflection ) ? $reflection->getFileName() : null;
if ( ! isset( $file ) ) {
return null;
}

preg_match( ':' . wp_normalize_path( WP_PLUGIN_DIR ) . '([^/]+)/:s', $file, $plugin_matches );
preg_match( ':' . wp_normalize_path( get_theme_root() ) . '/([^/]+)/:s', $file, $theme_matches );
preg_match( ':' . wp_normalize_path( WPMU_PLUGIN_DIR ) . '/([^/]+)/:s', $file, $mu_plugin_matches );
$closing_pattern = '([^/]+)/:s';
preg_match( ':' . trailingslashit( wp_normalize_path( WP_PLUGIN_DIR ) ) . $closing_pattern, $file, $plugin_matches );
preg_match( ':' . trailingslashit( wp_normalize_path( get_theme_root() ) ) . $closing_pattern, $file, $theme_matches );
preg_match( ':' . trailingslashit( wp_normalize_path( WPMU_PLUGIN_DIR ) ) . $closing_pattern, $file, $mu_plugin_matches );

if ( isset( $plugin_matches[1] ) ) {
$type = 'plugin';
Expand Down Expand Up @@ -483,8 +507,7 @@ public static function display_error( $response ) {
* @return boolean
*/
public static function should_validate_front_end() {
$post = get_post();
return ( isset( $post ) && post_supports_amp( $post ) && self::has_cap() && ( isset( $_GET[ self::VALIDATION_QUERY_VAR ] ) ) ); // WPCS: CSRF ok.
return ( self::has_cap() && ( isset( $_GET[ self::VALIDATION_QUERY_VAR ] ) ) ); // WPCS: CSRF ok.
}

/**
Expand Down Expand Up @@ -524,13 +547,17 @@ public static function register_post_type() {
/**
* Stores the validation errors.
*
* After the preprocessors run, this gets the validation response.
* After the preprocessors run, this gets the validation response if the query var is present.
* It then stores the response in a custom post type.
* If there's already an error post, but there's no error anymore, it deletes the post.
* If there's already an error post for the URL, but there's no error anymore, it deletes it.
*
* @return int|null $post_id The post ID of the custom post type used, or null.
*/
public static function store_validation_errors() {
if ( ! self::should_validate_front_end() ) {
return null;
}

$response = self::get_response();
$url = isset( $response['url'] ) ? $response['url'] : null;
unset( $response['url'] );
Expand All @@ -541,12 +568,12 @@ public static function store_validation_errors() {
'post_type' => self::POST_TYPE_SLUG,
'post_name' => $post_name,
'post_content' => $encoded_errors,
'post_status' => 'publish',
);
$existing_post_id = self::existing_post( $url );

if ( isset( $existing_post_id ) ) {
// A post for the $url already exists.
if ( empty( $response[ self::PLUGINS_INVALID_OUTPUT ] ) ) {
if ( empty( $response[ self::SOURCES_INVALID_OUTPUT ] ) ) {
wp_delete_post( $existing_post_id, true );
return null;
} else {
Expand All @@ -559,7 +586,7 @@ public static function store_validation_errors() {
}
return $existing_post_id;
} elseif ( isset( $different_post_same_error->ID ) ) {
// The error is already stored somewhere, so append it to the meta.
// The error is already stored somewhere, so append it to the post meta.
$updated_meta = add_post_meta( $different_post_same_error->ID, self::URLS_VALIDATION_ERROR, $url, true );
if ( ! $updated_meta ) {
$meta = get_post_meta( $different_post_same_error->ID, self::URLS_VALIDATION_ERROR, true );
Expand All @@ -568,10 +595,18 @@ public static function store_validation_errors() {
update_post_meta( $different_post_same_error->ID, self::URLS_VALIDATION_ERROR, $meta );
}
return $different_post_same_error->ID;
} elseif ( ! empty( $response[ self::PLUGINS_INVALID_OUTPUT ] ) ) {
} elseif ( ! empty( $response[ self::SOURCES_INVALID_OUTPUT ] ) ) {
// There are validation issues from a plugin, but no existing post for them, so create one.
$new_post_id = wp_insert_post( $post_args );
update_post_meta( $new_post_id, self::AMP_URL_META, $url );
$new_post_id = wp_insert_post(
array_merge(
array(
'meta_input' => array(
self::AMP_URL_META => $url,
),
),
$post_args
)
);
return $new_post_id;
}

Expand All @@ -585,23 +620,66 @@ public static function store_validation_errors() {
* @return int|null $post_id The post ID of the existing custom post, or null.
*/
public static function existing_post( $url ) {
$meta_query = new WP_Query( array(
$query = new WP_Query( array(
'post_type' => self::POST_TYPE_SLUG,
'meta_query' => array(
array(
'key' => self::AMP_URL_META,
'value' => $url,
'compare' => '=',
'key' => self::AMP_URL_META,
'value' => $url,
),
),
) );

if ( $meta_query->have_posts() ) {
$posts = $meta_query->get_posts();
if ( $query->have_posts() ) {
$posts = $query->get_posts();
$post = reset( $posts );
return $post->ID;
}
return null;
}

/**
* Validate the home page.
*
* @return WP_Error|array The response array, or WP_Error.
*/
public static function validate_home() {
return wp_remote_get( add_query_arg(
self::VALIDATION_QUERY_VAR,
1,
get_home_url()
) );
}

/**
* On activating a plugin, display a notice if a plugin causes an AMP validation error.
*
* @return void
*/
public static function plugin_notice() {
global $pagenow;
if ( ( 'plugins.php' === $pagenow ) && isset( $_GET['activate'] ) && ( 'true' === wp_unslash( $_GET['activate'] ) ) ) { // WPCS: CSRF ok.
$home_errors = self::existing_post( get_home_url() );
if ( ! is_int( $home_errors ) ) {
return;
}
$error_post = get_post( $home_errors );
if ( ! isset( $error_post->post_content ) ) {
return;
}
$errors = json_decode( $error_post->post_content, true );
$invalid_plugins = isset( $errors[ self::SOURCES_INVALID_OUTPUT ]['plugin'] ) ? $errors[ self::SOURCES_INVALID_OUTPUT ]['plugin'] : null;
if ( isset( $invalid_plugins ) ) {
echo '<div class="notice notice-warning"><p>';
$reported_plugins = array();
foreach ( $invalid_plugins as $plugin ) {
$reported_plugins[] = sprintf( '<code>%s</code>', esc_html( $plugin ) );
}
esc_html_e( 'Warning: the following plugins are incompatible with AMP: ', 'amp' );
echo implode( ', ', $reported_plugins ); // WPCS: XSS ok.
echo '</p></div>';
}
}
}

}
22 changes: 12 additions & 10 deletions tests/test-class-amp-base-sanitizer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@ public function test_sanitize_dimension( $source_params, $expected_value, $args
* @covers AMP_Base_Sanitizer::remove_invalid_child()
*/
public function test_remove_child() {
AMP_Validation_Utils::reset_removed();
$parent_tag_name = 'div';
$dom_document = new DOMDocument( '1.0', 'utf-8' );
$parent = $dom_document->createElement( $parent_tag_name );
Expand Down Expand Up @@ -351,7 +352,7 @@ public function test_remove_attribute() {
*/
public function test_capture_current_source() {
$dom = new DOMDocument();
$node = $dom->createComment( 'plugin:amp' );
$node = $dom->createComment( '/plugin:amp' );
$sanitizer = new AMP_Tag_And_Attribute_Sanitizer( $dom );
$this->assertEquals( array(), $sanitizer->current_sources );
$sanitizer->capture_current_source( $node );
Expand All @@ -371,18 +372,19 @@ public function test_capture_current_source() {
$sanitizer->current_sources[0]
);

$expected_sources = array(
$amp_source,
$foo_source,
);

$node = $dom->createComment( 'theme:foo' );
$node = $dom->createComment( '/theme:foo' );
$sanitizer->capture_current_source( $node );
$this->assertEquals( $expected_sources, $sanitizer->current_sources );
$this->assertEquals(
array(
$amp_source,
$foo_source,
),
$sanitizer->current_sources
);

$sanitizer->capture_current_source( $dom->createComment( '/theme:foo' ) );
$sanitizer->capture_current_source( $dom->createComment( 'theme:foo' ) );
$this->assertEquals( array( $amp_source ), $sanitizer->current_sources );
$sanitizer->capture_current_source( $dom->createComment( '/plugin:amp' ) );
$sanitizer->capture_current_source( $dom->createComment( 'plugin:amp' ) );
$this->assertEquals( array(), $sanitizer->current_sources );
}

Expand Down
Loading

0 comments on commit 4d0569f

Please sign in to comment.