Skip to content

Commit

Permalink
Merge pull request #907 from Automattic/add/form-sanitization
Browse files Browse the repository at this point in the history 
Add form sanitizer
  • Loading branch information
@westonruter
westonruter committed Jan 26, 2018
2 parents f877050 + 8263f74 commit ed9e327
Show file tree
Hide file tree
Showing 4 changed files with 217 additions and 0 deletions.
1 change: 1 addition & 0 deletions includes/amp-helper-functions.php
View file
Original file line number Diff line number Diff line change
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
View file
Original file line number Diff line number Diff line change
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
View file
Original file line number Diff line number Diff line change
@@ -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
View file
Original file line number Diff line number Diff line change
@@ -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 );
}
}

0 comments on commit ed9e327

Please sign in to comment.