Fix conversion of forms with relative action URLs (#4003)

* Fix conversion of forms with relative action URLs * Re-use schemeless-host variable Co-Authored-By: Pierre Gordon <16200219+pierlon@users.noreply.github.com> Co-authored-by: Pierre Gordon <16200219+pierlon@users.noreply.github.com>
ampproject · Dec 22, 2019 · c26b635 · c26b635
1 parent ebead4f
commit c26b635
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 3 deletions.
diff --git a/includes/sanitizers/class-amp-form-sanitizer.php b/includes/sanitizers/class-amp-form-sanitizer.php
@@ -67,9 +67,21 @@ public function sanitize() {
 				$action_url = esc_url_raw( '//' . $_SERVER['HTTP_HOST'] . wp_unslash( $_SERVER['REQUEST_URI'] ) );
 			} else {
 				$action_url = $node->getAttribute( 'action' );
-				// Check if action_url is a relative path and add the host to it.
+
+				// Handle relative URLs.
 				if ( ! preg_match( '#^(https?:)?//#', $action_url ) ) {
-					$action_url = esc_url_raw( '//' . $_SERVER['HTTP_HOST'] . $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 );
 				}
 			}
 			$xhr_action = $node->getAttribute( 'action-xhr' );

diff --git a/tests/php/test-amp-form-sanitizer.php b/tests/php/test-amp-form-sanitizer.php
@@ -93,7 +93,18 @@ public function get_data() {
 				'<form method="post" action="/login/"></form>',
 				'#' . preg_quote( '<form method="post" action-xhr="//example.org/login/?_wp_amp_action_xhr_converted=1" target="_top">', '#' ) . $form_template_pattern . '</form>#s',
 			],
-
+			'form_with_relative_path_action_url' => [
+				'<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_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_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',
+			],
 			'test_with_dev_mode' => [
 				'<form data-ampdevmode="" action="javascript:"></form>',
 				null, // No change.