Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Normalize invalid URL stored for amp_invalid_url post type #1436

Merged
merged 1 commit into from
Sep 17, 2018
Merged

Normalize invalid URL stored for amp_invalid_url post type #1436

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 85 additions & 11 deletions includes/validation/class-amp-invalid-url-post-type.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand All @@ -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;
}

Expand All @@ -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.
) );
}

/*
Expand Down Expand Up @@ -1731,7 +1805,7 @@ public static function filter_post_row_actions( $actions, $post ) {
'<a href="%s" class="submitdelete" aria-label="%s">%s</a>',
get_delete_post_link( $post->ID ),
/* translators: %s: post title */
esc_attr( sprintf( __( 'Forget &#8220;%s&#8221;', 'amp' ), $post->post_title ) ),
esc_attr( sprintf( __( 'Forget &#8220;%s&#8221;', 'amp' ), self::get_url_from_post( $post ) ) ),
esc_html__( 'Forget', 'amp' )
);
}
Expand All @@ -1741,7 +1815,7 @@ public static function filter_post_row_actions( $actions, $post ) {
'<a href="%s" class="submitdelete" aria-label="%s">%s</a>',
get_delete_post_link( $post->ID, '', true ),
/* translators: %s: post title */
esc_attr( sprintf( __( 'Forget &#8220;%s&#8221; permanently', 'amp' ), $post->post_title ) ),
esc_attr( sprintf( __( 'Forget &#8220;%s&#8221; permanently', 'amp' ), self::get_url_from_post( $post ) ) ),
esc_html__( 'Forget Permanently', 'amp' )
);
}
Expand Down
40 changes: 34 additions & 6 deletions tests/validation/test-class-amp-invalid-url-post-type.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
}

/**
Expand Down Expand Up @@ -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 ) );
}

/**
Expand Down Expand Up @@ -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
);

Expand Down Expand Up @@ -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( '<h2 class="amp-invalid-url">', $output );
$this->assertContains( $url, $output );
$this->assertContains( home_url(), $output );
}

/**
Expand Down Expand Up @@ -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' => '<a href="" class="submitdelete" aria-label="Forget &#8220;http://example.org/&#8221;">Forget</a>',
'trash' => '<a href="" class="submitdelete" aria-label="Forget &#8220;' . $validated_url . '&#8221;">Forget</a>',
);

$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() ) );
Expand Down