Skip to content

Commit

Permalink
Remove process_markup method from validation manager
Browse files Browse the repository at this point in the history 
Fix additional static analysis issues
  • Loading branch information
@westonruter
westonruter committed May 27, 2018
1 parent 19aa607 commit 41570b0
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 55 deletions.
24 changes: 17 additions & 7 deletions includes/class-amp-theme-support.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,13 @@ public static function is_paired_available() {
return false;
}

if ( is_singular() && ! post_supports_amp( get_queried_object() ) ) {
/**
* Queried object.
*
* @var WP_Post $queried_object
*/
$queried_object = get_queried_object();
if ( is_singular() && ! post_supports_amp( $queried_object ) ) {
return false;
}

Expand Down Expand Up @@ -417,7 +423,7 @@ protected static function wp_kses_amp_mustache( $text ) {
*
* @param string $url Comment permalink to redirect to.
* @param WP_Comment $comment Posted comment.
* @return string URL.
* @return string|null URL if redirect to be done; otherwise function will exist.
*/
public static function filter_comment_post_redirect( $url, $comment ) {
$theme_support = get_theme_support( 'amp' );
Expand Down Expand Up @@ -452,6 +458,7 @@ public static function filter_comment_post_redirect( $url, $comment ) {
wp_send_json( array(
'message' => self::wp_kses_amp_mustache( $message ),
) );
return null;
}

/**
Expand Down Expand Up @@ -857,6 +864,13 @@ public static function print_amp_styles() {
* @param DOMDocument $dom Doc.
*/
public static function ensure_required_markup( DOMDocument $dom ) {
/**
* Elements.
*
* @var DOMElement $meta
* @var DOMElement $script
* @var DOMElement $link
*/
$head = $dom->getElementsByTagName( 'head' )->item( 0 );
if ( ! $head ) {
$head = $dom->createElement( 'head' );
Expand All @@ -865,11 +879,6 @@ public static function ensure_required_markup( DOMDocument $dom ) {
$meta_charset = null;
$meta_viewport = null;
foreach ( $head->getElementsByTagName( 'meta' ) as $meta ) {
/**
* Meta.
*
* @var DOMElement $meta
*/
if ( $meta->hasAttribute( 'charset' ) && 'utf-8' === strtolower( $meta->getAttribute( 'charset' ) ) ) { // @todo Also look for meta[http-equiv="Content-Type"]?
$meta_charset = $meta;
} elseif ( 'viewport' === $meta->getAttribute( 'name' ) ) {
Expand Down Expand Up @@ -1058,6 +1067,7 @@ public static function prepare_response( $response, $args = array() ) {
);

// Return cache if enabled and found.
$response_cache_key = null;
if ( true === $args['enable_response_caching'] ) {
// Set response cache hash, the data values dictates whether a new hash key should be generated or not.
$response_cache_key = md5( wp_json_encode( array(
Expand Down
7 changes: 4 additions & 3 deletions includes/validation/class-amp-validation-error-taxonomy.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -571,11 +571,12 @@ public static function add_admin_menu_validation_error_item() {
$menu_item_label .= ' <span class="awaiting-mod"><span class="pending-count">' . esc_html( number_format_i18n( $new_error_count ) ) . '</span></span>';
}

$taxonomy_caps = (object) get_taxonomy( self::TAXONOMY_SLUG )->cap; // Yes, cap is an object not an array.
add_submenu_page(
AMP_Options_Manager::OPTION_NAME,
esc_html__( 'Validation Errors', 'amp' ),
$menu_item_label,
get_taxonomy( self::TAXONOMY_SLUG )->cap->manage_terms, // Yes, cap is an object not an array.
$taxonomy_caps->manage_terms,
// The following esc_attr() is sadly needed due to <https://github.com/WordPress/wordpress-develop/blob/4.9.5/src/wp-admin/menu-header.php#L201>.
esc_attr( 'edit-tags.php?taxonomy=' . self::TAXONOMY_SLUG . '&post_type=' . AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG )
);
Expand Down Expand Up @@ -858,8 +859,8 @@ public static function handle_inline_edit_request() {
}
$action = sanitize_key( $_GET['action'] ); // WPCS: CSRF ok.
check_admin_referer( $action );
$tax = get_taxonomy( self::TAXONOMY_SLUG );
if ( ! current_user_can( $tax->cap->manage_terms ) ) { // Yes it is an object.
$taxonomy_caps = (object) get_taxonomy( self::TAXONOMY_SLUG )->cap; // Yes, cap is an object not an array.
if ( ! current_user_can( $taxonomy_caps->manage_terms ) ) {
return;
}

Expand Down
32 changes: 1 addition & 31 deletions includes/validation/class-amp-validation-manager.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -331,31 +331,6 @@ public static function get_amp_validity_rest_field( $post_data, $field_name, $re
return $field;
}

/**
* Processes markup, to determine AMP validity.
*
* Passes $markup through the AMP sanitizers.
* Also passes a 'validation_error_callback' to keep track of stripped attributes and nodes.
*
* @todo Eliminate since unused.
*
* @param string $markup The markup to process.
* @return string Sanitized markup.
*/
public static function process_markup( $markup ) {
AMP_Theme_Support::register_content_embed_handlers();

/** This filter is documented in wp-includes/post-template.php */
$markup = apply_filters( 'the_content', $markup );
$args = array(
'content_max_width' => ! empty( $content_width ) ? $content_width : AMP_Post_Template::CONTENT_MAX_WIDTH,
'validation_error_callback' => 'AMP_Validation_Manager::add_validation_error',
);

$results = AMP_Content_Sanitizer::sanitize( $markup, amp_get_content_sanitizers(), $args );
return $results[0];
}

/**
* Whether the user has the required capability.
*
Expand All @@ -370,12 +345,7 @@ public static function has_cap() {
/**
* Add validation error.
*
* @param array $error {
* Data.
*
* @type string $code Error code.
* @type DOMElement|DOMNode $node The removed node.
* }
* @param array $error Error info, especially code.
* @param array $data Additional data, including the node.
*
* @return bool Whether the validation error should result in sanitization.
Expand Down
49 changes: 35 additions & 14 deletions tests/test-class-amp-validation-utils.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -295,42 +295,63 @@ public function test_was_node_removed() {
}

/**
* Test process_markup.
* Processes markup, to determine AMP validity.
*
* Passes $markup through the AMP sanitizers.
* Also passes a 'validation_error_callback' to keep track of stripped attributes and nodes.
*
* @covers AMP_Validation_Manager::process_markup()
* @param string $markup The markup to process.
* @return string Sanitized markup.
*/
public function process_markup( $markup ) {
AMP_Theme_Support::register_content_embed_handlers();

/** This filter is documented in wp-includes/post-template.php */
$markup = apply_filters( 'the_content', $markup );
$args = array(
'content_max_width' => ! empty( $content_width ) ? $content_width : AMP_Post_Template::CONTENT_MAX_WIDTH,
'validation_error_callback' => 'AMP_Validation_Manager::add_validation_error',
);

$results = AMP_Content_Sanitizer::sanitize( $markup, amp_get_content_sanitizers(), $args );
return $results[0];
}

/**
* Test process_markup.
*/
public function test_process_markup() {
add_filter( 'amp_validation_error_sanitized', '__return_true' );

$this->set_capability();
AMP_Validation_Manager::process_markup( $this->valid_amp_img );
$this->process_markup( $this->valid_amp_img );
$this->assertEquals( array(), AMP_Validation_Manager::$validation_results );

AMP_Validation_Manager::reset_validation_results();
$video = '<video src="https://example.com/video">';
AMP_Validation_Manager::process_markup( $video );
$this->process_markup( $video );
// This isn't valid AMP, but the sanitizer should convert it to an <amp-video>, without stripping anything.
$this->assertEquals( array(), AMP_Validation_Manager::$validation_results );

AMP_Validation_Manager::reset_validation_results();

AMP_Validation_Manager::process_markup( $this->disallowed_tag );
$this->process_markup( $this->disallowed_tag );
$this->assertCount( 1, AMP_Validation_Manager::$validation_results );
$this->assertEquals( 'script', AMP_Validation_Manager::$validation_results[0]['error']['node_name'] );

AMP_Validation_Manager::reset_validation_results();
$disallowed_style = '<div style="display:none"></div>';
AMP_Validation_Manager::process_markup( $disallowed_style );
$this->process_markup( $disallowed_style );
$this->assertEquals( array(), AMP_Validation_Manager::$validation_results );

AMP_Validation_Manager::reset_validation_results();
$invalid_video = '<video width="200" height="100"></video>';
AMP_Validation_Manager::process_markup( $invalid_video );
$this->process_markup( $invalid_video );
$this->assertCount( 1, AMP_Validation_Manager::$validation_results );
$this->assertEquals( 'video', AMP_Validation_Manager::$validation_results[0]['error']['node_name'] );
AMP_Validation_Manager::reset_validation_results();

AMP_Validation_Manager::process_markup( '<button onclick="evil()">Do it</button>' );
$this->process_markup( '<button onclick="evil()">Do it</button>' );
$this->assertCount( 1, AMP_Validation_Manager::$validation_results );
$this->assertEquals( 'onclick', AMP_Validation_Manager::$validation_results[0]['error']['node_name'] );
AMP_Validation_Manager::reset_validation_results();
Expand Down Expand Up @@ -361,7 +382,7 @@ public function test_summarize_validation_errors() {

global $post;
$post = $this->factory()->post->create_and_get(); // WPCS: global override ok.
AMP_Validation_Manager::process_markup( $this->disallowed_tag );
$this->process_markup( $this->disallowed_tag );
$response = AMP_Validation_Manager::summarize_validation_errors( wp_list_pluck( AMP_Validation_Manager::$validation_results, 'error' ) );
AMP_Validation_Manager::reset_validation_results();
$expected_response = array(
Expand Down Expand Up @@ -864,7 +885,7 @@ public function test_store_validation_errors() {
global $post;
$post = $this->factory()->post->create_and_get(); // WPCS: global override ok.
add_theme_support( 'amp' );
AMP_Validation_Manager::process_markup( '<!--amp-source-stack {"type":"plugin","name":"foo"}-->' . $this->disallowed_tag . '<!--/amp-source-stack {"type":"plugin","name":"foo"}-->' );
$this->process_markup( '<!--amp-source-stack {"type":"plugin","name":"foo"}-->' . $this->disallowed_tag . '<!--/amp-source-stack {"type":"plugin","name":"foo"}-->' );

$this->assertCount( 1, AMP_Validation_Manager::$validation_results );
$this->assertEquals( 'script', AMP_Validation_Manager::$validation_results[0]['error']['node_name'] );
Expand Down Expand Up @@ -896,7 +917,7 @@ public function test_store_validation_errors() {

AMP_Validation_Manager::reset_validation_results();
$url = home_url( '/?baz' );
AMP_Validation_Manager::process_markup( '<!--amp-source-stack {"type":"plugin","name":"foo"}-->' . $this->disallowed_tag . '<!--/amp-source-stack {"type":"plugin","name":"foo"}-->' );
$this->process_markup( '<!--amp-source-stack {"type":"plugin","name":"foo"}-->' . $this->disallowed_tag . '<!--/amp-source-stack {"type":"plugin","name":"foo"}-->' );
$custom_post_id = AMP_Validation_Manager::store_validation_errors( wp_list_pluck( AMP_Validation_Manager::$validation_results, 'error' ), $url );
AMP_Validation_Manager::reset_validation_results();
$meta = get_post_meta( $post_id, AMP_Validation_Manager::AMP_URL_META, false );
Expand All @@ -905,7 +926,7 @@ public function test_store_validation_errors() {
$this->assertContains( $url, $meta );

$url = home_url( '/?foo-bar' );
AMP_Validation_Manager::process_markup( '<!--amp-source-stack {"type":"plugin","name":"foo"}-->' . $this->disallowed_tag . '<!--/amp-source-stack {"type":"plugin","name":"foo"}-->' );
$this->process_markup( '<!--amp-source-stack {"type":"plugin","name":"foo"}-->' . $this->disallowed_tag . '<!--/amp-source-stack {"type":"plugin","name":"foo"}-->' );
$custom_post_id = AMP_Validation_Manager::store_validation_errors( wp_list_pluck( AMP_Validation_Manager::$validation_results, 'error' ), $url );
AMP_Validation_Manager::reset_validation_results();
$meta = get_post_meta( $post_id, AMP_Validation_Manager::AMP_URL_META, false );
Expand All @@ -915,7 +936,7 @@ public function test_store_validation_errors() {
$this->assertContains( $url, $meta );

AMP_Validation_Manager::reset_validation_results();
AMP_Validation_Manager::process_markup( '<!--amp-source-stack {"type":"plugin","name":"foo"}--><nonexistent></nonexistent><!--/amp-source-stack {"type":"plugin","name":"foo"}-->' );
$this->process_markup( '<!--amp-source-stack {"type":"plugin","name":"foo"}--><nonexistent></nonexistent><!--/amp-source-stack {"type":"plugin","name":"foo"}-->' );
$custom_post_id = AMP_Validation_Manager::store_validation_errors( wp_list_pluck( AMP_Validation_Manager::$validation_results, 'error' ), $url );
AMP_Validation_Manager::reset_validation_results();
$error_post = get_post( $custom_post_id );
Expand All @@ -930,7 +951,7 @@ public function test_store_validation_errors() {
$this->assertContains( $url, get_post_meta( $custom_post_id, AMP_Validation_Manager::AMP_URL_META, false ) );

AMP_Validation_Manager::reset_validation_results();
AMP_Validation_Manager::process_markup( $this->valid_amp_img );
$this->process_markup( $this->valid_amp_img );

// There are no errors, so the existing error post should be deleted.
$custom_post_id = AMP_Validation_Manager::store_validation_errors( wp_list_pluck( AMP_Validation_Manager::$validation_results, 'error' ), $url );
Expand Down

0 comments on commit 41570b0

Please sign in to comment.