Skip to content

Commit

Permalink
Fix ampproject#1349 relative path issue in action
Browse files Browse the repository at this point in the history 
Detect if action URL in the form is relative and format it properly
  • Loading branch information
@ricardobrg
ricardobrg authored Aug 22, 2018
1 parent 274971c commit fae8ab9
Showing 1 changed file with 5 additions and 0 deletions.
5 changes: 5 additions & 0 deletions includes/sanitizers/class-amp-form-sanitizer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@ public function sanitize() {
$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' );
//check if action_url is a relative path and add the host to it
preg_match('(^\/?[^\/].*)',$action_url,$matches);

This comment has been minimized.

Sign in to view

Copy link
@westonruter

westonruter Aug 22, 2018

I suggest instead:

if ( ! preg_match( '#^(https?:)?//#', $action_url ) ) {
    $action_url = esc_url_raw( '//' . $_SERVER['HTTP_HOST'] . $action_url );
}

In actuality, we should also make sure that if it starts with http: that it should also get that stripped out, since forms must be submitted over HTTPS in AMP.

This comment has been minimized.

Sign in to view

Copy link
@ricardobrg

ricardobrg Aug 22, 2018 •
edited
Loading

Author Owner

@westonruter The http strip out is already done at L77-82.

I will update with your regex. It makes more sense to check if it doesn't start with http(s) then to check if it start with a single /
if ($matches){
$action_url = '//' . $_SERVER['HTTP_HOST'] . $action_url;
}
}
$xhr_action = $node->getAttribute( 'action-xhr' );

Expand Down

0 comments on commit fae8ab9

Please sign in to comment.