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

[WIP] Add dynamic handling of validation errors #1063

Closed
wants to merge 3 commits into from
Closed

[WIP] Add dynamic handling of validation errors #1063

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7bd658d
Explicitly only use (short) strings for description
westonruter Apr 6, 2018
73f4373
Allow the validation_error_callback to return false to prevent saniti…
westonruter Apr 6, 2018
f11e6c4
Improve handling of validation errors in style sanitizer
westonruter Apr 7, 2018
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
4 changes: 3 additions & 1 deletion amp.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,9 @@ function amp_init() {

add_rewrite_endpoint( amp_get_slug(), EP_PERMALINK );

AMP_Validation_Utils::init();
AMP_Validation_Utils::init( array(
'debug' => isset( $_REQUEST[ AMP_Validation_Utils::DEBUG_QUERY_VAR ] ), // phpcs:ignore WordPress.CSRF.NonceVerification.NoNonceVerification
) );
AMP_Theme_Support::init();
AMP_Post_Type_Support::add_post_type_support();
add_filter( 'request', 'amp_force_query_var_value' );
Expand Down
5 changes: 1 addition & 4 deletions includes/class-amp-response-headers.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,6 @@ public static function send_header( $name, $value, $args = array() ) {
* Send Server-Timing header.
*
* @since 1.0
* @todo What is the ordering in Chrome dev tools? What are the colors about?
* @todo Is there a better name standardization?
* @todo Is there a way to indicate nested server timings, so an outer method's own time can be seen separately from the inner method's time?
*
* @param string $name Name.
* @param float $duration Duration. If negative, will be added to microtime( true ). Optional.
Expand All @@ -75,7 +72,7 @@ public static function send_header( $name, $value, $args = array() ) {
public static function send_server_timing( $name, $duration = null, $description = null ) {
$value = $name;
if ( isset( $description ) ) {
$value .= sprintf( ';desc=%s', wp_json_encode( $description ) );
$value .= sprintf( ';desc="%s"', str_replace( array( '\\', '"' ), '', substr( $description, 0, 100 ) ) );
}
if ( isset( $duration ) ) {
if ( $duration < 0 ) {
Expand Down
17 changes: 6 additions & 11 deletions includes/class-amp-theme-support.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -604,7 +604,7 @@ public static function amend_comment_form() {
* @see get_query_template()
*
* @param array $templates Template hierarchy.
* @returns array Templates.
* @return array Templates.
*/
public static function filter_paired_template_hierarchy( $templates ) {
$support = get_theme_support( 'amp' );
Expand Down Expand Up @@ -991,15 +991,12 @@ public static function prepare_response( $response, $args = array() ) {
return $response;
}

$is_validation_debug_mode = ! empty( $_REQUEST[ AMP_Validation_Utils::DEBUG_QUERY_VAR ] ); // WPCS: csrf ok.

$args = array_merge(
array(
'content_max_width' => ! empty( $content_width ) ? $content_width : AMP_Post_Template::CONTENT_MAX_WIDTH, // Back-compat.
'use_document_element' => true,
'allow_dirty_styles' => self::is_customize_preview_iframe(), // Dirty styles only needed when editing (e.g. for edit shortcodes).
'allow_dirty_scripts' => is_customize_preview(), // Scripts are always needed to inject changeset UUID.
'disable_invalid_removal' => $is_validation_debug_mode,
'content_max_width' => ! empty( $content_width ) ? $content_width : AMP_Post_Template::CONTENT_MAX_WIDTH, // Back-compat.
'use_document_element' => true,
'allow_dirty_styles' => self::is_customize_preview_iframe(), // Dirty styles only needed when editing (e.g. for edit shortcodes).
'allow_dirty_scripts' => is_customize_preview(), // Scripts are always needed to inject changeset UUID.
),
$args
);
Expand Down Expand Up @@ -1051,9 +1048,7 @@ public static function prepare_response( $response, $args = array() ) {
}

if ( AMP_Validation_Utils::should_validate_response() ) {
AMP_Validation_Utils::finalize_validation( $dom, array(
'remove_source_comments' => ! $is_validation_debug_mode,
) );
AMP_Validation_Utils::finalize_validation( $dom );
}

$response = "<!DOCTYPE html>\n";
Expand Down
26 changes: 17 additions & 9 deletions includes/sanitizers/class-amp-base-sanitizer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ abstract class AMP_Base_Sanitizer {
* @type array $amp_bind_placeholder_prefix
* @type bool $allow_dirty_styles
* @type bool $allow_dirty_scripts
* @type bool $disable_invalid_removal
* @type callable $validation_error_callback
* }
*/
Expand Down Expand Up @@ -290,18 +289,22 @@ public function maybe_enforce_https_src( $src, $force_https = false ) {
*
* @param DOMNode|DOMElement $node The node to remove.
* @param array $args Additional args to pass to validation error callback.
*
* @return void
* @return bool Whether the node should have been removed, that is, that the node was sanitized for validity.
*/
public function remove_invalid_child( $node, $args = array() ) {
$should_remove = true;
if ( isset( $this->args['validation_error_callback'] ) ) {
call_user_func( $this->args['validation_error_callback'],
$result = call_user_func( $this->args['validation_error_callback'],
array_merge( compact( 'node' ), $args )
);
if ( is_bool( $result ) ) {
$should_remove = $result;
}
}
if ( empty( $this->args['disable_invalid_removal'] ) ) {
if ( $should_remove ) {
$node->parentNode->removeChild( $node );
}
return $should_remove;
}

/**
Expand All @@ -315,32 +318,37 @@ public function remove_invalid_child( $node, $args = array() ) {
* @param DOMElement $element The node for which to remove the attribute.
* @param DOMAttr|string $attribute The attribute to remove from the element.
* @param array $args Additional args to pass to validation error callback.
* @return void
* @return bool Whether the node should have been removed, that is, that the node was sanitized for validity.
*/
public function remove_invalid_attribute( $element, $attribute, $args = array() ) {
$should_remove = true;
if ( isset( $this->args['validation_error_callback'] ) ) {
if ( is_string( $attribute ) ) {
$attribute = $element->getAttributeNode( $attribute );
}
if ( $attribute ) {
call_user_func( $this->args['validation_error_callback'],
$result = call_user_func( $this->args['validation_error_callback'],
array_merge(
array(
'node' => $attribute,
),
$args
)
);
if ( empty( $this->args['disable_invalid_removal'] ) ) {
if ( is_bool( $result ) ) {
$should_remove = $result;
}
if ( $should_remove ) {
$element->removeAttributeNode( $attribute );
}
}
} elseif ( empty( $this->args['disable_invalid_removal'] ) ) {
} else {
if ( is_string( $attribute ) ) {
$element->removeAttribute( $attribute );
} else {
$element->removeAttributeNode( $attribute );
}
}
return $should_remove;
}
}
Loading