Merge pull request #4542 from ampproject/fix/validating-urls

Expose fatal errors that occur while validating a URL
ampproject · Apr 21, 2020 · 07b638e · 07b638e
2 parents 087e48a + 6da70d7
commit 07b638e
Show file tree

Hide file tree

Showing 5 changed files with 65 additions and 10 deletions.
diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php
@@ -2014,10 +2014,16 @@ public static function prepare_response( $response, $args = [] ) {
 		if ( AMP_Validation_Manager::$is_validate_request ) {
 			status_header( 200 );
 			header( 'Content-Type: application/json; charset=utf-8' );
-			return wp_json_encode(
-				AMP_Validation_Manager::get_validate_response_data( $sanitization_results ),
-				JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES
-			);
+			$data       = [
+				'http_status_code' => $status_code,
+				'php_fatal_error'  => false,
+			];
+			$last_error = error_get_last();
+			if ( $last_error && in_array( $last_error['type'], [ E_ERROR, E_RECOVERABLE_ERROR, E_CORE_ERROR, E_COMPILE_ERROR, E_USER_ERROR, E_PARSE ], true ) ) {
+				$data['php_fatal_error'] = $last_error;
+			}
+			$data = array_merge( $data, AMP_Validation_Manager::get_validate_response_data( $sanitization_results ) );
+			return wp_json_encode( $data, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES );
 		}
 
 		// Determine what the validation errors are.

diff --git a/includes/cli/class-amp-cli-validation-command.php b/includes/cli/class-amp-cli-validation-command.php
@@ -701,7 +701,7 @@ private function validate_and_store_url( $url, $type ) {
 		AMP_Validated_URL_Post_Type::store_validation_errors(
 			$validation_errors,
 			$validity['url'],
-			wp_array_slice_assoc( $validity, [ 'queried_object', 'stylesheets' ] )
+			wp_array_slice_assoc( $validity, [ 'queried_object', 'stylesheets', 'php_fatal_error' ] )
 		);
 		$unaccepted_error_count = count(
 			array_filter(

diff --git a/includes/options/class-amp-options-manager.php b/includes/options/class-amp-options-manager.php
@@ -590,7 +590,7 @@ public static function handle_updated_theme_support_option() {
 				$invalid_url_post_id    = AMP_Validated_URL_Post_Type::store_validation_errors(
 					$errors,
 					$url,
-					wp_array_slice_assoc( $validation, [ 'queried_object', 'stylesheets' ] )
+					wp_array_slice_assoc( $validation, [ 'queried_object', 'stylesheets', 'php_fatal_error' ] )
 				);
 				$invalid_url_screen_url = ! is_wp_error( $invalid_url_post_id ) ? get_edit_post_link( $invalid_url_post_id, 'raw' ) : null;
 

diff --git a/includes/validation/class-amp-validated-url-post-type.php b/includes/validation/class-amp-validated-url-post-type.php
@@ -848,6 +848,14 @@ public static function store_validation_errors( $validation_errors, $url, $args
 			// Note that json_encode() is being used here because wp_slash() will coerce scalar values to strings.
 			update_post_meta( $post_id, '_amp_stylesheets', wp_slash( wp_json_encode( $args['stylesheets'] ) ) );
 		}
+		if ( isset( $args['php_fatal_error'] ) ) {
+			if ( empty( $args['php_fatal_error'] ) ) {
+				delete_post_meta( $post_id, '_amp_php_fatal_error' );
+			} else {
+				// Note that json_encode() is being used here because wp_slash() will coerce scalar values to strings.
+				update_post_meta( $post_id, '_amp_php_fatal_error', wp_slash( wp_json_encode( $args['php_fatal_error'] ) ) );
+			}
+		}
 
 		delete_transient( static::NEW_VALIDATION_ERROR_URLS_COUNT_TRANSIENT );
 
@@ -1291,7 +1299,7 @@ public static function handle_bulk_action( $redirect, $action, $items ) {
 			self::store_validation_errors(
 				$validation_errors,
 				$validity['url'],
-				wp_array_slice_assoc( $validity, [ 'queried_object', 'stylesheets' ] )
+				wp_array_slice_assoc( $validity, [ 'queried_object', 'stylesheets', 'php_fatal_error' ] )
 			);
 			$unaccepted_error_count = count(
 				array_filter(
@@ -1401,6 +1409,12 @@ public static function print_admin_notice() {
 			);
 		}
 
+		// Add notice for PHP fatal error during validation request.
+		$post = get_post();
+		if ( $post instanceof WP_Post && 'post' === get_current_screen()->base && self::POST_TYPE_SLUG === get_current_screen()->post_type ) {
+			self::render_php_fatal_error_admin_notice( $post );
+		}
+
 		/**
 		 * Adds notices to the single error page.
 		 * 1. Notice with detailed error information in an expanding box.
@@ -1502,6 +1516,34 @@ public static function print_admin_notice() {
 		}
 	}
 
+	/**
+	 * Render PHP fatal error admin notice.
+	 *
+	 * @param WP_Post $post Post.
+	 */
+	private static function render_php_fatal_error_admin_notice( WP_Post $post ) {
+		$error = get_post_meta( $post->ID, '_amp_php_fatal_error', true );
+		if ( empty( $error ) ) {
+			return;
+		}
+		$error_data = json_decode( $error, true );
+		if ( ! is_array( $error_data ) || ! isset( $error_data['message'], $error_data['file'], $error_data['line'] ) ) {
+			return;
+		}
+		?>
+		<div class="notice notice-error">
+			<p><?php echo AMP_Validation_Manager::get_validate_url_error_message( 'fatal_error_during_validation' ); // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped ?></p>
+			<blockquote>
+				<pre><?php echo esc_html( $error_data['message'] ); ?></pre>
+			</blockquote>
+			<p>
+				<?php esc_html_e( 'Location:', 'amp' ); ?>
+				<code><?php echo esc_html( sprintf( '%s:%d', $error_data['file'], $error_data['line'] ) ); ?></code>
+			</p>
+		</div>
+		<?php
+	}
+
 	/**
 	 * Handles clicking 'recheck' on the inline post actions and in the admin bar on the frontend.
 	 *
@@ -1559,7 +1601,7 @@ public static function handle_validate_request() {
 					[
 						'invalid_url_post' => $post,
 					],
-					wp_array_slice_assoc( $validity, [ 'queried_object', 'stylesheets' ] )
+					wp_array_slice_assoc( $validity, [ 'queried_object', 'stylesheets', 'php_fatal_error' ] )
 				)
 			);
 			if ( is_wp_error( $stored ) ) {
@@ -1633,7 +1675,7 @@ public static function recheck_post( $post ) {
 				[
 					'invalid_url_post' => $post,
 				],
-				wp_array_slice_assoc( $validity, [ 'queried_object', 'stylesheets' ] )
+				wp_array_slice_assoc( $validity, [ 'queried_object', 'stylesheets', 'php_fatal_error' ] )
 			)
 		);
 		foreach ( $validation_errors  as $error ) {

diff --git a/includes/validation/class-amp-validation-manager.php b/includes/validation/class-amp-validation-manager.php
@@ -726,7 +726,7 @@ function( $post ) {
 						[
 							'invalid_url_post' => $invalid_url_post_id,
 						],
-						wp_array_slice_assoc( $validity, [ 'queried_object', 'stylesheets' ] )
+						wp_array_slice_assoc( $validity, [ 'queried_object', 'stylesheets', 'php_fatal_error' ] )
 					)
 				);
 
@@ -2242,6 +2242,13 @@ public static function get_validate_url_error_message( $error_code, $error_messa
 						$support_forum_message,
 					]
 				);
+			case 'fatal_error_during_validation':
+				return $implode_non_empty_strings_with_spaces_and_sanitize(
+					[
+						esc_html__( 'A PHP fatal error occurred while validating the URL. This may indicate either a bug in theme/plugin code or it may be due to an issue in the AMP plugin itself. The error details appear below.', 'amp' ),
+						$support_forum_message,
+					]
+				);
 			case 'response_not_json':
 				return $implode_non_empty_strings_with_spaces_and_sanitize(
 					[