Harden validation of user-submitted LCP background image URL

plugins/image-prioritizer/helper.php

@@ -137,6 +137,66 @@ function image_prioritizer_add_element_item_schema_properties( array $additional
 	return $additional_properties;
 }
 
+/**
+ * Validates that the provided background image URL is valid.
+ *
+ * @since n.e.x.t
+ *
+ * @param bool|WP_Error|mixed  $validity   Validity. Valid if true or a WP_Error without any errors, or invalid otherwise.
+ * @param OD_Strict_URL_Metric $url_metric URL Metric, already validated against the JSON Schema.
+ * @return bool|WP_Error Validity. Valid if true or a WP_Error without any errors, or invalid otherwise.
+ */
+function image_prioritizer_filter_store_url_metric_validity( $validity, OD_Strict_URL_Metric $url_metric ) {
+	if ( ! is_bool( $validity ) && ! ( $validity instanceof WP_Error ) ) {
+		$validity = (bool) $validity;
+	}
+
+	$data = $url_metric->get( 'lcpElementExternalBackgroundImage' );
+	if ( ! is_array( $data ) ) {
+		return $validity;
+	}
+
+	$r = wp_safe_remote_head(
+		$data['url'],
+		array(
+			'redirection' => 3, // Allow up to 3 redirects.
+		)
+	);
+	if ( $r instanceof WP_Error ) {
+		return new WP_Error(
+			WP_DEBUG ? $r->get_error_code() : 'head_request_failure',
+			__( 'HEAD request for background image URL failed.', 'image-prioritizer' ) . ( WP_DEBUG ? ' ' . $r->get_error_message() : '' ),
+			array(
+				'code' => 500,
+			)
+		);
+	}
+	$response_code = wp_remote_retrieve_response_code( $r );
+	if ( $response_code < 200 || $response_code >= 400 ) {
+		return new WP_Error(
+			'background_image_response_not_ok',
+			__( 'HEAD request for background image URL did not return with a success status code.', 'image-prioritizer' ),
+			array(
+				'code' => WP_DEBUG ? $response_code : 400,
+			)
+		);
+	}
+
+	$content_type = wp_remote_retrieve_header( $r, 'Content-Type' );
+	if ( ! is_string( $content_type ) || ! str_starts_with( $content_type, 'image/' ) ) {
+		return new WP_Error(
+			'background_image_response_not_image',
+			__( 'HEAD request for background image URL did not return an image Content-Type.', 'image-prioritizer' ),
+			array(
+				'code' => 400,
+			)
+		);
+	}
+
+	// TODO: Check for the Content-Length and return invalid if it is gigantic?
+	return $validity;
+}
+
 /**
  * Gets the path to a script or stylesheet.
  *

plugins/image-prioritizer/hooks.php

@@ -13,3 +13,4 @@
 add_action( 'od_init', 'image_prioritizer_init' );
 add_filter( 'od_extension_module_urls', 'image_prioritizer_filter_extension_module_urls' );
 add_filter( 'od_url_metric_schema_root_additional_properties', 'image_prioritizer_add_element_item_schema_properties' );
+add_filter( 'od_store_url_metric_validity', 'image_prioritizer_filter_store_url_metric_validity', 10, 2 );

plugins/optimization-detective/class-od-element.php

@@ -208,18 +208,53 @@ public function offsetSet( $offset, $value ): void {
 	}
 
 	/**
-	 * Offset to unset.
+	 * Unsets a property.
 	 *
-	 * This is disallowed. Attempting to unset a property will throw an error.
+	 * This is particularly useful in conjunction with the `od_url_metric_schema_element_item_additional_properties` filter.
+	 * This will throw an exception if the property is required by the schema.
 	 *
-	 * @since 0.7.0
+	 * @since n.e.x.t
+	 *
+	 * @param string $key Property.
+	 *
+	 * @throws OD_Data_Validation_Exception When attempting to unset a property required by the JSON Schema.
+	 */
+	public function unset( string $key ): void {
+		$schema = OD_URL_Metric::get_json_schema(); // TODO: This should be a non-static method once the URL Metric is instantiated.
+		if (
+			isset( $schema['properties']['elements']['items']['properties'][ $key ]['required'] )
+			&& true === $schema['properties']['elements']['items']['properties'][ $key ]['required']
+		) {
+			throw new OD_Data_Validation_Exception(
+				esc_html(
+					sprintf(
+						/* translators: %s is the property key. */
+						__( 'The %s key is required for an item of elements in a URL Metric.', 'optimization-detective' ),
+						$key
+					)
+				)
+			);
+		}
+		unset( $this->data[ $key ] ); // @phpstan-ignore assign.propertyType (Above required check ensures $key is not isLCP, isLCPCandidate, xpath, intersectionRatio, intersectionRect, boundingClientRect.)
+		$group = $this->url_metric->get_group();
+		if ( $group instanceof OD_URL_Metric_Group ) {
+			$group->clear_cache();
+		}
+	}
+
+	/**
+	 * Unsets an offset.
+	 *
+	 * This will throw an exception if the offset is required by the schema.
+	 *
+	 * @since n.e.x.t
 	 *
 	 * @param mixed $offset Offset.
 	 *
-	 * @throws Exception When attempting to unset a property.
+	 * @throws OD_Data_Validation_Exception When attempting to unset a property required by the JSON Schema.
 	 */
 	public function offsetUnset( $offset ): void {
-		throw new Exception( 'Element data may not be unset.' );
+		$this->unset( (string) $offset );
 	}
 
 	/**

plugins/optimization-detective/class-od-url-metric-group.php

@@ -237,8 +237,7 @@ public function add_url_metric( OD_URL_Metric $url_metric ): void {
 			);
 		}
 
-		$this->result_cache = array();
-		$this->collection->clear_cache();
+		$this->clear_cache();
 
 		$url_metric->set_group( $this );
 		$this->url_metrics[] = $url_metric;
@@ -471,6 +470,16 @@ public function count(): int {
 		return count( $this->url_metrics );
 	}
 
+	/**
+	 * Clear result cache.
+	 *
+	 * @since n.e.x.t
+	 */
+	public function clear_cache(): void {
+		$this->result_cache = array();
+		$this->collection->clear_cache();
+	}
+
 	/**
 	 * Specifies data which should be serialized to JSON.
 	 *

plugins/optimization-detective/class-od-url-metric.php

@@ -503,6 +503,33 @@ function ( array $element ): OD_Element {
 		return $this->elements;
 	}
 
+	/**
+	 * Unsets a property from the URL Metric.
+	 *
+	 * @since n.e.x.t
+	 *
+	 * @param string $key Key to unset.
+	 * @throws OD_Data_Validation_Exception If the property is required an exception will be thrown.
+	 */
+	public function unset( string $key ): void {
+		$schema = self::get_json_schema(); // TODO: Consider capturing the schema as a private member variable once the URL Metric is constructed.
+		if ( isset( $schema['properties'][ $key ]['required'] ) && true === $schema['properties'][ $key ]['required'] ) {
+			throw new OD_Data_Validation_Exception(
+				esc_html(
+					sprintf(
+						/* translators: %s is the property key. */
+						__( 'The %s key is required at the root of a URL Metric.', 'optimization-detective' ),
+						$key
+					)
+				)
+			);
+		}
+		unset( $this->data[ $key ] ); // @phpstan-ignore assign.propertyType (Above required check ensures $key is not uuid, url, timestamp, viewport, or elements.)
+		if ( $this->group instanceof OD_URL_Metric_Group ) {
+			$this->group->clear_cache();
+		}
+	}
+
 	/**
 	 * Specifies data which should be serialized to JSON.
 	 *
@@ -511,6 +538,15 @@ function ( array $element ): OD_Element {
 	 * @return Data Exports to be serialized by json_encode().
 	 */
 	public function jsonSerialize(): array {
-		return $this->data;
+		$data = $this->data;
+
+		$data['elements'] = array_map(
+			static function ( OD_Element $element ): array {
+				return $element->jsonSerialize();
+			},
+			$this->get_elements()
+		);
+
+		return $data;
 	}
 }

plugins/optimization-detective/storage/rest-api.php

@@ -210,6 +210,35 @@ function od_handle_rest_request( WP_REST_Request $request ) {
 		);
 	}
 
+	/**
+	 * Filters whether a URL Metric is valid for storage.
+	 *
+	 * This allows for custom validation constraints to be applied beyond what can be expressed in JSON Schema. This is
+	 * also necessary because the 'validate_callback' key in a JSON Schema is not respected when gathering the REST API
+	 * endpoint args via the {@see rest_get_endpoint_args_for_schema()} function. Besides this, the REST API doesn't
+	 * support 'validate_callback' for any nested arguments in any case, meaning that custom constraints would be able
+	 * to be applied to multidimensional objects, such as the items inside 'elements'.
+	 *
+	 * This filter only applies when storing a URL Metric via the REST API. It does not run when a stored URL Metric
+	 * loaded from the od_url_metric post type. This means that validation logic enforced via this filter can be more
+	 * expensive, such as doing filesystem checks or HTTP requests.
+	 *
+	 * @since n.e.x.t
+	 *
+	 * @param bool|WP_Error        $validity   Validity. Valid if true or a WP_Error without any errors, or invalid otherwise.
+	 * @param OD_Strict_URL_Metric $url_metric URL Metric, already validated against the JSON Schema.
+	 */
+	$validity = apply_filters( 'od_store_url_metric_validity', true, $url_metric );
+	if ( false === $validity || ( $validity instanceof WP_Error && $validity->has_errors() ) ) {
+		if ( false === $validity ) {
+			$validity = new WP_Error( 'invalid_url_metric', __( 'Validity of URL Metric was rejected by filter.', 'optimization-detective' ) );
+		}
+		if ( ! isset( $validity->error_data['status'] ) ) {
+			$validity->error_data['status'] = 400;
+		}
+		return $validity;
+	}
+
 	// TODO: This should be changed from store_url_metric($slug, $url_metric) instead be update_post( $slug, $group_collection ). As it stands, store_url_metric() is duplicating logic here.
 	$result = OD_URL_Metrics_Post_Type::store_url_metric(
 		$request->get_param( 'slug' ),

plugins/optimization-detective/tests/test-class-od-element.php

@@ -24,6 +24,7 @@ class Test_OD_Element extends WP_UnitTestCase {
 	 * @covers ::offsetExists
 	 * @covers ::offsetGet
 	 * @covers ::offsetSet
+	 * @covers ::unset
 	 * @covers ::offsetUnset
 	 * @covers ::jsonSerialize
 	 */
@@ -130,22 +131,68 @@ static function ( array $schema ): array {
 		$this->assertTrue( isset( $element['customProp'] ) );
 		$this->assertTrue( $element->offsetExists( 'customProp' ) );
 
-		$this->assertEquals( $element_data, $element->jsonSerialize() );
+		// Try setting property (which is not currently supported).
+		$functions = array(
+			static function ( OD_Element $element ): void {
+				$element['isLCP'] = true;
+			},
+			static function ( OD_Element $element ): void {
+				$element->offsetSet( 'isLCP', true );
+			},
+		);
+		foreach ( $functions as $function ) {
+			$exception = null;
+			try {
+				$function( $element );
+			} catch ( Exception $e ) {
+				$exception = $e;
+			}
+			$this->assertInstanceOf( Exception::class, $exception );
+			$this->assertFalse( $element->get( 'isLCP' ) );
+		}
 
-		$exception = null;
-		try {
-			$element['isLCP'] = true;
-		} catch ( Exception $e ) {
-			$exception = $e;
+		// Try unsetting a required property.
+		$functions = array(
+			static function ( OD_Element $element ): void {
+				unset( $element['isLCP'] );
+			},
+			static function ( OD_Element $element ): void {
+				$element->unset( 'isLCP' );
+			},
+			static function ( OD_Element $element ): void {
+				$element->offsetUnset( 'isLCP' );
+			},
+		);
+		foreach ( $functions as $function ) {
+			$exception = null;
+			try {
+				$function( $element );
+			} catch ( Exception $e ) {
+				$exception = $e;
+			}
+			$this->assertInstanceOf( Exception::class, $exception );
+			$this->assertArrayHasKey( 'isLCP', $element->jsonSerialize() );
 		}
-		$this->assertInstanceOf( Exception::class, $exception );
 
-		$exception = null;
-		try {
-			unset( $element['isLCP'] );
-		} catch ( Exception $e ) { // @phpstan-ignore catch.neverThrown (It is thrown by offsetUnset actually.)
-			$exception = $e;
+		$this->assertEquals( $element_data, $element->jsonSerialize() );
+
+		// Try unsetting a non-required property.
+		$functions = array(
+			static function ( OD_Element $element ): void {
+				unset( $element['customProp'] );
+			},
+			static function ( OD_Element $element ): void {
+				$element->unset( 'customProp' );
+			},
+			static function ( OD_Element $element ): void {
+				$element->offsetUnset( 'customProp' );
+			},
+		);
+		foreach ( $functions as $function ) {
+			$cloned_element = clone $element;
+			$function( $cloned_element );
+			$this->assertFalse( $cloned_element->offsetExists( 'customProp' ) );
+			$this->assertArrayNotHasKey( 'customProp', $cloned_element->jsonSerialize() );
 		}
-		$this->assertInstanceOf( Exception::class, $exception ); // @phpstan-ignore method.impossibleType (It is thrown by offsetUnset actually.)
 	}
 }

plugins/optimization-detective/tests/test-class-od-url-metric.php

@@ -277,6 +277,7 @@ static function ( $value ) {
 	 * @covers ::get_json_schema
 	 * @covers ::set_group
 	 * @covers ::get_group
+	 * @covers ::unset
 	 *
 	 * @dataProvider data_provider_to_test_constructor
 	 *
@@ -336,6 +337,15 @@ static function ( OD_Element $element ) {
 		$this->assertTrue( wp_is_uuid( $url_metric->get_uuid() ) );
 		$this->assertSame( $url_metric->get_uuid(), $url_metric->get( 'uuid' ) );
 
+		$exception = null;
+		try {
+			$url_metric->unset( 'elements' );
+		} catch ( OD_Data_Validation_Exception $e ) {
+			$exception = $e;
+		}
+		$this->assertInstanceOf( OD_Data_Validation_Exception::class, $exception );
+		$url_metric->unset( 'does_not_exist' );
+
 		$serialized = $url_metric->jsonSerialize();
 		if ( ! array_key_exists( 'uuid', $data ) ) {
 			$this->assertTrue( wp_is_uuid( $serialized['uuid'] ) );
@@ -397,6 +407,12 @@ static function ( array $properties ): array {
 					$this->assertArrayHasKey( 'isTouch', $extended_data );
 					$this->assertTrue( $extended_data['isTouch'] );
 					$this->assertTrue( $extended_url_metric->get( 'isTouch' ) );
+
+					$this->assertTrue( $extended_url_metric->get( 'isTouch' ) );
+					$extended_url_metric->unset( 'isTouch' );
+					$this->assertNull( $extended_url_metric->get( 'isTouch' ) );
+					$extended_data = $extended_url_metric->jsonSerialize();
+					$this->assertArrayNotHasKey( 'isTouch', $extended_data );
 				},
 			),
 
@@ -489,6 +505,13 @@ static function ( array $properties ): array {
 					$this->assertArrayHasKey( 'isColorful', $extended_data['elements'][0] );
 					$this->assertFalse( $extended_data['elements'][0]['isColorful'] );
 					$this->assertFalse( $extended_url_metric->get_elements()[0]['isColorful'] );
+
+					$element = $extended_url_metric->get_elements()[0];
+					$this->assertFalse( $element->get( 'isColorful' ) );
+					$element->unset( 'isColorful' );
+					$this->assertNull( $element->get( 'isColorful' ) );
+					$extended_data = $extended_url_metric->jsonSerialize();
+					$this->assertArrayNotHasKey( 'isColorful', $extended_data['elements'][0] );
 				},
 			),
 
@@ -554,6 +577,8 @@ static function ( array $properties ): array {
 	 * Tests construction with extended schema.
 	 *
 	 * @covers ::get_json_schema
+	 * @covers ::unset
+	 * @covers OD_Element::unset
 	 *
 	 * @dataProvider data_provider_to_test_constructor_with_extended_schema
 	 *