Merge pull request #4137 from ampproject/enhancement/4070-validate-props

Raise validation errors for properties in attributes

Author: westonruter

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

@@ -79,6 +79,15 @@ static function ( $element ) {
 		);
 
 		foreach ( $elements as $element ) {
+
+			// Strip whitespace around equal signs. Won't be needed after <https://github.com/ampproject/amphtml/issues/26496> is resolved.
+			if ( $element->hasAttribute( 'content' ) ) {
+				$element->setAttribute(
+					'content',
+					preg_replace( '/\s*=\s*/', '=', $element->getAttribute( 'content' ) )
+				);
+			}
+
 			/**
 			 * Meta tag to process.
 			 *
@@ -126,37 +135,7 @@ protected function ensure_charset_is_present() {
 	protected function ensure_viewport_is_present() {
 		if ( empty( $this->meta_tags[ self::TAG_VIEWPORT ] ) ) {
 			$this->meta_tags[ self::TAG_VIEWPORT ][] = $this->create_viewport_element( static::AMP_VIEWPORT );
-			return;
-		}
-
-		// Ensure we have the 'width=device-width' setting included.
-		/**
-		 * Viewport tag.
-		 *
-		 * @var DOMElement $viewport_tag
-		 */
-		$viewport_tag      = $this->meta_tags[ self::TAG_VIEWPORT ][0];
-		$viewport_content  = $viewport_tag->getAttribute( 'content' );
-		$viewport_settings = array_map( 'trim', explode( ',', $viewport_content ) );
-		$width_found       = false;
-
-		// @todo The invalid/missing properties should raise validation errors. See <https://github.com/ampproject/amp-wp/pull/3758/files#r348074703>.
-		foreach ( $viewport_settings as $index => $viewport_setting ) {
-			list( $property, $value ) = array_map( 'trim', explode( '=', $viewport_setting ) );
-			if ( 'width' === $property ) {
-				if ( 'device-width' !== $value ) {
-					$viewport_settings[ $index ] = 'width=device-width';
-				}
-				$width_found = true;
-				break;
-			}
 		}
-
-		if ( ! $width_found ) {
-			array_unshift( $viewport_settings, 'width=device-width' );
-		}
-
-		$viewport_tag->setAttribute( 'content', implode( ',', $viewport_settings ) );
 	}
 
 	/**

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

@@ -2131,11 +2131,11 @@ private function process_css_declaration_block( RuleSet $ruleset, CSSList $css_l
 				$vendorless_property_name = preg_replace( '/^-\w+-/', '', $property->getRule() );
 				if ( ! in_array( $vendorless_property_name, $options['property_whitelist'], true ) ) {
 					$error     = [
-						'code'           => self::CSS_SYNTAX_INVALID_PROPERTY,
-						'property_name'  => $property->getRule(),
-						'property_value' => $property->getValue(),
-						'type'           => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE,
-						'spec_name'      => $options['spec_name'],
+						'code'               => self::CSS_SYNTAX_INVALID_PROPERTY,
+						'css_property_name'  => $property->getRule(),
+						'css_property_value' => $property->getValue(),
+						'type'               => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE,
+						'spec_name'          => $options['spec_name'],
 					];
 					$sanitized = $this->should_sanitize_validation_error( $error );
 					if ( $sanitized ) {
@@ -2149,11 +2149,11 @@ private function process_css_declaration_block( RuleSet $ruleset, CSSList $css_l
 				$properties = $ruleset->getRules( $illegal_property_name );
 				foreach ( $properties as $property ) {
 					$error     = [
-						'code'           => self::CSS_SYNTAX_INVALID_PROPERTY_NOLIST,
-						'property_name'  => $property->getRule(),
-						'property_value' => (string) $property->getValue(),
-						'type'           => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE,
-						'spec_name'      => $options['spec_name'],
+						'code'               => self::CSS_SYNTAX_INVALID_PROPERTY_NOLIST,
+						'css_property_name'  => $property->getRule(),
+						'css_property_value' => (string) $property->getValue(),
+						'type'               => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE,
+						'spec_name'          => $options['spec_name'],
 					];
 					$sanitized = $this->should_sanitize_validation_error( $error );
 					if ( $sanitized ) {
@@ -2397,11 +2397,11 @@ private function process_css_keyframes( KeyFrame $css_list, $options ) {
 					$vendorless_property_name = preg_replace( '/^-\w+-/', '', $property->getRule() );
 					if ( ! in_array( $vendorless_property_name, $options['property_whitelist'], true ) ) {
 						$error     = [
-							'code'           => self::CSS_SYNTAX_INVALID_PROPERTY,
-							'property_name'  => $property->getRule(),
-							'property_value' => (string) $property->getValue(),
-							'type'           => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE,
-							'spec_name'      => $options['spec_name'],
+							'code'               => self::CSS_SYNTAX_INVALID_PROPERTY,
+							'css_property_name'  => $property->getRule(),
+							'css_property_value' => (string) $property->getValue(),
+							'type'               => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE,
+							'spec_name'          => $options['spec_name'],
 						];
 						$sanitized = $this->should_sanitize_validation_error( $error );
 						if ( $sanitized ) {
@@ -2447,11 +2447,11 @@ private function transform_important_qualifiers( RuleSet $ruleset, CSSList $css_
 					$ruleset->removeRule( $property->getRule() );
 				} else {
 					$error     = [
-						'code'           => self::CSS_SYNTAX_INVALID_IMPORTANT,
-						'type'           => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE,
-						'property_name'  => $property->getRule(),
-						'property_value' => $property->getValue(),
-						'spec_name'      => $options['spec_name'],
+						'code'               => self::CSS_SYNTAX_INVALID_IMPORTANT,
+						'type'               => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE,
+						'css_property_name'  => $property->getRule(),
+						'css_property_value' => $property->getValue(),
+						'spec_name'          => $options['spec_name'],
 					];
 					$sanitized = $this->should_sanitize_validation_error( $error );
 					if ( $sanitized ) {