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.

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

Add form sanitizer #907

Merged
merged 7 commits into from Jan 26, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions includes/amp-helper-functions.php
Expand Up @@ -225,6 +225,7 @@ function amp_get_content_sanitizers( $post = null ) {
'AMP_Iframe_Sanitizer' => array(
'add_placeholder' => true,
),
'AMP_Form_Sanitizer' => array(),
'AMP_Tag_And_Attribute_Sanitizer' => array(), // Note: This whitelist sanitizer must come at the end to clean up any remaining issues the other sanitizers didn't catch.
),
$post
Expand Down
1 change: 1 addition & 0 deletions includes/class-amp-autoloader.php
Expand Up @@ -65,6 +65,7 @@ class AMP_Autoloader {
'AMP_Blacklist_Sanitizer' => 'includes/sanitizers/class-amp-blacklist-sanitizer',
'AMP_Iframe_Sanitizer' => 'includes/sanitizers/class-amp-iframe-sanitizer',
'AMP_Img_Sanitizer' => 'includes/sanitizers/class-amp-img-sanitizer',
'AMP_Form_Sanitizer' => 'includes/sanitizers/class-amp-form-sanitizer',
'AMP_Playbuzz_Sanitizer' => 'includes/sanitizers/class-amp-playbuzz-sanitizer',
'AMP_Style_Sanitizer' => 'includes/sanitizers/class-amp-style-sanitizer',
'AMP_Tag_And_Attribute_Sanitizer' => 'includes/sanitizers/class-amp-tag-and-attribute-sanitizer',
Expand Down
111 changes: 111 additions & 0 deletions includes/sanitizers/class-amp-form-sanitizer.php
@@ -0,0 +1,111 @@
<?php
/**
* Class AMP_Form_Sanitizer.
*
* @package AMP
* @since 0.7
*/

/**
* Class AMP_Form_Sanitizer
*
* Strips and corrects attributes in forms.
*
* @since 0.7
*/
class AMP_Form_Sanitizer extends AMP_Base_Sanitizer {

/**
* Tag.
*
* @var string HTML <form> tag to identify and process.
*
* @since 0.7
*/
public static $tag = 'form';

/**
* Sanitize the <form> elements from the HTML contained in this instance's DOMDocument.
*
* @link https://www.ampproject.org/docs/reference/components/amp-form
* @since 0.7
*/
public function sanitize() {

/**
* Node list.
*
* @var DOMNodeList $node
*/
$nodes = $this->dom->getElementsByTagName( self::$tag );
$num_nodes = $nodes->length;

if ( 0 === $num_nodes ) {
return;
}

for ( $i = $num_nodes - 1; $i >= 0; $i-- ) {
$node = $nodes->item( $i );
if ( ! $node instanceof DOMElement ) {
continue;
}

// In HTML, the default method is 'get'.
$method = 'get';
if ( $node->getAttribute( 'method' ) ) {
$method = strtolower( $node->getAttribute( 'method' ) );
} else {
$node->setAttribute( 'method', $method );
}

/*
* In HTML, the default action is just the current URL that the page is served from.
* The action "specifies a server endpoint to handle the form input. The value must be an
* https URL and must not be a link to a CDN".
*/
if ( ! $node->getAttribute( 'action' ) ) {
$action_url = esc_url_raw( '//' . $_SERVER['HTTP_HOST'] . wp_unslash( $_SERVER['REQUEST_URI'] ) ); // WPCS: ignore. input var okay, sanitization ok.
} else {
$action_url = $node->getAttribute( 'action' );
}
$xhr_action = $node->getAttribute( 'action-xhr' );

// Make HTTP URLs protocol-less, since HTTPS is required for forms.
if ( 'http://' === strtolower( substr( $action_url, 0, 7 ) ) ) {
$action_url = substr( $action_url, 5 );
}

/*
* "For GET submissions, provide at least one of action or action-xhr".
* "This attribute is required for method=GET. For method=POST, the
* action attribute is invalid, use action-xhr instead".
*/
if ( 'get' === $method ) {
if ( $action_url !== $node->getAttribute( 'action' ) ) {
$node->setAttribute( 'action', $action_url );
}
} elseif ( 'post' === $method ) {
$node->removeAttribute( 'action' );
if ( ! $xhr_action ) {
$node->setAttribute( 'action-xhr', $action_url );
} elseif ( 'http://' === substr( $xhr_action, 0, 7 ) ) {
$node->setAttribute( 'action-xhr', substr( $xhr_action, 5 ) );
}
}

/*
* The target "indicates where to display the form response after submitting the form.
* The value must be _blank or _top". The _self and _parent values are treated
* as synonymous with _top, and anything else is treated like _blank.
*/
$target = $node->getAttribute( 'target' );
if ( '_top' !== $target ) {
if ( ! $target || in_array( $target, array( '_self', '_parent' ), true ) ) {
$node->setAttribute( 'target', '_top' );
} elseif ( '_blank' !== $target ) {
$node->setAttribute( 'target', '_blank' );
}
}
}
}
}
104 changes: 104 additions & 0 deletions tests/test-amp-form-sanitizer.php
@@ -0,0 +1,104 @@
<?php
/**
* Tests for form sanitisation.
*
* @package AMP
*/

/**
* Class AMP_Form_Sanitizer_Test
*
* @group amp-comments
* @group amp-form
*/
class AMP_Form_Sanitizer_Test extends WP_UnitTestCase {

/**
* Set up.
*/
public function setUp() {
parent::setUp();
$this->go_to( '/current-page/' );
}

/**
* Data strings for testing converter.
*
* @return array
*/
public function get_data() {
return array(
'no_form' => array(
'<p>Lorem Ipsum Demet Delorit.</p>',
null, // Same.
),
'form_with_get_method_http_action_and_no_target' => array(
'<form method="get" action="http://example.org/example-page/"></form>',
'<form method="get" action="//example.org/example-page/" target="_top"></form>',
),
'form_with_implicit_method_http_action_and_no_action_or_target' => array(
'<form></form>',
sprintf( '<form method="get" action="%s" target="_top"></form>', preg_replace( '#^https?:#', '', home_url( '/current-page/' ) ) ),
),
'form_with_empty_method_http_action_and_no_action_or_target' => array(
'<form method="" action="https://example.com/" target="_top"></form>',
'<form method="get" action="https://example.com/" target="_top"></form>',
),
'form_with_post_method_http_action_and_no_target' => array(
'<form method="post" action="http://example.org/example-page/"></form>',
'<form method="post" action-xhr="//example.org/example-page/" target="_top"></form>',
),
'form_with_post_method_http_action_and_blank_target' => array(
'<form method="post" action-xhr="http://example.org/example-page/" target="_blank"></form>',
'<form method="post" action-xhr="//example.org/example-page/" target="_blank"></form>',
),
'form_with_post_method_http_action_and_self_target' => array(
'<form method="get" action="https://example.org/" target="_self"></form>',
'<form method="get" action="https://example.org/" target="_top"></form>',
),
'form_with_post_method_https_action_and_custom_target' => array(
'<form method="post" action="https://example.org/" target="some_other_target"></form>',
'<form method="post" target="_blank" action-xhr="https://example.org/"></form>',
),
);
}

/**
* Test html conversion.
*
* @param string $source The source HTML.
* @param string|null $expected The expected HTML after conversion. Null means same as $source.
* @dataProvider get_data
*/
public function test_converter( $source, $expected = null ) {
if ( is_null( $expected ) ) {
$expected = $source;
}
$dom = AMP_DOM_Utils::get_dom_from_content( $source );

$sanitizer = new AMP_Form_Sanitizer( $dom );
$sanitizer->sanitize();

$whitelist_sanitizer = new AMP_Tag_And_Attribute_Sanitizer( $dom );
$whitelist_sanitizer->sanitize();

$content = AMP_DOM_Utils::get_content_from_dom( $dom );
$this->assertEquals( $expected, $content );
}

/**
* Test scripts.
*/
public function test_scripts() {
$source = '<form method="post" action-xhr="//example.org/example-page/" target="_top"></form>';
$expected = array( 'amp-form' => 'https://cdn.ampproject.org/v0/amp-form-latest.js' );

$dom = AMP_DOM_Utils::get_dom_from_content( $source );
$whitelist_sanitizer = new AMP_Tag_And_Attribute_Sanitizer( $dom );
$whitelist_sanitizer->sanitize();

$scripts = $whitelist_sanitizer->get_scripts();

$this->assertEquals( $expected, $scripts );
}
}