Skip to content

Commit

Permalink
Issue #842: Output validation errors in header.
Browse files Browse the repository at this point in the history 
If the query var of 'validate' is present,
Output the previous validation response,
like on the post.php page.
But also add a 'plugins_with_invalid_output' value.
This includes all of the plugins that output markup
which had removed elements or attributes.
  • Loading branch information
Ryan Kienstra committed Feb 20, 2018
1 parent 780b746 commit 33456a7
Show file tree
Hide file tree
Showing 4 changed files with 148 additions and 40 deletions.
1 change: 1 addition & 0 deletions includes/class-amp-theme-support.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -820,6 +820,7 @@ public static function prepare_response( $response, $args = array() ) {
trigger_error( esc_html( sprintf( __( 'The database has the %s encoding when it needs to be utf-8 to work with AMP.', 'amp' ), get_bloginfo( 'charset' ) ) ), E_USER_WARNING ); // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_trigger_error
}

AMP_Validation_Utils::add_header();
$response = "<!DOCTYPE html>\n";
$response .= AMP_DOM_Utils::get_content_from_dom_node( $dom, $dom->documentElement );

Expand Down
3 changes: 0 additions & 3 deletions includes/sanitizers/class-amp-style-sanitizer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -239,9 +239,6 @@ public function sanitize() {
$this->dom->createComment( sprintf( 'Skipped including %s stylesheet since too large.', $skip ) ),
$this->amp_custom_style_element->nextSibling
);
if ( isset( $this->args['remove_style_callback'] ) ) {
call_user_func( $this->args['remove_style_callback'], $skip );
}
}
}
}
Expand Down
64 changes: 57 additions & 7 deletions includes/utils/class-amp-validation-utils.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,20 @@ class AMP_Validation_Utils {
*/
const STYLE_SANITIZER = 'AMP_Style_Sanitizer';

/**
* Query var that triggers validation.
*
* @var string
*/
const VALIDATION_QUERY_VAR = 'validate';

/**
* Key of the callback to track invalid markup.
*
* @var string
*/
const CALLBACK_KEY = 'remove_invalid_callback';

/**
* The nodes that the sanitizer removed.
*
Expand All @@ -55,6 +69,8 @@ class AMP_Validation_Utils {
public static function init() {
add_action( 'rest_api_init', array( __CLASS__, 'amp_rest_validation' ) );
add_action( 'edit_form_top', array( __CLASS__, 'validate_content' ), 10, 2 );
add_action( 'wp', array( __CLASS__, 'callback_wrappers' ) );
add_filter( 'amp_content_sanitizers', array( __CLASS__, 'add_validation_callback' ) );
}

/**
Expand Down Expand Up @@ -129,7 +145,6 @@ public static function process_markup( $markup ) {
}

AMP_Theme_Support::register_content_embed_handlers();
add_filter( 'amp_content_sanitizers', array( __CLASS__, 'style_callback' ), 10, 2 );
remove_filter( 'the_content', 'wpautop' );

/** This filter is documented in wp-includes/post-template.php */
Expand Down Expand Up @@ -203,6 +218,9 @@ public static function get_response( $markup = null ) {
self::process_markup( $markup );
$response['processed_markup'] = $markup;
}
if ( self::do_validate_front_end() ) {
$response['plugins_with_invalid_output'] = self::$plugins_removed_nodes;
}

$removed_elements = array();
$removed_attributes = array();
Expand Down Expand Up @@ -298,6 +316,9 @@ public static function validate_content( $post ) {
*/
public static function callback_wrappers() {
global $wp_filter;
if ( ! self::do_validate_front_end() ) {
return;
}
foreach ( $wp_filter as $filter_tag => $wp_hook ) {
if ( 'query' === $filter_tag ) {
continue;
Expand Down Expand Up @@ -421,14 +442,43 @@ public static function display_error( $response ) {
}

/**
* Adds a callback to the style sanitizer, to track stylesheet removal.
* Whether to validate the front end response.
*
* @param array $sanitizers The content sanitizers.
* @return array $sanitizers The filtered content sanitizers.
* @return boolean
*/
public static function style_callback( $sanitizers ) {
if ( self::has_cap() && isset( $sanitizers[ self::STYLE_SANITIZER ] ) && is_array( $sanitizers[ self::STYLE_SANITIZER ] ) ) {
$sanitizers[ self::STYLE_SANITIZER ]['remove_style_callback'] = __CLASS__ . '::track_style';
public static function do_validate_front_end() {
$post = get_post();
return ( isset( $post ) && post_supports_amp( $post ) && self::has_cap() && ( isset( $_GET[ self::VALIDATION_QUERY_VAR ] ) ) ); // WPCS: CSRF ok.
}

/**
* Outputs AMP validation data in the response header of a frontend GET request.
*
* This must be called before the document output begins.
* Because the document is buffered,
* The sanitizers run after the 'send_headers' action.
* So it's not possible to call this function on that hook.
*
* @return void
*/
public static function add_header() {
if ( self::do_validate_front_end() ) {
header( sprintf( 'AMP-Validation-Error: %s', wp_json_encode( self::get_response() ) ) );
}
}

/**
* Adds the validation callback if front-end validation is needed.
*
* @param array $sanitizers The AMP sanitizers.
* @return array $sanitizers The filtered AMP sanitizers.
*/
public static function add_validation_callback( $sanitizers ) {
if ( self::do_validate_front_end() ) {
foreach ( $sanitizers as $sanitizer => $args ) {
$args[ self::CALLBACK_KEY ] = __CLASS__ . '::track_removed';
$sanitizers[ $sanitizer ] = $args;
}
}
return $sanitizers;
}
Expand Down
120 changes: 90 additions & 30 deletions tests/test-class-amp-validation-utils.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,11 @@ public function setUp() {
* @see AMP_Validation_Utils::init()
*/
public function test_init() {
AMP_Validation_Utils::init();
$this->assertEquals( 10, has_action( 'rest_api_init', 'AMP_Validation_Utils::amp_rest_validation' ) );
$this->assertEquals( 10, has_action( 'edit_form_top', 'AMP_Validation_Utils::validate_content' ) );
$this->assertEquals( 10, has_action( 'wp', 'AMP_Validation_Utils::callback_wrappers' ) );
$this->assertEquals( 10, has_action( 'amp_content_sanitizers', 'AMP_Validation_Utils::add_validation_callback' ) );
}

/**
Expand Down Expand Up @@ -170,10 +173,9 @@ public function test_was_node_removed() {
* @see AMP_Validation_Utils::process_markup()
*/
public function test_process_markup() {
$this->set_authorized();
$this->set_capability();
AMP_Validation_Utils::process_markup( $this->valid_amp_img );
$this->assertEquals( array(), AMP_Validation_Utils::$removed_nodes );
$this->assertEquals( 10, has_filter( 'amp_content_sanitizers', 'AMP_Validation_Utils::style_callback' ) );

AMP_Validation_Utils::reset_removed();
$video = '<video src="https://example.com/video">';
Expand Down Expand Up @@ -239,7 +241,7 @@ public function test_has_cap() {
) ) );
$this->assertFalse( AMP_Validation_Utils::has_cap() );

$this->set_authorized();
$this->set_capability();
$this->assertTrue( AMP_Validation_Utils::has_cap() );
}

Expand All @@ -249,7 +251,7 @@ public function test_has_cap() {
* @see AMP_Validation_Utils::validate_markup()
*/
public function test_validate_markup() {
$this->set_authorized();
$this->set_capability();
$request = new WP_REST_Request( 'POST', $this->expected_route );
$request->set_header( 'content-type', 'application/json' );
$request->set_body( wp_json_encode( array(
Expand Down Expand Up @@ -285,7 +287,7 @@ public function test_validate_markup() {
* @see AMP_Validation_Utils::get_response()
*/
public function test_get_response() {
$this->set_authorized();
$this->set_capability();
$response = AMP_Validation_Utils::get_response( $this->disallowed_tag );
$expected_response = array(
$this->error_key => true,
Expand Down Expand Up @@ -331,7 +333,7 @@ public function test_validate_arg() {
* @see AMP_Validation_Utils::validate_content()
*/
public function test_validate_content() {
$this->set_authorized();
$this->set_capability();
$post = $this->factory()->post->create_and_get();
ob_start();
AMP_Validation_Utils::validate_content( $post );
Expand Down Expand Up @@ -369,7 +371,8 @@ public function test_validate_content() {
*/
public function test_callback_wrappers() {
global $post;
$post = $this->factory()->post->create_and_get(); // WPCS: global override ok.
$post = $this->factory()->post->create_and_get(); // WPCS: global override ok.
$this->set_capability();
$action_non_plugin = 'foo_action';
$action_no_output = 'bar_action_no_output';
$action_no_argument = 'test_action_no_argument';
Expand All @@ -387,6 +390,7 @@ public function test_callback_wrappers() {
$this->assertEquals( 10, has_action( $action_one_argument, __CLASS__ . '::output_notice' ) );
$this->assertEquals( 10, has_action( $action_two_arguments, __CLASS__ . '::output_message' ) );

$_GET[ AMP_Validation_Utils::VALIDATION_QUERY_VAR ] = 1;
AMP_Validation_Utils::callback_wrappers();
$this->assertEquals( 10, has_action( $action_non_plugin, 'the_ID' ) );
$this->assertEquals( 10, has_action( $action_no_output, '__return_false' ) );
Expand Down Expand Up @@ -507,33 +511,12 @@ public function test_display_error() {
$this->assertContains( $removed_attribute, $output );
}

/**
* Test style_callback().
*
* @see AMP_Validation_Utils::style_callback()
*/
public function test_style_callback() {
$sanitizers = array(
AMP_Validation_Utils::STYLE_SANITIZER => array(),
);
$this->assertEquals( $sanitizers, AMP_Validation_Utils::style_callback( $sanitizers ) );

$this->set_authorized();
$expected = array(
AMP_Validation_Utils::STYLE_SANITIZER => array(
'remove_style_callback' => 'AMP_Validation_Utils::track_style',
),
);
$this->assertEquals( $expected, AMP_Validation_Utils::style_callback( $sanitizers ) );
AMP_Validation_Utils::reset_removed();
}

/**
* Add a nonce to the $_REQUEST, so that is_authorized() returns true.
*
* @return void
*/
public function set_authorized() {
public function set_capability() {
wp_set_current_user( $this->factory()->user->create( array(
'role' => 'administrator',
) ) );
Expand All @@ -552,6 +535,7 @@ public static function output_div() {
* Outputs a notice.
*
* @param string $message The message to output.
*
* @return void
*/
public static function output_notice( $message ) {
Expand All @@ -562,11 +546,87 @@ public static function output_notice( $message ) {
* Outputs a message with an excerpt.
*
* @param string $message The message to output.
* @param string $id The ID of the post.
* @param string $id The ID of the post.
*
* @return void
*/
public static function output_message( $message, $id ) {
printf( '<<p>%s</p><p>%s</p>', esc_attr( $message ), esc_html( $id ) );
}

/**
* Test add_header
*
* The headers are output by the time this function executes,
* And the tested method calls header().
* This produces an error.
* So simply check that the error message includes 'header,'
* meaning that the method called header() as expected.
*
* @see AMP_Validation_Utils::add_header()
*/
public function test_do_validate_front_end() {
global $post;
$post = $this->factory()->post->create(); // WPCS: global override ok.
add_theme_support( 'amp' );
$this->assertFalse( AMP_Validation_Utils::do_validate_front_end() );
$_GET[ AMP_Validation_Utils::VALIDATION_QUERY_VAR ] = 1;
$this->assertFalse( AMP_Validation_Utils::do_validate_front_end() );
$this->set_capability();
$this->assertTrue( AMP_Validation_Utils::do_validate_front_end() );
remove_theme_support( 'amp' );
}

/**
* Test add_header
*
* The headers are output by the time this function executes,
* And the tested method calls header().
* This produces an error.
* So simply check that the error message includes 'header,'
* meaning that the method called header() as expected.
*
* @see AMP_Validation_Utils::add_header()
*/
public function test_add_header() {
global $post;
$post = $this->factory()->post->create(); // WPCS: global override ok.
add_theme_support( 'amp' );
$this->set_capability();
$_GET[ AMP_Validation_Utils::VALIDATION_QUERY_VAR ] = 1;
AMP_Validation_Utils::process_markup( $this->disallowed_tag );
try {
AMP_Validation_Utils::add_header();
} catch ( Exception $exception ) {
$e = $exception;
}
$this->assertContains( 'header', $e->getMessage() );
remove_theme_support( 'amp' );
}

/**
* Test add_validation_callback
*
* @see AMP_Validation_Utils::add_validation_callback()
*/
public function test_add_validation_callback() {
global $post;
$post = $this->factory()->post->create_and_get(); // WPCS: global override ok.
$sanitizers = array(
'AMP_Img_Sanitizer' => array(),
'AMP_Form_Sanitizer' => array(),
'AMP_Comments_Sanitizer' => array(),
);
$expected_callback = 'AMP_Validation_Utils::track_removed';
$this->assertEquals( $sanitizers, AMP_Validation_Utils::add_validation_callback( $sanitizers ) );
add_theme_support( 'amp' );
$this->set_capability();
$_GET[ AMP_Validation_Utils::VALIDATION_QUERY_VAR ] = 1;
$filtered_sanitizers = AMP_Validation_Utils::add_validation_callback( $sanitizers );
foreach ( $filtered_sanitizers as $sanitizer => $args ) {
$this->assertEquals( $expected_callback, $args[ AMP_Validation_Utils::CALLBACK_KEY ] );
}
remove_theme_support( 'amp' );
}

}

0 comments on commit 33456a7

Please sign in to comment.