Merge pull request #5296 from ampproject/fix/validating-amp-unavailable-urls

Show error when attempting to validate an AMP-unavailable URL instead of forcing AMP to be available

Author: westonruter

includes/amp-helper-functions.php

@@ -480,16 +480,6 @@ function amp_is_available() {
 		return false;
 	}
 
-	/*
-	 * If this is a URL for validation, and validation is forced for all URLs, return true.
-	 * Normally, this would be false if the user has deselected a template,
-	 * like by unchecking 'Categories' in 'AMP Settings' > 'Supported Templates'.
-	 * But there's a flag for the WP-CLI command that sets this query var to validate all URLs.
-	 */
-	if ( AMP_Validation_Manager::is_theme_support_forced() ) {
-		return true;
-	}
-
 	$queried_object = get_queried_object();
 	if ( ! $is_legacy ) {
 		// Abort if in Transitional mode and AMP is not available for the URL.

includes/class-amp-theme-support.php

@@ -1791,6 +1791,9 @@ public static function prepare_response( $response, $args = [] ) {
 			if ( is_bool( $status_code ) ) {
 				$status_code = 200; // Not a web server environment.
 			}
+			if ( ! headers_sent() ) {
+				header( 'Content-Type: application/json; charset=utf-8' );
+			}
 			return wp_json_encode(
 				[
 					'status_code' => $status_code,
@@ -1824,6 +1827,19 @@ public static function prepare_response( $response, $args = [] ) {
 				$response
 			)
 		) ) {
+			if ( AMP_Validation_Manager::$is_validate_request ) {
+				if ( ! headers_sent() ) {
+					status_header( 400 );
+					header( 'Content-Type: application/json; charset=utf-8' );
+				}
+				return wp_json_encode(
+					[
+						'code'    => 'RENDERED_PAGE_NOT_AMP',
+						'message' => __( 'The requested URL did not result in an AMP page being rendered.', 'amp' ),
+					]
+				);
+			}
+
 			return $response;
 		}
 

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

@@ -5,10 +5,8 @@
  * @package AMP
  */
 
-use AmpProject\AmpWP\DevTools\BlockSources;
 use AmpProject\AmpWP\DevTools\UserAccess;
 use AmpProject\AmpWP\Icon;
-use AmpProject\AmpWP\Option;
 use AmpProject\AmpWP\QueryVar;
 use AmpProject\AmpWP\Services;
 use AmpProject\Attribute;
@@ -211,6 +209,7 @@ static function() {
 
 		add_action( 'all_admin_notices', [ __CLASS__, 'print_plugin_notice' ] );
 		add_action( 'admin_bar_menu', [ __CLASS__, 'add_admin_bar_menu_items' ], 101 );
+		add_action( 'wp', [ __CLASS__, 'maybe_fail_validate_request' ] );
 		add_action( 'wp', [ __CLASS__, 'override_validation_error_statuses' ] );
 	}
 
@@ -244,17 +243,6 @@ public static function post_supports_validation( $post ) {
 		);
 	}
 
-	/**
-	 * Determine whether AMP theme support is forced via the amp_validate query param.
-	 *
-	 * @since 1.0
-	 *
-	 * @return bool Whether theme support forced.
-	 */
-	public static function is_theme_support_forced() {
-		return self::$is_validate_request;
-	}
-
 	/**
 	 * Return whether sanitization is initially accepted (by default) for newly encountered validation errors.
 	 *
@@ -428,7 +416,7 @@ public static function add_admin_bar_menu_items( $wp_admin_bar ) {
 	/**
 	 * Override validation error statuses (when requested).
 	 *
-	 * When a query var is present along with the required nonce, override the status of the status of the invalid markup
+	 * When a query var is present along with the required nonce, override the status of the invalid markup
 	 * as requested.
 	 *
 	 * @since 1.5.0
@@ -468,6 +456,26 @@ public static function override_validation_error_statuses() {
 		}
 	}
 
+	/**
+	 * Short-circuit validation requests which are for URLs that are not AMP pages.
+	 *
+	 * @since 2.1
+	 */
+	public static function maybe_fail_validate_request() {
+		if ( ! self::$is_validate_request || amp_is_request() ) {
+			return;
+		}
+
+		if ( ! amp_is_available() ) {
+			$code    = 'AMP_NOT_AVAILABLE';
+			$message = __( 'The requested URL is not an AMP page. AMP may have been disabled for the URL. If so, you can forget the Validated URL.', 'amp' );
+		} else {
+			$code    = 'AMP_NOT_REQUESTED';
+			$message = __( 'The requested URL is not an AMP page.', 'amp' );
+		}
+		wp_send_json( compact( 'code', 'message' ), 400 );
+	}
+
 	/**
 	 * Initialize a validate request.
 	 *

tests/php/test-amp-helper-functions.php

@@ -906,11 +906,6 @@ public function test_amp_is_request_and_amp_is_available() {
 		$this->go_to( home_url( '/' ) );
 		$this->assertFalse( amp_is_available() );
 		$this->assertFalse( amp_is_request() );
-
-		// When the user passes a flag to the WP-CLI command, it forces AMP validation no matter whether the user disabled AMP on any template.
-		AMP_Validation_Manager::$is_validate_request = true;
-		$this->assertTrue( amp_is_available() );
-		$this->assertTrue( amp_is_request() );
 	}
 
 	/**

tests/php/test-class-amp-theme-support.php

@@ -78,6 +78,7 @@ public function tearDown() {
 
 		parent::tearDown();
 		unset( $GLOBALS['show_admin_bar'] );
+		AMP_Validation_Manager::$is_validate_request = false;
 		AMP_Validation_Manager::reset_validation_results();
 		$this->set_template_mode( AMP_Theme_Support::READER_MODE_SLUG );
 		remove_theme_support( 'custom-header' );
@@ -1758,6 +1759,19 @@ public function test_prepare_response_for_submitted_form() {
 		unset( $_SERVER['REQUEST_METHOD'] );
 	}
 
+	/**
+	 * Test prepare_response when validating an invalid AMP page.
+	 *
+	 * @covers AMP_Theme_Support::prepare_response()
+	 */
+	public function test_prepare_response_for_validating_invalid_amp_page() {
+		AMP_Validation_Manager::$is_validate_request = true;
+
+		$response = AMP_Theme_Support::prepare_response( '' );
+		$this->assertJson( $response );
+		$this->assertStringContains( 'RENDERED_PAGE_NOT_AMP', $response );
+	}
+
 	/**
 	 * Initializes and returns the original HTML.
 	 */

tests/php/validation/test-class-amp-validation-manager.php

@@ -157,7 +157,66 @@ public function test_init() {
 
 		$this->assertEquals( 101, has_action( 'admin_bar_menu', [ self::TESTED_CLASS, 'add_admin_bar_menu_items' ] ) );
 
-		$this->assertFalse( has_action( 'wp', [ self::TESTED_CLASS, 'wrap_widget_callbacks' ] ) );
+		$this->assertEquals( 10, has_action( 'wp', [ self::TESTED_CLASS, 'maybe_fail_validate_request' ] ) );
+		$this->assertEquals( 10, has_action( 'wp', [ self::TESTED_CLASS, 'override_validation_error_statuses' ] ) );
+	}
+
+	/**
+	 * Test ::maybe_fail_validate_request().
+	 *
+	 * @covers AMP_Validation_Manager::maybe_fail_validate_request()
+	 */
+	public function test_maybe_fail_validate_request() {
+		$post_id = self::factory()->post->create();
+
+		remove_filter( 'wp', [ 'AMP_Validation_Manager', 'maybe_fail_validate_request' ] );
+		add_filter( 'wp_doing_ajax', '__return_true' );
+		add_filter(
+			'wp_die_ajax_handler',
+			static function () {
+				return static function () {};
+			} 
+		);
+
+		$get_output = static function () {
+			ob_start();
+			AMP_Validation_Manager::maybe_fail_validate_request();
+			return ob_get_clean();
+		};
+
+		// Verify there is no output if it is not a validation request.
+		AMP_Validation_Manager::$is_validate_request = false;
+		$this->assertEmpty( $get_output() );
+
+		// Verify there is no output if it is an AMP request.
+		AMP_Validation_Manager::$is_validate_request = true;
+		AMP_Options_Manager::update_option( Option::THEME_SUPPORT, AMP_Theme_Support::STANDARD_MODE_SLUG );
+		$this->go_to( get_permalink( $post_id ) );
+		$this->assertEmpty( $get_output() );
+
+		// Verify correct response if not an AMP page.
+		AMP_Options_Manager::update_option( Option::THEME_SUPPORT, AMP_Theme_Support::READER_MODE_SLUG );
+		$output = $get_output();
+		$this->assertJson( $output );
+		$this->assertStringContains( 'AMP_NOT_REQUESTED', $output );
+
+		// Verify correct response if AMP not available.
+		AMP_Validation_Manager::$is_validate_request = true;
+		AMP_Options_Manager::update_option( Option::THEME_SUPPORT, AMP_Theme_Support::READER_MODE_SLUG );
+
+		add_filter(
+			'amp_skip_post',
+			static function ( $skipped, $_post_id ) use ( $post_id ) {
+				return $_post_id === $post_id;
+			},
+			10,
+			2 
+		);
+
+		$this->go_to( amp_get_permalink( $post_id ) );
+		$output = $get_output();
+		$this->assertJson( $output );
+		$this->assertStringContains( 'AMP_NOT_AVAILABLE', $output );
 	}
 
 	/**