Fetch (local) stylesheets with @import instead of removing

contributing.md

@@ -39,7 +39,7 @@ TO create a build of the plugin as it will be deployed to WordPress.org, run:
 npm run build-release
 ```
 
-Note that this will currently take much longer than a regular build because it generates the files required for translation. You also must have WP-CLI installed with the `i18n` package.
+Note that this will currently take much longer than a regular build because it generates the files required for translation. You also must have WP-CLI installed with the [`i18n-command` package](https://github.com/wp-cli/i18n-command).
 
 ## Updating Allowed Tags And Attributes
 

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

@@ -77,6 +77,13 @@ abstract class AMP_Base_Sanitizer {
 	 */
 	protected $root_element;
 
+	/**
+	 * Keep track of nodes that should not be removed to prevent duplicated validation errors since sanitization is rejected.
+	 *
+	 * @var array
+	 */
+	private $should_not_removed_nodes = array();
+
 	/**
 	 * AMP_Base_Sanitizer constructor.
 	 *
@@ -295,9 +302,17 @@ public function maybe_enforce_https_src( $src, $force_https = false ) {
 	 * @return bool Whether the node should have been removed, that is, that the node was sanitized for validity.
 	 */
 	public function remove_invalid_child( $node, $validation_error = array() ) {
+
+		// Prevent double-reporting nodes that are rejected for sanitization.
+		if ( isset( $this->should_not_removed_nodes[ $node->nodeName ] ) && in_array( $node, $this->should_not_removed_nodes[ $node->nodeName ], true ) ) {
+			return false;
+		}
+
 		$should_remove = $this->should_sanitize_validation_error( $validation_error, compact( 'node' ) );
 		if ( $should_remove ) {
 			$node->parentNode->removeChild( $node );
+		} else {
+			$this->should_not_removed_nodes[ $node->nodeName ][] = $node;
 		}
 		return $should_remove;
 	}
@@ -404,7 +419,7 @@ public function prepare_validation_error( array $error = array(), array $data =
 	/**
 	 * Get data-amp-* values from the parent node 'figure' added by editor block.
 	 *
-	 * @param DOMNode $node Base node.
+	 * @param DOMElement $node Base node.
 	 * @return array AMP data array.
 	 */
 	public function get_data_amp_attributes( $node ) {
@@ -445,9 +460,9 @@ public function filter_data_amp_attributes( $attributes, $amp_data ) {
 	/**
 	 * Set attributes to node's parent element according to layout.
 	 *
-	 * @param DOMNode $node Node.
-	 * @param array   $new_attributes Attributes array.
-	 * @param string  $layout Layout.
+	 * @param DOMElement $node Node.
+	 * @param array      $new_attributes Attributes array.
+	 * @param string     $layout Layout.
 	 * @return array New attributes.
 	 */
 	public function filter_attachment_layout_attributes( $node, $new_attributes, $layout ) {

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

@@ -184,6 +184,13 @@ class AMP_Style_Sanitizer extends AMP_Base_Sanitizer {
 	 */
 	private $calc_placeholders = array();
 
+	/**
+	 * Log of the stylesheet URLs that have been imported to guard against infinite loops.
+	 *
+	 * @var array
+	 */
+	private $processed_imported_stylesheet_urls = array();
+
 	/**
 	 * Get error codes that can be raised during parsing of CSS.
 	 *
@@ -201,6 +208,9 @@ public static function get_css_parser_validation_error_codes() {
 			'illegal_css_property',
 			'removed_unused_css_rules',
 			'unrecognized_css',
+			'disallowed_external_file_url',
+			'disallowed_file_extension',
+			'file_path_not_found',
 		);
 	}
 
@@ -600,8 +610,8 @@ private function process_link_element( DOMElement $element ) {
 	 *
 	 *     @type string[] $property_whitelist          Exclusively-allowed properties.
 	 *     @type string[] $property_blacklist          Disallowed properties.
-	 *     @type string   $stylesheet_url              Original URL for stylesheet when originating via link (or @import?).
-	 *     @type string   $stylesheet_path             Original filesystem path for stylesheet when originating via link (or @import?).
+	 *     @type string   $stylesheet_url              Original URL for stylesheet when originating via link or @import.
+	 *     @type string   $stylesheet_path             Original filesystem path for stylesheet when originating via link or @import.
 	 *     @type array    $allowed_at_rules            Allowed @-rules.
 	 *     @type bool     $validate_keyframes          Whether keyframes should be validated.
 	 * }
@@ -610,7 +620,7 @@ private function process_link_element( DOMElement $element ) {
 	private function process_stylesheet( $stylesheet, $options = array() ) {
 		$parsed      = null;
 		$cache_key   = null;
-		$cache_group = 'amp-parsed-stylesheet-v5';
+		$cache_group = 'amp-parsed-stylesheet-v6';
 
 		$cache_impacting_options = array_merge(
 			wp_array_slice_assoc(
@@ -659,6 +669,82 @@ private function process_stylesheet( $stylesheet, $options = array() ) {
 		return $parsed['stylesheet'];
 	}
 
+	/**
+	 * Parse imported stylesheet.
+	 *
+	 * @param Import  $item     Import object.
+	 * @param CSSList $css_list CSS List.
+	 * @param array   $options {
+	 *     Options.
+	 *
+	 *     @type string $stylesheet_url Original URL for stylesheet when originating via link or @import.
+	 * }
+	 * @return array Validation results.
+	 */
+	private function parse_import_stylesheet( Import $item, CSSList $css_list, $options ) {
+		$results      = array();
+		$at_rule_args = $item->atRuleArgs();
+		$location     = array_shift( $at_rule_args );
+		$media_query  = array_shift( $at_rule_args );
+
+		if ( isset( $options['stylesheet_url'] ) ) {
+			$this->real_path_urls( array( $location ), $options['stylesheet_url'] );
+		}
+
+		$import_stylesheet_url = $location->getURL()->getString();
+
+		// Prevent importing something that has already been imported, and avoid infinite recursion.
+		if ( isset( $this->processed_imported_stylesheet_urls[ $import_stylesheet_url ] ) ) {
+			$css_list->remove( $item );
+			return array();
+		}
+		$this->processed_imported_stylesheet_urls[ $import_stylesheet_url ] = true;
+
+		// @todo Handle external per <https://github.com/Automattic/amp-wp/issues/1083>.
+		$css_file_path = $this->get_validated_url_file_path( $import_stylesheet_url, array( 'css', 'less', 'scss', 'sass' ) );
+
+		if ( is_wp_error( $css_file_path ) ) {
+			$error     = array(
+				'code'    => $css_file_path->get_error_code(),
+				'message' => $css_file_path->get_error_message(),
+			);
+			$sanitized = $this->should_sanitize_validation_error( $error );
+			if ( $sanitized ) {
+				$css_list->remove( $item );
+			}
+			$results[] = compact( 'error', 'sanitized' );
+			return $results;
+		}
+
+		// Load the CSS from the filesystem.
+		$stylesheet = file_get_contents( $css_file_path ); // phpcs:ignore -- It's a local filesystem path not a remote request.
+
+		if ( $media_query ) {
+			$stylesheet = sprintf( '@media %s { %s }', $media_query, $stylesheet );
+		}
+
+		$stylesheet = $this->process_stylesheet( $stylesheet, array(
+			'allowed_at_rules'   => $this->style_custom_cdata_spec['css_spec']['allowed_at_rules'],
+			'property_whitelist' => $this->style_custom_cdata_spec['css_spec']['allowed_declarations'],
+			'stylesheet_url'     => $import_stylesheet_url,
+			'stylesheet_path'    => $css_file_path,
+		) );
+
+		$pending_stylesheet = array(
+			'keyframes'  => false,
+			'stylesheet' => $stylesheet,
+			'node'       => $this->current_node,
+			'sources'    => $this->current_sources,
+			'imported'   => $import_stylesheet_url,
+		);
+
+		// Stylesheet will now be inlined, so @import can be removed.
+		$css_list->remove( $item );
+
+		$this->pending_stylesheets[] = $pending_stylesheet;
+		return $results;
+	}
+
 	/**
 	 * Parse stylesheet.
 	 *
@@ -971,12 +1057,10 @@ private function process_css_list( CSSList $css_list, $options ) {
 					);
 				}
 			} elseif ( $css_item instanceof Import ) {
-				$error     = array(
-					'code'    => 'illegal_css_at_rule',
-					'at_rule' => 'import',
+				$results = array_merge(
+					$results,
+					$this->parse_import_stylesheet( $css_item, $css_list, $options )
 				);
-				$sanitized = $this->should_sanitize_validation_error( $error );
-				$results[] = compact( 'error', 'sanitized' );
 			} elseif ( $css_item instanceof AtRuleSet ) {
 				if ( ! in_array( $css_item->atRuleName(), $options['allowed_at_rules'], true ) ) {
 					$error     = array(
@@ -1468,21 +1552,32 @@ private function finalize_styles() {
 			),
 		);
 
+		/*
+		 * On Native AMP themes when there are new/rejected validation errors present, a parsed stylesheet may include
+		 * @import rules. These must be moved to the beginning to be honored.
+		 */
+		$imports = array();
+
 		// Divide pending stylesheet between custom and keyframes, and calculate size of each.
 		while ( ! empty( $this->pending_stylesheets ) ) {
 			$pending_stylesheet = array_shift( $this->pending_stylesheets );
 
 			$set_name = ! empty( $pending_stylesheet['keyframes'] ) ? 'keyframes' : 'custom';
 			$size     = 0;
-			foreach ( $pending_stylesheet['stylesheet'] as $part ) {
+			foreach ( $pending_stylesheet['stylesheet'] as $i => $part ) {
 				if ( is_string( $part ) ) {
 					$size += strlen( $part );
+					if ( '@import' === substr( $part, 0, 7 ) ) {
+						$imports[] = $part;
+						unset( $pending_stylesheet['stylesheet'][ $i ] );
+					}
 				} elseif ( is_array( $part ) ) {
 					$size += strlen( implode( ',', array_keys( $part[0] ) ) ); // Selectors.
 					$size += strlen( $part[1] ); // Declaration block.
 				}
 			}
 			$stylesheet_sets[ $set_name ]['total_size']           += $size;
+			$stylesheet_sets[ $set_name ]['imports']               = $imports;
 			$stylesheet_sets[ $set_name ]['pending_stylesheets'][] = $pending_stylesheet;
 		}
 
@@ -1513,7 +1608,8 @@ private function finalize_styles() {
 				$head->appendChild( $this->amp_custom_style_element );
 			}
 
-			$css = implode( '', $stylesheet_sets['custom']['final_stylesheets'] );
+			$css  = implode( '', $stylesheet_sets['custom']['imports'] ); // For native dirty AMP.
+			$css .= implode( '', $stylesheet_sets['custom']['final_stylesheets'] );
 
 			/*
 			 * Let the style[amp-custom] be populated with the concatenated CSS.
@@ -1544,6 +1640,9 @@ private function finalize_styles() {
 						$message .= sprintf( '[%s=%s]', $attribute->nodeName, $attribute->nodeValue );
 					}
 				}
+				if ( ! empty( $pending_stylesheet['imported'] ) ) {
+					$message .= sprintf( ' @import url(%s)', $pending_stylesheet['imported'] );
+				}
 				$message .= sprintf(
 					/* translators: %d is number of bytes */
 					_n( ' (%d byte)', ' (%d bytes)', $pending_stylesheet['size'], 'amp' ),

includes/validation/class-amp-invalid-url-post-type.php

@@ -793,6 +793,12 @@ public static function handle_validation_error_status_update() {
 			return;
 		}
 		$updated_count = 0;
+
+		$has_pre_term_description_filter = has_filter( 'pre_term_description', 'wp_filter_kses' );
+		if ( false !== $has_pre_term_description_filter ) {
+			remove_filter( 'pre_term_description', 'wp_filter_kses', $has_pre_term_description_filter );
+		}
+
 		foreach ( $_POST[ AMP_Validation_Manager::VALIDATION_ERROR_TERM_STATUS_QUERY_VAR ] as $term_slug => $status ) {
 			$term_slug = sanitize_key( $term_slug );
 			$term      = get_term_by( 'slug', $term_slug, AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG );
@@ -805,6 +811,11 @@ public static function handle_validation_error_status_update() {
 				wp_update_term( $term->term_id, AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG, compact( 'term_group' ) );
 			}
 		}
+
+		if ( false !== $has_pre_term_description_filter ) {
+			add_filter( 'pre_term_description', 'wp_filter_kses', $has_pre_term_description_filter );
+		}
+
 		$args = array(
 			'amp_taxonomy_terms_updated' => $updated_count,
 		);
@@ -943,14 +954,14 @@ public static function print_validation_errors_meta_box( $post ) {
 			validateUrl = <?php echo wp_json_encode( add_query_arg( AMP_Validation_Manager::VALIDATE_QUERY_VAR, AMP_Validation_Manager::get_amp_validate_nonce(), $post->post_title ) ); ?>;
 			postId = <?php echo wp_json_encode( $post->ID ); ?>;
 			$( '#preview_validation_errors' ).on( 'click', function() {
-				var params = {};
+				var params = {}, validatePreviewUrl = validateUrl;
 				$( '.amp-validation-error-status' ).each( function() {
 					if ( this.value && ! this.options[ this.selectedIndex ].defaultSelected ) {
 						params[ this.name ] = this.value;
 					}
 				} );
-				validateUrl += '&' + $.param( params );
-				window.open( validateUrl, 'amp-validation-error-term-status-preview-' + String( postId ) );
+				validatePreviewUrl += '&' + $.param( params );
+				window.open( validatePreviewUrl, 'amp-validation-error-term-status-preview-' + String( postId ) );
 			} );
 		} );
 		</script>

tests/test-amp-style-sanitizer.php

@@ -801,4 +801,66 @@ public function test_font_urls( $url, $error_codes ) {
 			$this->assertEmpty( $link );
 		}
 	}
+
+	/**
+	 * Get data for CSS @import test.
+	 *
+	 * @return array
+	 */
+	public function get_css_import_data() {
+		$css_url             = admin_url( 'css/login.css' );
+		$html                = sprintf( '<html><head><link rel="stylesheet" href="%s"></head><style>@import url("https://something.example.com/style.css"); body { color:red; }</style></html>', $css_url ); // phpcs:ignore WordPress.WP.EnqueuedResources.NonEnqueuedStylesheet
+		$stylesheet_patterns = array(
+			'/' . preg_quote( 'input[type="checkbox"]:disabled' ) . '/',
+			'/' . preg_quote( 'body.rtl' ) . '/',
+			'/' . preg_quote( '.login .message' ) . '/',
+			'/^' . preg_quote( 'body{color:red}' ) . '$/',
+		);
+		return array(
+			'sanitized'   => array(
+				$html,
+				$stylesheet_patterns,
+				'/^(?!@import)/',
+				true,
+			),
+			'unsanitized' => array(
+				$html,
+				$stylesheet_patterns,
+				'/^@import/',
+				false,
+			),
+		);
+	}
+
+	/**
+	 * Test CSS imports.
+	 *
+	 * @dataProvider get_css_import_data
+	 * @covers AMP_Style_Sanitizer::parse_import_stylesheet()
+	 *
+	 * @param string $markup              Markup.
+	 * @param array  $stylesheet_patterns Stylesheet patterns.
+	 * @param string $style_pattern       Style pattern for resulting style element.
+	 * @param bool   $sanitized           Whether validation errors should be sanitized.
+	 */
+	public function test_css_import( $markup, $stylesheet_patterns, $style_pattern, $sanitized ) {
+		$dom       = AMP_DOM_Utils::get_dom( $markup );
+		$sanitizer = new AMP_Style_Sanitizer( $dom, array(
+			'use_document_element'      => true,
+			'remove_unused_rules'       => 'never',
+			'validation_error_callback' => function() use ( $sanitized ) {
+				return $sanitized;
+			},
+		) );
+		$sanitizer->sanitize();
+		$stylesheets = array_values( $sanitizer->get_stylesheets() );
+		$this->assertCount( count( $stylesheet_patterns ), $stylesheets );
+		do {
+			$this->assertRegExp( array_shift( $stylesheet_patterns ), array_shift( $stylesheets ) );
+		} while ( ! empty( $stylesheet_patterns ) );
+
+		$dom_xpath  = new DOMXPath( $dom );
+		$amp_custom = $dom_xpath->query( '//style[@amp-custom]' )->item( 0 );
+		$this->assertRegExp( $style_pattern, $amp_custom->textContent );
+	}
 }