diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index b18d0b9a470..567604fd37f 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -362,12 +362,40 @@ public static function display_invalid_url_validation_error_counts_summary( $pos /** * Gets the existing custom post that stores errors for the $url, if it exists. * - * @param string $url The (in)valid URL. + * @param string $url The (in)valid URL. + * @param array $options { + * Options. + * + * @type bool $normalize Whether to normalize the URL. + * @type bool $include_trashed Include trashed. + * } * @return WP_Post|null The post of the existing custom post, or null. */ - public static function get_invalid_url_post( $url ) { - $url = remove_query_arg( amp_get_slug(), $url ); - return get_page_by_path( md5( $url ), OBJECT, self::POST_TYPE_SLUG ); + public static function get_invalid_url_post( $url, $options = array() ) { + $default = array( + 'normalize' => true, + 'include_trashed' => false, + ); + $options = wp_parse_args( $options, $default ); + + if ( $options['normalize'] ) { + $url = self::normalize_url_for_storage( $url ); + } + $slug = md5( $url ); + + $post = get_page_by_path( $slug, OBJECT, self::POST_TYPE_SLUG ); + if ( $post ) { + return $post; + } + + if ( $options['include_trashed'] ) { + $post = get_page_by_path( $slug . '__trashed', OBJECT, self::POST_TYPE_SLUG ); + if ( $post ) { + return $post; + } + } + + return null; } /** @@ -385,9 +413,55 @@ public static function get_url_from_post( $post ) { return null; } $url = $post->post_title; + + // Add AMP query var if in paired mode. if ( ! amp_is_canonical() ) { $url = add_query_arg( amp_get_slug(), '', $url ); } + + // Set URL scheme based on whether HTTPS is current. + $url = set_url_scheme( $url, ( 'http' === wp_parse_url( home_url(), PHP_URL_SCHEME ) ) ? 'http' : 'https' ); + + return $url; + } + + /** + * Normalize a URL for storage. + * + * This ensures that query vars like utm_* and the like will not cause duplicates. + * The AMP query param is removed to facilitate switching between native and paired. + * The URL scheme is also normalized to HTTPS to help with transition from HTTP to HTTPS. + * + * @param string $url URL. + * @return string Normalized URL. + * @global WP $wp + */ + protected static function normalize_url_for_storage( $url ) { + global $wp; + + // Only ever store the canonical version. + $url = amp_remove_endpoint( $url ); + + // Remove fragment identifier in the rare case it could be provided. It is irrelevant for validation. + $url = strtok( $url, '#' ); + + // Normalize query args, removing all that are not recognized or which are removable. + $url_parts = explode( '?', $url, 2 ); + if ( 2 === count( $url_parts ) ) { + parse_str( $url_parts[1], $args ); + foreach ( wp_removable_query_args() as $removable_query_arg ) { + unset( $args[ $removable_query_arg ] ); + } + $args = wp_array_slice_assoc( $args, $wp->public_query_vars ); + $url = $url_parts[0]; + if ( ! empty( $args ) ) { + $url = $url_parts[0] . '?' . build_query( $args ); + } + } + + // Normalize the scheme as HTTPS. + $url = set_url_scheme( $url, 'https' ); + return $url; } @@ -408,17 +482,17 @@ public static function get_url_from_post( $post ) { * @global WP $wp */ public static function store_validation_errors( $validation_errors, $url, $args = array() ) { - $url = remove_query_arg( amp_get_slug(), $url ); // Only ever store the canonical version. + $url = self::normalize_url_for_storage( $url ); $slug = md5( $url ); $post = null; if ( ! empty( $args['invalid_url_post'] ) ) { $post = get_post( $args['invalid_url_post'] ); } if ( ! $post ) { - $post = get_page_by_path( $slug, OBJECT, self::POST_TYPE_SLUG ); - if ( ! $post ) { - $post = get_page_by_path( $slug . '__trashed', OBJECT, self::POST_TYPE_SLUG ); - } + $post = self::get_invalid_url_post( $url, array( + 'include_trashed' => true, + 'normalize' => false, // Since already normalized. + ) ); } /* @@ -1731,7 +1805,7 @@ public static function filter_post_row_actions( $actions, $post ) { '%s', get_delete_post_link( $post->ID ), /* translators: %s: post title */ - esc_attr( sprintf( __( 'Forget “%s”', 'amp' ), $post->post_title ) ), + esc_attr( sprintf( __( 'Forget “%s”', 'amp' ), self::get_url_from_post( $post ) ) ), esc_html__( 'Forget', 'amp' ) ); } @@ -1741,7 +1815,7 @@ public static function filter_post_row_actions( $actions, $post ) { '%s', get_delete_post_link( $post->ID, '', true ), /* translators: %s: post title */ - esc_attr( sprintf( __( 'Forget “%s” permanently', 'amp' ), $post->post_title ) ), + esc_attr( sprintf( __( 'Forget “%s” permanently', 'amp' ), self::get_url_from_post( $post ) ) ), esc_html__( 'Forget Permanently', 'amp' ) ); } diff --git a/tests/validation/test-class-amp-invalid-url-post-type.php b/tests/validation/test-class-amp-invalid-url-post-type.php index 406cb720e3a..ca9c74f4c6c 100644 --- a/tests/validation/test-class-amp-invalid-url-post-type.php +++ b/tests/validation/test-class-amp-invalid-url-post-type.php @@ -247,6 +247,28 @@ public function test_get_invalid_url_post() { $invalid_post_id, AMP_Invalid_URL_Post_Type::get_invalid_url_post( get_permalink( $post ) )->ID ); + + // Test trashed. + wp_trash_post( $invalid_post_id ); + $this->assertNull( AMP_Invalid_URL_Post_Type::get_invalid_url_post( get_permalink( $post ) ) ); + $args = array( 'include_trashed' => true ); + $this->assertEquals( + $invalid_post_id, + AMP_Invalid_URL_Post_Type::get_invalid_url_post( get_permalink( $post ), $args )->ID + ); + wp_untrash_post( $invalid_post_id ); + + // Test normalized. + $args = array( 'normalize' => false ); + $url = add_query_arg( 'utm_foo', 'bar', get_permalink( $post ) . '#baz' ); + $url = set_url_scheme( $url, 'http' ); + $this->assertNull( AMP_Invalid_URL_Post_Type::get_invalid_url_post( $url, $args ) ); + $args = array( 'normalize' => true ); + $this->assertEquals( $invalid_post_id, AMP_Invalid_URL_Post_Type::get_invalid_url_post( $url, $args )->ID ); + $this->assertEquals( $invalid_post_id, AMP_Invalid_URL_Post_Type::get_invalid_url_post( $url )->ID ); + $url = set_url_scheme( get_permalink( $post ), 'http' ); + $this->assertNull( AMP_Invalid_URL_Post_Type::get_invalid_url_post( $url, array( 'normalize' => false ) ) ); + $this->assertEquals( $invalid_post_id, AMP_Invalid_URL_Post_Type::get_invalid_url_post( $url, array( 'normalize' => true ) )->ID ); } /** @@ -280,6 +302,12 @@ public function test_get_url_from_post() { get_permalink( $post ), AMP_Invalid_URL_Post_Type::get_url_from_post( $invalid_post_id ) ); + + // Check URL scheme. + update_option( 'home', home_url( '/', 'http' ) ); + $this->assertEquals( 'http', wp_parse_url( AMP_Invalid_URL_Post_Type::get_url_from_post( $invalid_post_id ), PHP_URL_SCHEME ) ); + update_option( 'home', home_url( '/', 'https' ) ); + $this->assertEquals( 'https', wp_parse_url( AMP_Invalid_URL_Post_Type::get_url_from_post( $invalid_post_id ), PHP_URL_SCHEME ) ); } /** @@ -381,7 +409,7 @@ public function test_store_validation_errors() { ); $this->assertEquals( - home_url( '/something/else/' ), + home_url( '/something/else/', 'https' ), get_post( $invalid_url_post_id )->post_title ); @@ -1087,16 +1115,15 @@ public function test_print_url_as_title() { $this->assertEmpty( ob_get_clean() ); // The post has the correct type and a validation URL in the title, so this should output markup. - $url = 'https://example.com'; $post_correct_post_type = $this->factory()->post->create_and_get( array( 'post_type' => AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG, - 'post_title' => $url, + 'post_title' => home_url(), ) ); ob_start(); AMP_Invalid_URL_Post_Type::print_url_as_title( $post_correct_post_type ); $output = ob_get_clean(); $this->assertContains( '

', $output ); - $this->assertContains( $url, $output ); + $this->assertContains( home_url(), $output ); } /** @@ -1226,15 +1253,16 @@ public function test_filter_post_row_actions() { add_theme_support( 'amp' ); AMP_Validation_Manager::init(); + $validated_url = home_url( '/' ); $initial_actions = array( - 'trash' => 'Forget', + 'trash' => 'Forget', ); $invalid_post_id = AMP_Invalid_URL_Post_Type::store_validation_errors( array( array( 'code' => 'foo' ), ), - home_url( '/' ) + $validated_url ); $this->assertEquals( $initial_actions, AMP_Invalid_URL_Post_Type::filter_post_row_actions( $initial_actions, $this->factory()->post->create_and_get() ) );