Refactor storage of validation errors to use taxonomy terms

Gruntfile.js

@@ -100,7 +100,7 @@ module.exports = function( grunt ) {
 			versionAppend = commitHash + '-' + new Date().toISOString().replace( /\.\d+/, '' ).replace( /-|:/g, '' );
 
 			paths = lsOutput.trim().split( /\n/ ).filter( function( file ) {
-				return ! /^(blocks|\.|bin|([^/]+)+\.(md|json|xml)|Gruntfile\.js|tests|wp-assets|dev-lib|readme\.md|composer\..*)/.test( file );
+				return ! /^(blocks|\.|bin|([^/]+)+\.(md|json|xml)|Gruntfile\.js|tests|wp-assets|dev-lib|readme\.md|composer\..*|webpack.*)/.test( file );
 			} );
 			paths.push( 'vendor/autoload.php' );
 			paths.push( 'assets/js/*-compiled.js' );

assets/js/amp-block-validation.js

@@ -172,13 +172,19 @@ var ampBlockValidation = ( function() { // eslint-disable-line no-unused-vars
 				);
 			}
 
-			noticeMessage += ' ' + wp.i18n.__( 'Invalid code is stripped when displaying AMP.', 'amp' );
+			noticeMessage += ' ' + wp.i18n.__( 'Non-accepted validation errors prevent AMP from being served.', 'amp' );
 			noticeElement = wp.element.createElement( 'p', {}, [
 				noticeMessage + ' ',
-				ampValidity.link && wp.element.createElement(
+				ampValidity.review_link && wp.element.createElement(
 					'a',
-					{ key: 'details', href: ampValidity.link, target: '_blank' },
-					wp.i18n.__( 'Details', 'amp' )
+					{ key: 'review_link', href: ampValidity.review_link, target: '_blank' },
+					wp.i18n.__( 'Review issues', 'amp' )
+				),
+				ampValidity.review_link && ' | ',
+				ampValidity.debug_link && wp.element.createElement(
+					'a',
+					{ key: 'debug_link', href: ampValidity.debug_link, target: '_blank' },
+					wp.i18n.__( 'Debug', 'amp' )
 				)
 			] );
 

includes/class-amp-autoloader.php

@@ -89,7 +89,9 @@ class AMP_Autoloader {
 		'AMP_DOM_Utils'                               => 'includes/utils/class-amp-dom-utils',
 		'AMP_HTML_Utils'                              => 'includes/utils/class-amp-html-utils',
 		'AMP_Image_Dimension_Extractor'               => 'includes/utils/class-amp-image-dimension-extractor',
-		'AMP_Validation_Utils'                        => 'includes/utils/class-amp-validation-utils',
+		'AMP_Validation_Manager'                      => 'includes/validation/class-amp-validation-manager',
+		'AMP_Invalid_URL_Post_Type'                   => 'includes/validation/class-amp-invalid-url-post-type',
+		'AMP_Validation_Error_Taxonomy'               => 'includes/validation/class-amp-validation-error-taxonomy',
 		'AMP_String_Utils'                            => 'includes/utils/class-amp-string-utils',
 		'AMP_WP_Utils'                                => 'includes/utils/class-amp-wp-utils',
 		'AMP_Widget_Archives'                         => 'includes/widgets/class-amp-widget-archives',

includes/class-amp-response-headers.php

@@ -63,9 +63,6 @@ public static function send_header( $name, $value, $args = array() ) {
 	 * Send Server-Timing header.
 	 *
 	 * @since 1.0
-	 * @todo What is the ordering in Chrome dev tools? What are the colors about?
-	 * @todo Is there a better name standardization?
-	 * @todo Is there a way to indicate nested server timings, so an outer method's own time can be seen separately from the inner method's time?
 	 *
 	 * @param string $name        Name.
 	 * @param float  $duration    Duration. If negative, will be added to microtime( true ). Optional.
@@ -75,7 +72,7 @@ public static function send_header( $name, $value, $args = array() ) {
 	public static function send_server_timing( $name, $duration = null, $description = null ) {
 		$value = $name;
 		if ( isset( $description ) ) {
-			$value .= sprintf( ';desc=%s', wp_json_encode( $description ) );
+			$value .= sprintf( ';desc="%s"', str_replace( array( '\\', '"' ), '', substr( $description, 0, 100 ) ) );
 		}
 		if ( isset( $duration ) ) {
 			if ( $duration < 0 ) {

includes/class-amp-theme-support.php

@@ -100,8 +100,13 @@ public static function init() {
 			return;
 		}
 
+		// @todo Rename to query var to indicate whether sources should be obtained. It's sources that are needing to be to be conditional.
+		AMP_Validation_Manager::init( array(
+			'locate_sources' => AMP_Validation_Manager::should_validate_response(),
+			'debug'          => isset( $_REQUEST[ AMP_Validation_Manager::DEBUG_QUERY_VAR ] ), // phpcs:ignore WordPress.CSRF.NonceVerification.NoNonceVerification
+		) );
+
 		self::$init_start_time = microtime( true );
-		AMP_Validation_Utils::init();
 
 		self::purge_amp_query_vars();
 		self::handle_xhr_request();
@@ -154,15 +159,20 @@ public static function finish_init() {
 
 		self::add_hooks();
 		self::$sanitizer_classes = amp_get_content_sanitizers();
+		self::$sanitizer_classes = AMP_Validation_Manager::add_validation_callback( self::$sanitizer_classes );
 		self::$embed_handlers    = self::register_content_embed_handlers();
 	}
 
 	/**
 	 * Redirect to canonical URL if the AMP URL was loaded, since canonical is now AMP.
 	 *
 	 * @since 0.7
+	 * @since 1.0 Added $exit param.
+	 * @todo Rename to redirect_non_amp().
+	 *
+	 * @param bool $exit Whether to exit after redirecting.
 	 */
-	public static function redirect_canonical_amp() {
+	public static function redirect_canonical_amp( $exit = true ) {
 		if ( false !== get_query_var( amp_get_slug(), false ) ) { // Because is_amp_endpoint() now returns true if amp_is_canonical().
 			$url = preg_replace( '#^(https?://.+?)(/.*)$#', '$1', home_url( '/' ) );
 			if ( isset( $_SERVER['REQUEST_URI'] ) ) {
@@ -171,8 +181,14 @@ public static function redirect_canonical_amp() {
 
 			$url = amp_remove_endpoint( $url );
 
-			wp_safe_redirect( $url, 302 ); // Temporary redirect because canonical may change in future.
-			exit;
+			/*
+			 * Temporary redirect because AMP URL may return when blocking validation errors
+			 * occur or when a non-canonical AMP theme is used.
+			 */
+			wp_safe_redirect( $url, 302 );
+			if ( $exit ) {
+				exit;
+			}
 		}
 	}
 
@@ -275,11 +291,6 @@ public static function add_hooks() {
 		$priority = defined( 'PHP_INT_MIN' ) ? PHP_INT_MIN : ~PHP_INT_MAX; // phpcs:ignore PHPCompatibility.PHP.NewConstants.php_int_minFound
 		add_action( 'template_redirect', array( __CLASS__, 'start_output_buffering' ), $priority );
 
-		// Add validation hooks *after* output buffering has started for the response.
-		if ( AMP_Validation_Utils::should_validate_response() ) {
-			AMP_Validation_Utils::add_validation_hooks();
-		}
-
 		// Commenting hooks.
 		add_filter( 'wp_list_comments_args', array( __CLASS__, 'set_comments_walker' ), PHP_INT_MAX );
 		add_filter( 'comment_form_defaults', array( __CLASS__, 'filter_comment_form_defaults' ) );
@@ -631,7 +642,7 @@ public static function amend_comment_form() {
 	 * @see get_query_template()
 	 *
 	 * @param array $templates Template hierarchy.
-	 * @returns array Templates.
+	 * @return array Templates.
 	 */
 	public static function filter_paired_template_hierarchy( $templates ) {
 		$support = get_theme_support( 'amp' );
@@ -1012,17 +1023,18 @@ public static function filter_customize_partial_render( $partial ) {
 	 * @since 0.7
 	 *
 	 * @param string $response HTML document response. By default it expects a complete document.
-	 * @param array  $args {
-	 *     Args to send to the preprocessor/sanitizer.
-	 *
-	 *     @type callable $remove_invalid_callback Function to call whenever a node is removed due to being invalid.
-	 * }
+	 * @param array  $args     Args to send to the preprocessor/sanitizer.
 	 * @return string AMP document response.
 	 * @global int $content_width
 	 */
 	public static function prepare_response( $response, $args = array() ) {
 		global $content_width;
 
+		if ( isset( $args['validation_error_callback'] ) ) {
+			_doing_it_wrong( __METHOD__, 'Do not supply validation_error_callback arg.', '1.0' );
+			unset( $args['validation_error_callback'] );
+		}
+
 		/*
 		 * Check if the response starts with HTML markup.
 		 * Without this check, JSON responses will be erroneously corrupted,
@@ -1032,19 +1044,16 @@ public static function prepare_response( $response, $args = array() ) {
 			return $response;
 		}
 
-		$is_validation_debug_mode = isset( $_REQUEST[ AMP_Validation_Utils::DEBUG_QUERY_VAR ] ); // WPCS: csrf ok.
-
 		$args = array_merge(
 			array(
 				'content_max_width'       => ! empty( $content_width ) ? $content_width : AMP_Post_Template::CONTENT_MAX_WIDTH, // Back-compat.
 				'use_document_element'    => true,
 				'allow_dirty_styles'      => self::is_customize_preview_iframe(), // Dirty styles only needed when editing (e.g. for edit shortcodes).
 				'allow_dirty_scripts'     => is_customize_preview(), // Scripts are always needed to inject changeset UUID.
-				'disable_invalid_removal' => $is_validation_debug_mode,
 				'enable_response_caching' => (
 					( ! defined( 'WP_DEBUG' ) || true !== WP_DEBUG )
 					&&
-					! AMP_Validation_Utils::should_validate_response()
+					! AMP_Validation_Manager::should_validate_response()
 				),
 			),
 			$args
@@ -1110,16 +1119,23 @@ public static function prepare_response( $response, $args = array() ) {
 		$dom_serialize_start = microtime( true );
 		self::ensure_required_markup( $dom );
 
+		if ( ! AMP_Validation_Manager::should_validate_response() && AMP_Validation_Manager::has_blocking_validation_errors() ) {
+			if ( amp_is_canonical() ) {
+				$dom->documentElement->removeAttribute( 'amp' );
+			} else {
+				self::redirect_canonical_amp( false );
+				return esc_html__( 'Redirecting to non-AMP version.', 'amp' );
+			}
+		}
+
 		// @todo If 'utf-8' is not the blog charset, then we'll need to do some character encoding conversation or "entityification".
 		if ( 'utf-8' !== strtolower( get_bloginfo( 'charset' ) ) ) {
 			/* translators: %s is the charset of the current site */
 			trigger_error( esc_html( sprintf( __( 'The database has the %s encoding when it needs to be utf-8 to work with AMP.', 'amp' ), get_bloginfo( 'charset' ) ) ), E_USER_WARNING ); // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_trigger_error
 		}
 
-		if ( AMP_Validation_Utils::should_validate_response() ) {
-			AMP_Validation_Utils::finalize_validation( $dom, array(
-				'remove_source_comments' => ! $is_validation_debug_mode,
-			) );
+		if ( AMP_Validation_Manager::should_validate_response() ) {
+			AMP_Validation_Manager::finalize_validation( $dom );
 		}
 
 		$response  = "<!DOCTYPE html>\n";

includes/options/class-amp-options-manager.php

@@ -31,6 +31,7 @@ public static function register_settings() {
 		);
 
 		add_action( 'update_option_' . self::OPTION_NAME, array( __CLASS__, 'maybe_flush_rewrite_rules' ), 10, 2 );
+		add_action( 'admin_notices', array( __CLASS__, 'persistent_object_caching_notice' ) );
 	}
 
 	/**
@@ -259,4 +260,20 @@ public static function update_analytics_options( $data ) {
 		_deprecated_function( __METHOD__, '0.6', __CLASS__ . '::update_option' );
 		return self::update_option( 'analytics', wp_unslash( $data ) );
 	}
+
+	/**
+	 * Outputs an admin notice if persistent object cache is not present.
+	 *
+	 * @return void
+	 */
+	public static function persistent_object_caching_notice() {
+		if ( ! wp_using_ext_object_cache() && 'toplevel_page_' . self::OPTION_NAME === get_current_screen()->id ) {
+			printf(
+				'<div class="notice notice-warning"><p>%s <a href="%s">%s</a></p></div>',
+				esc_html__( 'The AMP plugin performs at its best when persistent object cache is enabled.', 'amp' ),
+				esc_url( 'https://codex.wordpress.org/Class_Reference/WP_Object_Cache#Persistent_Caching' ),
+				esc_html__( 'More details', 'amp' )
+			);
+		}
+	}
 }

includes/sanitizers/class-amp-base-sanitizer.php

@@ -54,7 +54,7 @@ abstract class AMP_Base_Sanitizer {
 	 *      @type array $amp_bind_placeholder_prefix
 	 *      @type bool $allow_dirty_styles
 	 *      @type bool $allow_dirty_scripts
-	 *      @type bool $disable_invalid_removal
+	 *      @type bool $locate_sources
 	 *      @type callable $validation_error_callback
 	 * }
 	 */
@@ -290,20 +290,16 @@ public function maybe_enforce_https_src( $src, $force_https = false ) {
 	 *
 	 * @since 0.7
 	 *
-	 * @param DOMNode|DOMElement $node The node to remove.
-	 * @param array              $args Additional args to pass to validation error callback.
-	 *
-	 * @return void
+	 * @param DOMNode|DOMElement $node             The node to remove.
+	 * @param array              $validation_error Validation error details.
+	 * @return bool Whether the node should have been removed, that is, that the node was sanitized for validity.
 	 */
-	public function remove_invalid_child( $node, $args = array() ) {
-		if ( isset( $this->args['validation_error_callback'] ) ) {
-			call_user_func( $this->args['validation_error_callback'],
-				array_merge( compact( 'node' ), $args )
-			);
-		}
-		if ( empty( $this->args['disable_invalid_removal'] ) ) {
+	public function remove_invalid_child( $node, $validation_error = array() ) {
+		$should_remove = $this->should_sanitize_validation_error( $validation_error, compact( 'node' ) );
+		if ( $should_remove ) {
 			$node->parentNode->removeChild( $node );
 		}
+		return $should_remove;
 	}
 
 	/**
@@ -316,34 +312,97 @@ public function remove_invalid_child( $node, $args = array() ) {
 	 *
 	 * @param DOMElement     $element   The node for which to remove the attribute.
 	 * @param DOMAttr|string $attribute The attribute to remove from the element.
-	 * @param array          $args      Additional args to pass to validation error callback.
-	 * @return void
+	 * @param array          $validation_error Validation error details.
+	 * @return bool Whether the node should have been removed, that is, that the node was sanitized for validity.
+	 */
+	public function remove_invalid_attribute( $element, $attribute, $validation_error = array() ) {
+		if ( is_string( $attribute ) ) {
+			$node = $element->getAttributeNode( $attribute );
+		} else {
+			$node = $attribute;
+		}
+		$should_remove = $this->should_sanitize_validation_error( $validation_error, compact( 'node' ) );
+		if ( $should_remove ) {
+			$element->removeAttributeNode( $node );
+		}
+		return $should_remove;
+	}
+
+	/**
+	 * Call the validation_error_callback.
+	 *
+	 * Check whether or not sanitization should occur in response to validation error.
+	 *
+	 * @since 1.0
+	 *
+	 * @todo Each sanitizer needs a $locate_sources arg.
+	 *
+	 * @param array $validation_error Validation error.
+	 * @param array $data             Data including the node.
+	 * @return bool Whether to sanitize.
+	 */
+	public function should_sanitize_validation_error( $validation_error, $data = array() ) {
+		if ( empty( $this->args['validation_error_callback'] ) || ! is_callable( $this->args['validation_error_callback'] ) ) {
+			return true;
+		}
+		$validation_error = $this->prepare_validation_error( $validation_error, $data );
+		return false !== call_user_func( $this->args['validation_error_callback'], $validation_error, $data );
+	}
+
+	/**
+	 * Prepare validation error.
+	 *
+	 * @param array $error {
+	 *     Error.
+	 *
+	 *     @type string $code Error code.
+	 * }
+	 * @param array $data {
+	 *     Data.
+	 *
+	 *     @type DOMElement|DOMNode $node The removed node.
+	 * }
+	 * @return array Error.
 	 */
-	public function remove_invalid_attribute( $element, $attribute, $args = array() ) {
-		if ( isset( $this->args['validation_error_callback'] ) ) {
-			if ( is_string( $attribute ) ) {
-				$attribute = $element->getAttributeNode( $attribute );
+	public function prepare_validation_error( array $error = array(), array $data = array() ) {
+		$node    = null;
+		$matches = null;
+
+		if ( isset( $data['node'] ) && $data['node'] instanceof DOMNode ) {
+			$node = $data['node'];
+
+			$error['node_name'] = $node->nodeName;
+			if ( $node->parentNode ) {
+				$error['parent_name'] = $node->parentNode->nodeName;
 			}
-			if ( $attribute ) {
-				call_user_func( $this->args['validation_error_callback'],
-					array_merge(
-						array(
-							'node' => $attribute,
-						),
-						$args
-					)
-				);
-				if ( empty( $this->args['disable_invalid_removal'] ) ) {
-					$element->removeAttributeNode( $attribute );
-				}
+		}
+
+		if ( $node instanceof DOMElement ) {
+			if ( ! isset( $error['code'] ) ) {
+				$error['code'] = AMP_Validation_Error_Taxonomy::INVALID_ELEMENT_CODE;
+			}
+			$error['node_attributes'] = array();
+			foreach ( $node->attributes as $attribute ) {
+				$error['node_attributes'][ $attribute->nodeName ] = $attribute->nodeValue;
+			}
+
+			// Capture script contents.
+			if ( 'script' === $node->nodeName && ! $node->hasAttribute( 'src' ) ) {
+				$error['text'] = $node->textContent;
+			}
+		} elseif ( $node instanceof DOMAttr ) {
+			if ( ! isset( $error['code'] ) ) {
+				$error['code'] = AMP_Validation_Error_Taxonomy::INVALID_ATTRIBUTE_CODE;
 			}
-		} elseif ( empty( $this->args['disable_invalid_removal'] ) ) {
-			if ( is_string( $attribute ) ) {
-				$element->removeAttribute( $attribute );
-			} else {
-				$element->removeAttributeNode( $attribute );
+			$error['element_attributes'] = array();
+			if ( $node->parentNode && $node->parentNode->hasAttributes() ) {
+				foreach ( $node->parentNode->attributes as $attribute ) {
+					$error['element_attributes'][ $attribute->nodeName ] = $attribute->nodeValue;
+				}
 			}
 		}
+
+		return $error;
 	}
 
 	/**