diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index 99820b1ab70..52b13f98111 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -67,6 +67,15 @@ class AMP_Theme_Support { */ public static $purged_amp_query_vars = array(); + /** + * Headers sent (or attempted to be sent). + * + * @since 0.7 + * @see AMP_Theme_Support::send_header() + * @var array[] + */ + public static $headers_sent = array(); + /** * Initialize. */ @@ -87,7 +96,7 @@ public static function init() { $args = array_shift( $support ); if ( ! is_array( $args ) ) { trigger_error( esc_html__( 'Expected AMP theme support arg to be array.', 'amp' ) ); // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_trigger_error - } elseif ( count( array_diff( array_keys( $args ), array( 'template_dir', 'available_callback' ) ) ) !== 0 ) { + } elseif ( count( array_diff( array_keys( $args ), array( 'template_dir', 'available_callback', 'comments_live_list' ) ) ) !== 0 ) { trigger_error( esc_html__( 'Expected AMP theme support to only have template_dir and/or available_callback.', 'amp' ) ); // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_trigger_error } } @@ -311,64 +320,176 @@ public static function purge_amp_query_vars() { } /** - * Hook into a form submissions, such as the comment form or some other form submission. + * Send an HTTP response header. + * + * This largely exists to facilitate unit testing but it also provides a better interface for sending headers. + * + * @since 0.7.0 + * + * @param string $name Header name. + * @param string $value Header value. + * @param array $args { + * Args to header(). + * + * @type bool $replace Whether to replace a header previously sent. Default true. + * @type int $status_code Status code to send with the sent header. + * } + * @return bool Whether the header was sent. + */ + public static function send_header( $name, $value, $args = array() ) { + $args = array_merge( + array( + 'replace' => true, + 'status_code' => null, + ), + $args + ); + + self::$headers_sent[] = array_merge( compact( 'name', 'value' ), $args ); + if ( headers_sent() ) { + return false; + } + + header( + sprintf( '%s: %s', $name, $value ), + $args['replace'], + $args['status_code'] + ); + return true; + } + + /** + * Hook into a POST form submissions, such as the comment form or some other form submission. * * @since 0.7.0 - * @global string $pagenow */ public static function handle_xhr_request() { - global $pagenow; - if ( empty( self::$purged_amp_query_vars['__amp_source_origin'] ) ) { + if ( empty( self::$purged_amp_query_vars['__amp_source_origin'] ) || empty( $_SERVER['REQUEST_METHOD'] ) || 'POST' !== $_SERVER['REQUEST_METHOD'] ) { return; } - if ( isset( $pagenow ) && 'wp-comments-post.php' === $pagenow ) { - // We don't need any data, so just send a success. - add_filter( 'comment_post_redirect', function() { - // We don't need any data, so just send a success. - wp_send_json_success(); - }, PHP_INT_MAX ); - self::handle_xhr_headers_output(); - } elseif ( ! empty( self::$purged_amp_query_vars['_wp_amp_action_xhr_converted'] ) ) { - add_filter( 'wp_redirect', array( __CLASS__, 'intercept_post_request_redirect' ), PHP_INT_MAX ); - self::handle_xhr_headers_output(); + // Send AMP response header. + $origin = wp_validate_redirect( wp_sanitize_redirect( esc_url_raw( self::$purged_amp_query_vars['__amp_source_origin'] ) ) ); + if ( $origin ) { + self::send_header( 'AMP-Access-Control-Allow-Source-Origin', $origin, array( 'replace' => true ) ); } + + // Intercept POST requests which redirect. + add_filter( 'wp_redirect', array( __CLASS__, 'intercept_post_request_redirect' ), PHP_INT_MAX ); + + // Add special handling for redirecting after comment submission. + add_filter( 'comment_post_redirect', array( __CLASS__, 'filter_comment_post_redirect' ), PHP_INT_MAX, 2 ); + + // Add die handler for AMP error display, most likely due to problem with comment. + add_filter( 'wp_die_handler', function() { + return array( __CLASS__, 'handle_wp_die' ); + } ); + } /** - * Handle the AMP XHR headers and output errors. + * Strip tags that are not allowed in amp-mustache. * * @since 0.7.0 + * + * @param string $text Text to sanitize. + * @return string Sanitized text. */ - public static function handle_xhr_headers_output() { - // Add die handler for AMP error display. - add_filter( 'wp_die_handler', function() { - /** - * New error handler for AMP form submission. - * - * @param WP_Error|string $error The error to handle. + protected static function wp_kses_amp_mustache( $text ) { + $amp_mustache_allowed_html_tags = array( 'strong', 'b', 'em', 'i', 'u', 's', 'small', 'mark', 'del', 'ins', 'sup', 'sub' ); + return wp_kses( $text, array_fill_keys( $amp_mustache_allowed_html_tags, array() ) ); + } + + /** + * Handle comment_post_redirect to ensure page reload is done when comments_live_list is not supported, while sending back a success message when it is. + * + * @since 0.7.0 + * + * @param string $url Comment permalink to redirect to. + * @param WP_Comment $comment Posted comment. + * @return string URL. + */ + public static function filter_comment_post_redirect( $url, $comment ) { + $theme_support = get_theme_support( 'amp' ); + + // Cause a page refresh if amp-live-list is not implemented for comments via add_theme_support( 'amp', array( 'comments_live_list' => true ) ). + if ( empty( $theme_support[0]['comments_live_list'] ) ) { + /* + * Add the comment ID to the URL to force AMP to refresh the page. + * This is ideally a temporary workaround to deal with https://github.com/ampproject/amphtml/issues/14170 */ - return function( $error ) { - status_header( 400 ); - if ( is_wp_error( $error ) ) { - $error = $error->get_error_message(); - } - $amp_mustache_allowed_html_tags = array( 'strong', 'b', 'em', 'i', 'u', 's', 'small', 'mark', 'del', 'ins', 'sup', 'sub' ); - wp_send_json( array( - 'error' => wp_kses( $error, array_fill_keys( $amp_mustache_allowed_html_tags, array() ) ), - ) ); - }; - } ); + $url = add_query_arg( 'comment', $comment->comment_ID, $url ); + + // Pass URL along to wp_redirect(). + return $url; + } + + // Create a success message to display to the user. + if ( '1' === (string) $comment->comment_approved ) { + $message = __( 'Your comment has been posted.', 'amp' ); + } else { + $message = __( 'Your comment is awaiting moderation.', 'default' ); // Note core string re-use. + } + + /** + * Filters the message when comment submitted success message when + * + * @since 0.7 + */ + $message = apply_filters( 'amp_comment_posted_message', $message, $comment ); - // Send AMP header. - $origin = esc_url_raw( self::$purged_amp_query_vars['__amp_source_origin'] ); - header( 'AMP-Access-Control-Allow-Source-Origin: ' . $origin, true ); + // Message will be shown in template defined by AMP_Theme_Support::add_amp_comment_form_templates(). + wp_send_json( array( + 'message' => self::wp_kses_amp_mustache( $message ), + ) ); } /** - * Intercept the response to a non-comment POST request. + * New error handler for AMP form submission. * * @since 0.7.0 + * @see wp_die() + * + * @param WP_Error|string $error The error to handle. + * @param string|int $title Optional. Error title. If `$message` is a `WP_Error` object, + * error data with the key 'title' may be used to specify the title. + * If `$title` is an integer, then it is treated as the response + * code. Default empty. + * @param string|array|int $args { + * Optional. Arguments to control behavior. If `$args` is an integer, then it is treated + * as the response code. Default empty array. + * + * @type int $response The HTTP response code. Default 200 for Ajax requests, 500 otherwise. + * } + */ + public static function handle_wp_die( $error, $title = '', $args = array() ) { + if ( is_int( $title ) ) { + $status_code = $title; + } elseif ( is_int( $args ) ) { + $status_code = $args; + } elseif ( is_array( $args ) && isset( $args['response'] ) ) { + $status_code = $args['response']; + } else { + $status_code = 500; + } + status_header( $status_code ); + + if ( is_wp_error( $error ) ) { + $error = $error->get_error_message(); + } + + // Message will be shown in template defined by AMP_Theme_Support::add_amp_comment_form_templates(). + wp_send_json( array( + 'error' => self::wp_kses_amp_mustache( $error ), + ) ); + } + + /** + * Intercept the response to a POST request. + * + * @since 0.7.0 + * @see wp_redirect() + * * @param string $location The location to redirect to. */ public static function intercept_post_request_redirect( $location ) { @@ -378,12 +499,16 @@ public static function intercept_post_request_redirect( $location ) { array( 'scheme' => 'https', 'host' => wp_parse_url( home_url(), PHP_URL_HOST ), - 'path' => strtok( wp_unslash( $_SERVER['REQUEST_URI'] ), '?' ), + 'path' => isset( $_SERVER['REQUEST_URI'] ) ? strtok( wp_unslash( $_SERVER['REQUEST_URI'] ), '?' ) : '/', ), wp_parse_url( $location ) ); - $absolute_location = $parsed_location['scheme'] . '://' . $parsed_location['host']; + $absolute_location = ''; + if ( 'https' === $parsed_location['scheme'] ) { + $absolute_location .= $parsed_location['scheme'] . ':'; + } + $absolute_location .= '//' . $parsed_location['host']; if ( isset( $parsed_location['port'] ) ) { $absolute_location .= ':' . $parsed_location['port']; } @@ -395,9 +520,9 @@ public static function intercept_post_request_redirect( $location ) { $absolute_location .= '#' . $parsed_location['fragment']; } - header( 'AMP-Redirect-To: ' . $absolute_location ); - header( 'Access-Control-Expose-Headers: AMP-Redirect-To' ); - // Send json success as no data is required. + self::send_header( 'AMP-Redirect-To', $absolute_location ); + self::send_header( 'Access-Control-Expose-Headers', 'AMP-Redirect-To' ); + wp_send_json_success(); } @@ -516,7 +641,7 @@ public static function add_amp_comment_form_templates() { ?>
diff --git a/tests/test-class-amp-theme-support.php b/tests/test-class-amp-theme-support.php index 44baeb8a294..e672e5e0cdf 100644 --- a/tests/test-class-amp-theme-support.php +++ b/tests/test-class-amp-theme-support.php @@ -25,6 +25,9 @@ public function tearDown() { remove_theme_support( 'amp' ); $_REQUEST = array(); // phpcs:ignore WordPress.CSRF.NonceVerification.NoNonceVerification $_SERVER['QUERY_STRING'] = ''; + unset( $_SERVER['REQUEST_URI'] ); + unset( $_SERVER['REQUEST_METHOD'] ); + AMP_Theme_Support::$headers_sent = array(); } /** @@ -266,69 +269,232 @@ public function test_purge_amp_query_vars() { // phpcs:enable WordPress.Security.NonceVerification.NoNonceVerification } + /** + * Test handle_xhr_request(). + * + * @covers AMP_Theme_Support::handle_xhr_request() + */ + public function test_handle_xhr_request() { + AMP_Theme_Support::purge_amp_query_vars(); + AMP_Theme_Support::handle_xhr_request(); + $this->assertEmpty( AMP_Theme_Support::$headers_sent ); + + // Try bad source origin. + $_GET['__amp_source_origin'] = 'http://evil.example.com/'; + $_SERVER['REQUEST_METHOD'] = 'POST'; + AMP_Theme_Support::purge_amp_query_vars(); + AMP_Theme_Support::handle_xhr_request(); + $this->assertEmpty( AMP_Theme_Support::$headers_sent ); + + // Try home source origin. + $_GET['__amp_source_origin'] = home_url(); + $_SERVER['REQUEST_METHOD'] = 'POST'; + AMP_Theme_Support::purge_amp_query_vars(); + AMP_Theme_Support::handle_xhr_request(); + $this->assertCount( 1, AMP_Theme_Support::$headers_sent ); + $this->assertEquals( + array( + 'name' => 'AMP-Access-Control-Allow-Source-Origin', + 'value' => home_url(), + 'replace' => true, + 'status_code' => null, + ), + AMP_Theme_Support::$headers_sent[0] + ); + $this->assertEquals( PHP_INT_MAX, has_filter( 'wp_redirect', array( 'AMP_Theme_Support', 'intercept_post_request_redirect' ) ) ); + $this->assertEquals( PHP_INT_MAX, has_filter( 'comment_post_redirect', array( 'AMP_Theme_Support', 'filter_comment_post_redirect' ) ) ); + $this->assertEquals( + array( 'AMP_Theme_Support', 'handle_wp_die' ), + apply_filters( 'wp_die_handler', '__return_true' ) + ); + } + + /** + * Test filter_comment_post_redirect(). + * + * @covers AMP_Theme_Support::filter_comment_post_redirect() + */ + public function test_filter_comment_post_redirect() { + add_filter( 'wp_doing_ajax', '__return_true' ); + add_filter( 'wp_die_ajax_handler', function() { + return '__return_null'; + } ); + + add_theme_support( 'amp' ); + $post = $this->factory()->post->create_and_get(); + $comment = $this->factory()->comment->create_and_get( array( + 'comment_post_ID' => $post->ID, + ) ); + $url = get_comment_link( $comment ); + + // Test without comments_live_list. + $filtered_url = AMP_Theme_Support::filter_comment_post_redirect( $url, $comment ); + $this->assertNotEquals( + strtok( $url, '#' ), + strtok( $filtered_url, '#' ) + ); + + // Test with comments_live_list. + add_theme_support( 'amp', array( + 'comments_live_list' => true, + ) ); + add_filter( 'amp_comment_posted_message', function( $message, WP_Comment $filter_comment ) { + return sprintf( '(comment=%d,approved=%d)', $filter_comment->comment_ID, $filter_comment->comment_approved ); + }, 10, 2 ); + + // Test approved comment. + $comment->comment_approved = '1'; + ob_start(); + AMP_Theme_Support::filter_comment_post_redirect( $url, $comment ); + $response = json_decode( ob_get_clean(), true ); + $this->assertArrayHasKey( 'message', $response ); + $this->assertEquals( + sprintf( '(comment=%d,approved=1)', $comment->comment_ID ), + $response['message'] + ); + + // Test moderated comment. + $comment->comment_approved = '0'; + ob_start(); + AMP_Theme_Support::filter_comment_post_redirect( $url, $comment ); + $response = json_decode( ob_get_clean(), true ); + $this->assertArrayHasKey( 'message', $response ); + $this->assertEquals( + sprintf( '(comment=%d,approved=0)', $comment->comment_ID ), + $response['message'] + ); + } + + /** + * Test handle_wp_die(). + * + * @covers AMP_Theme_Support::handle_wp_die() + */ + public function test_handle_wp_die() { + add_filter( 'wp_doing_ajax', '__return_true' ); + add_filter( 'wp_die_ajax_handler', function() { + return '__return_null'; + } ); + + ob_start(); + AMP_Theme_Support::handle_wp_die( 'string' ); + $this->assertEquals( '{"error":"string"}', ob_get_clean() ); + + ob_start(); + $error = new WP_Error( 'code', 'The Message' ); + AMP_Theme_Support::handle_wp_die( $error ); + $this->assertEquals( '{"error":"The Message"}', ob_get_clean() ); + } + /** * Test intercept_post_request_redirect(). * - * @runInSeparateProcess - * @preserveGlobalState disabled * @covers AMP_Theme_Support::intercept_post_request_redirect() */ public function test_intercept_post_request_redirect() { - if ( ! function_exists( 'xdebug_get_headers' ) ) { - $this->markTestSkipped( 'xdebug is required for this test' ); - } add_theme_support( 'amp' ); - $url = get_home_url(); + $url = home_url( '/', 'https' ); add_filter( 'wp_doing_ajax', '__return_true' ); add_filter( 'wp_die_ajax_handler', function () { return '__return_false'; } ); + // Test redirecting to full URL with HTTPS protocol. + AMP_Theme_Support::$headers_sent = array(); ob_start(); AMP_Theme_Support::intercept_post_request_redirect( $url ); $this->assertEquals( '{"success":true}', ob_get_clean() ); + $this->assertContains( + array( + 'name' => 'AMP-Redirect-To', + 'value' => $url, + 'replace' => true, + 'status_code' => null, + ), + AMP_Theme_Support::$headers_sent + ); + $this->assertContains( + array( + 'name' => 'Access-Control-Expose-Headers', + 'value' => 'AMP-Redirect-To', + 'replace' => true, + 'status_code' => null, + ), + AMP_Theme_Support::$headers_sent + ); - $this->assertContains( 'AMP-Redirect-To: ' . $url, xdebug_get_headers() ); - $this->assertContains( 'Access-Control-Expose-Headers: AMP-Redirect-To', xdebug_get_headers() ); + // Test redirecting to non-HTTPS URL. + AMP_Theme_Support::$headers_sent = array(); + ob_start(); + $url = home_url( '/', 'http' ); + AMP_Theme_Support::intercept_post_request_redirect( $url ); + $this->assertEquals( '{"success":true}', ob_get_clean() ); + $this->assertContains( + array( + 'name' => 'AMP-Redirect-To', + 'value' => preg_replace( '#^\w+:#', '', $url ), + 'replace' => true, + 'status_code' => null, + ), + AMP_Theme_Support::$headers_sent + ); + $this->assertContains( + array( + 'name' => 'Access-Control-Expose-Headers', + 'value' => 'AMP-Redirect-To', + 'replace' => true, + 'status_code' => null, + ), + AMP_Theme_Support::$headers_sent + ); + // Test redirecting to host-less location. + AMP_Theme_Support::$headers_sent = array(); ob_start(); AMP_Theme_Support::intercept_post_request_redirect( '/new-location/' ); $this->assertEquals( '{"success":true}', ob_get_clean() ); - $this->assertContains( 'AMP-Redirect-To: https://example.org/new-location/', xdebug_get_headers() ); + $this->assertContains( + array( + 'name' => 'AMP-Redirect-To', + 'value' => set_url_scheme( home_url( '/new-location/' ), 'https' ), + 'replace' => true, + 'status_code' => null, + ), + AMP_Theme_Support::$headers_sent + ); + // Test redirecting to scheme-less location. + AMP_Theme_Support::$headers_sent = array(); ob_start(); - AMP_Theme_Support::intercept_post_request_redirect( '//example.com/new-location/' ); + $url = home_url( '/new-location/' ); + AMP_Theme_Support::intercept_post_request_redirect( substr( $url, strpos( $url, ':' ) + 1 ) ); $this->assertEquals( '{"success":true}', ob_get_clean() ); - $headers = xdebug_get_headers(); - $this->assertContains( 'AMP-Redirect-To: https://example.com/new-location/', $headers ); + $this->assertContains( + array( + 'name' => 'AMP-Redirect-To', + 'value' => set_url_scheme( home_url( '/new-location/' ), 'https' ), + 'replace' => true, + 'status_code' => null, + ), + AMP_Theme_Support::$headers_sent + ); + // Test redirecting to empty location. + AMP_Theme_Support::$headers_sent = array(); ob_start(); AMP_Theme_Support::intercept_post_request_redirect( '' ); $this->assertEquals( '{"success":true}', ob_get_clean() ); - $this->assertContains( 'AMP-Redirect-To: https://example.org', xdebug_get_headers() ); - } - - /** - * Test handle_xhr_request(). - * - * @runInSeparateProcess - * @preserveGlobalState disabled - * @covers AMP_Theme_Support::handle_xhr_request() - */ - public function test_handle_xhr_request() { - global $pagenow; - if ( ! function_exists( 'xdebug_get_headers' ) ) { - $this->markTestSkipped( 'xdebug is required for this test' ); - } - - $_GET['__amp_source_origin'] = 'https://example.org'; - $pagenow = 'wp-comments-post.php'; - AMP_Theme_Support::purge_amp_query_vars(); - - AMP_Theme_Support::handle_xhr_request(); - $this->assertContains( 'AMP-Access-Control-Allow-Source-Origin: https://example.org', xdebug_get_headers() ); + $this->assertContains( + array( + 'name' => 'AMP-Redirect-To', + 'value' => set_url_scheme( home_url(), 'https' ), + 'replace' => true, + 'status_code' => null, + ), + AMP_Theme_Support::$headers_sent + ); } /**