Skip to content

Commit

Permalink
Fix malformed conversion of relative action URLs for forms (#4250)
Browse files Browse the repository at this point in the history 
* Fix malformed conversion of relative action URLs for forms

* Simplify URL reconstruction by eliminating ternary expressions

* Add failing case for pathless URL

* Refactor determining action url to separate function

* Set an empty path if none is defined but there is a host

* Short-circuit when get_action_url has nothing to do

* Add test for dotless relative path action URL

Co-authored-by: Weston Ruter <westonruter@google.com>
  • Loading branch information
@pierlon @westonruter
pierlon and westonruter authored Feb 9, 2020
1 parent 224c5c2 commit 91cea86
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 26 deletions.
103 changes: 77 additions & 26 deletions includes/sanitizers/class-amp-form-sanitizer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,32 +58,8 @@ public function sanitize() {
$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'] ) );
} else {
$action_url = $node->getAttribute( 'action' );

// Handle relative URLs.
if ( ! preg_match( '#^(https?:)?//#', $action_url ) ) {
$schemeless_host = '//' . $_SERVER['HTTP_HOST'];
if ( '?' === $action_url[0] || '#' === $action_url[0] ) {
// For actions consisting of only a query or URL fragment, include the schemeless-host and the REQUEST URI of the current page.
$action_url = $schemeless_host . wp_unslash( $_SERVER['REQUEST_URI'] ) . $action_url;
} elseif ( '.' === $action_url[0] ) {
// For actions consisting of relative paths (e.g. '../'), prepend the schemeless-host and a trailing-slashed REQUEST URI.
$action_url = $schemeless_host . trailingslashit( wp_unslash( $_SERVER['REQUEST_URI'] ) ) . $action_url;
} else {
// Otherwise, when the action URL includes an absolute path, just append it to the schemeless-host.
$action_url = $schemeless_host . $action_url;
}
$action_url = esc_url_raw( $action_url );
}
}
$action_url = $this->get_action_url( $node->getAttribute( 'action' ) );

$xhr_action = $node->getAttribute( 'action-xhr' );

// Make HTTP URLs protocol-less, since HTTPS is required for forms.
Expand Down Expand Up @@ -133,6 +109,81 @@ public function sanitize() {
}
}

/**
* Get the action URL for the form element.
*
* @param string $action_url Action URL.
* @return string Action URL.
*/
protected function get_action_url( $action_url ) {
/*
* 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 ( ! $action_url ) {
return esc_url_raw( '//' . $_SERVER['HTTP_HOST'] . wp_unslash( $_SERVER['REQUEST_URI'] ) );
}

$parsed_url = wp_parse_url( $action_url );

if (
// Ignore a malformed URL - it will be later sanitized.
false === $parsed_url
||
// Ignore HTTPS URLs, because there is nothing left to do.
( isset( $parsed_url['scheme'] ) && 'https' === $parsed_url['scheme'] )
||
// Ignore protocol-relative URLs, because there is also nothing left to do.
( ! isset( $parsed_url['scheme'] ) && isset( $parsed_url['host'] ) )
) {
return $action_url;
}

// Make URL protocol relative.
$parsed_url['scheme'] = '//';

// Set an empty path if none is defined but there is a host.
if ( ! isset( $parsed_url['path'] ) && isset( $parsed_url['host'] ) ) {
$parsed_url['path'] = '';
}

if ( ! isset( $parsed_url['host'] ) ) {
$parsed_url['host'] = $_SERVER['HTTP_HOST'];
}

if ( ! isset( $parsed_url['path'] ) ) {
// If there is action URL path, use the one from the request.
$parsed_url['path'] = trailingslashit( wp_unslash( $_SERVER['REQUEST_URI'] ) );
} elseif ( '' !== $parsed_url['path'] && '/' !== $parsed_url['path'][0] ) {
// If the path is relative, append it to the current request path.
$parsed_url['path'] = trailingslashit( wp_unslash( $_SERVER['REQUEST_URI'] ) ) . trailingslashit( $parsed_url['path'] );
}

// Rebuild the URL.
$action_url = $parsed_url['scheme'];
if ( isset( $parsed_url['user'] ) ) {
$action_url .= $parsed_url['user'];
if ( isset( $parsed_url['pass'] ) ) {
$action_url .= ':' . $parsed_url['pass'];
}
$action_url .= '@';
}
$action_url .= $parsed_url['host'];
if ( isset( $parsed_url['port'] ) ) {
$action_url .= ':' . $parsed_url['port'];
}
$action_url .= $parsed_url['path'];
if ( isset( $parsed_url['query'] ) ) {
$action_url .= '?' . $parsed_url['query'];
}
if ( isset( $parsed_url['fragment'] ) ) {
$action_url .= '#' . $parsed_url['fragment'];
}

return esc_url_raw( $action_url );
}

/**
* Ensure that the form has a submit-success and submit-error element templates.
*
Expand Down
28 changes: 28 additions & 0 deletions tests/php/test-amp-form-sanitizer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,18 @@ public function get_data() {
'<form method="get" action="http://example.org/example-page/"></form>',
'<form method="get" action="//example.org/example-page/" target="_top"></form>',
],
'form_with_http_action_and_port' => [
'<form method="get" action="http://example.org:8080/example-page/"></form>',
'<form method="get" action="//example.org:8080/example-page/" target="_top"></form>',
],
'form_with_http_action_and_user' => [
'<form method="get" action="http://user@example.org:8080/example-page/"></form>',
'<form method="get" action="//user@example.org:8080/example-page/" target="_top"></form>',
],
'form_with_http_action_and_user_pass' => [
'<form method="get" action="http://user:pass@example.org:8080/example-page/"></form>',
'<form method="get" action="//user:pass@example.org:8080/example-page/" target="_top"></form>',
],
'form_with_implicit_method_http_action_and_no_action_or_target' => [
'<form></form>',
sprintf( '<form method="get" action="%s" target="_top"></form>', preg_replace( '#^https?:#', '', home_url( '/current-page/' ) ) ),
Expand Down Expand Up @@ -97,14 +109,30 @@ public function get_data() {
'<form method="post" action="../"></form>',
'#' . preg_quote( '<form method="post" action-xhr="//example.org/current-page/../?_wp_amp_action_xhr_converted=1" target="_top">', '#' ) . $form_template_pattern . '</form>#s',
],
'form_with_another_relative_path_action_url' => [
'<form method="post" action="foo/"></form>',
'#' . preg_quote( '<form method="post" action-xhr="//example.org/current-page/foo/?_wp_amp_action_xhr_converted=1" target="_top">', '#' ) . $form_template_pattern . '</form>#s',
],
'form_with_relative_query_action_url' => [
'<form method="post" action="?foo=bar"></form>',
'#' . preg_quote( '<form method="post" action-xhr="//example.org/current-page/?foo=bar&amp;_wp_amp_action_xhr_converted=1" target="_top">', '#' ) . $form_template_pattern . '</form>#s',
],
'form_with_multiple_relative_queries_action_url' => [
'<form method="post" action="?foo=bar&baz=buzz"></form>',
'#' . preg_quote( '<form method="post" action-xhr="//example.org/current-page/?foo=bar&amp;baz=buzz&amp;_wp_amp_action_xhr_converted=1" target="_top">', '#' ) . $form_template_pattern . '</form>#s',
],
'form_with_relative_fragment_action_url' => [
'<form method="post" action="#foo"></form>',
'#' . preg_quote( '<form method="post" action-xhr="//example.org/current-page/?_wp_amp_action_xhr_converted=1#foo" target="_top">', '#' ) . $form_template_pattern . '</form>#s',
],
'form_with_relative_query_and_fragment_action_url' => [
'<form method="post" action="?foo=bar#baz"></form>',
'#' . preg_quote( '<form method="post" action-xhr="//example.org/current-page/?foo=bar&amp;_wp_amp_action_xhr_converted=1#baz" target="_top">', '#' ) . $form_template_pattern . '</form>#s',
],
'form_with_pathless_url' => [
'<form method="post" action="//example.com"></form>',
'#' . preg_quote( '<form method="post" action-xhr="//example.com?_wp_amp_action_xhr_converted=1" target="_top">', '#' ) . $form_template_pattern . '</form>#s',
],
'test_with_dev_mode' => [
'<form data-ampdevmode="" action="javascript:"></form>',
null, // No change.
Expand Down

0 comments on commit 91cea86

Please sign in to comment.