From 2c11b6a5feddcf3cb08f594a489e6b19f3ba3391 Mon Sep 17 00:00:00 2001 From: Corey McKrill Date: Tue, 27 Sep 2016 15:16:19 -0700 Subject: [PATCH 1/9] Sanitize audio/video: update source attributes Both the audio and video sanitizers use the same routine to iterate through the source elements within the main node and filter/sanitize their attributes. However, the filtered attributes are not added back to the source element before it is appended to the new amp-audio or amp-video element. This is problematic when the attribute filtering method converts URLs from http to https, but the converted URL does not make it back into the source element. This commit addresses the issue by updating each source element's attributes before it is appended. --- includes/sanitizers/class-amp-audio-sanitizer.php | 1 + includes/sanitizers/class-amp-video-sanitizer.php | 1 + 2 files changed, 2 insertions(+) diff --git a/includes/sanitizers/class-amp-audio-sanitizer.php b/includes/sanitizers/class-amp-audio-sanitizer.php index a90040a7cce..8a47969a2be 100644 --- a/includes/sanitizers/class-amp-audio-sanitizer.php +++ b/includes/sanitizers/class-amp-audio-sanitizer.php @@ -39,6 +39,7 @@ public function sanitize() { // Only append source tags with a valid src attribute if ( ! empty( $new_child_attributes['src'] ) && 'source' === $new_child_node->tagName ) { + AMP_DOM_Utils::add_attributes_to_node( $new_child_node, $new_child_attributes ); $new_node->appendChild( $new_child_node ); } } diff --git a/includes/sanitizers/class-amp-video-sanitizer.php b/includes/sanitizers/class-amp-video-sanitizer.php index b17f9e5836e..6d5a099743a 100644 --- a/includes/sanitizers/class-amp-video-sanitizer.php +++ b/includes/sanitizers/class-amp-video-sanitizer.php @@ -36,6 +36,7 @@ public function sanitize() { // Only append source tags with a valid src attribute if ( ! empty( $new_child_attributes['src'] ) && 'source' === $new_child_node->tagName ) { + AMP_DOM_Utils::add_attributes_to_node( $new_child_node, $new_child_attributes ); $new_node->appendChild( $new_child_node ); } } From 696d1d08c3aef5580de63afc46479b314eb22bc3 Mon Sep 17 00:00:00 2001 From: Corey McKrill Date: Tue, 27 Sep 2016 17:25:04 -0700 Subject: [PATCH 2/9] Fallbacks: Replace invalid audio/video element with fallback element If an audio or video element, or its source child element, has a src URL, but it is invalid (eg it uses the http protocol instead of https), this creates a fallback blockquote element to replace the original that explains that the media could not be loaded. --- .../sanitizers/class-amp-audio-sanitizer.php | 42 +++++++++++++++++-- .../sanitizers/class-amp-video-sanitizer.php | 42 +++++++++++++++++-- 2 files changed, 76 insertions(+), 8 deletions(-) diff --git a/includes/sanitizers/class-amp-audio-sanitizer.php b/includes/sanitizers/class-amp-audio-sanitizer.php index 8a47969a2be..e038206609f 100644 --- a/includes/sanitizers/class-amp-audio-sanitizer.php +++ b/includes/sanitizers/class-amp-audio-sanitizer.php @@ -8,6 +8,10 @@ class AMP_Audio_Sanitizer extends AMP_Base_Sanitizer { private static $script_slug = 'amp-audio'; private static $script_src = 'https://cdn.ampproject.org/v0/amp-audio-0.1.js'; + protected $DEFAULT_ARGS = array( + 'require_https_src' => true, + ); + public function get_scripts() { if ( ! $this->did_convert_elements ) { return array(); @@ -24,9 +28,15 @@ public function sanitize() { } for ( $i = $num_nodes - 1; $i >= 0; $i-- ) { + $orig_src = array(); + $node = $nodes->item( $i ); $old_attributes = AMP_DOM_Utils::get_node_attributes_as_assoc_array( $node ); + if ( isset( $old_attributes['src'] ) ) { + $orig_src[] = $old_attributes['src']; + } + $new_attributes = $this->filter_attributes( $old_attributes ); $new_node = AMP_DOM_Utils::create_node( $this->dom, 'amp-audio', $new_attributes ); @@ -35,6 +45,11 @@ public function sanitize() { foreach ( $node->childNodes as $child_node ) { $new_child_node = $child_node->cloneNode( true ); $old_child_attributes = AMP_DOM_Utils::get_node_attributes_as_assoc_array( $new_child_node ); + + if ( isset( $old_child_attributes['src'] ) ) { + $orig_src[] = $old_child_attributes['src']; + } + $new_child_attributes = $this->filter_attributes( $old_child_attributes ); // Only append source tags with a valid src attribute @@ -45,12 +60,31 @@ public function sanitize() { } // If the node has at least one valid source, replace the old node with it. + // If the node has no valid sources, but at least one invalid (http) one, add a fallback element. // Otherwise, just remove the node. - // - // TODO: Add a fallback handler. - // See: https://github.com/ampproject/amphtml/issues/2261 if ( 0 === $new_node->childNodes->length && empty( $new_attributes['src'] ) ) { - $node->parentNode->removeChild( $node ); + if ( ! empty( $orig_src ) ) { + $fallback_node = AMP_DOM_Utils::create_node( + $this->dom, + 'blockquote', + array( + 'class' => 'amp-wp-fallback amp-wp-audio-fallback' + ) + ); + + $fallback_content = $this->dom->createDocumentFragment(); + $fallback_content->appendXML( sprintf( + wp_kses( __( 'Could not load audio.', 'amp' ), array( 'a' => array( 'href' => true ) ) ), + esc_url( array_shift( $orig_src ) ) + ) + ); + + $fallback_node->appendChild( $fallback_content ); + + $node->parentNode->replaceChild( $fallback_node, $node ); + } else { + $node->parentNode->removeChild( $node ); + } } else { $node->parentNode->replaceChild( $new_node, $node ); } diff --git a/includes/sanitizers/class-amp-video-sanitizer.php b/includes/sanitizers/class-amp-video-sanitizer.php index 6d5a099743a..d6cf51da2ee 100644 --- a/includes/sanitizers/class-amp-video-sanitizer.php +++ b/includes/sanitizers/class-amp-video-sanitizer.php @@ -10,6 +10,10 @@ class AMP_Video_Sanitizer extends AMP_Base_Sanitizer { public static $tag = 'video'; + protected $DEFAULT_ARGS = array( + 'require_https_src' => true, + ); + public function sanitize() { $nodes = $this->dom->getElementsByTagName( self::$tag ); $num_nodes = $nodes->length; @@ -18,9 +22,15 @@ public function sanitize() { } for ( $i = $num_nodes - 1; $i >= 0; $i-- ) { + $orig_src = array(); + $node = $nodes->item( $i ); $old_attributes = AMP_DOM_Utils::get_node_attributes_as_assoc_array( $node ); + if ( isset( $old_attributes['src'] ) ) { + $orig_src[] = $old_attributes['src']; + } + $new_attributes = $this->filter_attributes( $old_attributes ); $new_attributes = $this->enforce_fixed_height( $new_attributes ); @@ -32,6 +42,11 @@ public function sanitize() { foreach ( $node->childNodes as $child_node ) { $new_child_node = $child_node->cloneNode( true ); $old_child_attributes = AMP_DOM_Utils::get_node_attributes_as_assoc_array( $new_child_node ); + + if ( isset( $old_child_attributes['src'] ) ) { + $orig_src[] = $old_child_attributes['src']; + } + $new_child_attributes = $this->filter_attributes( $old_child_attributes ); // Only append source tags with a valid src attribute @@ -42,12 +57,31 @@ public function sanitize() { } // If the node has at least one valid source, replace the old node with it. + // If the node has no valid sources, but at least one invalid (http) one, add a fallback element. // Otherwise, just remove the node. - // - // TODO: Add a fallback handler. - // See: https://github.com/ampproject/amphtml/issues/2261 if ( 0 === $new_node->childNodes->length && empty( $new_attributes['src'] ) ) { - $node->parentNode->removeChild( $node ); + if ( ! empty( $orig_src ) ) { + $fallback_node = AMP_DOM_Utils::create_node( + $this->dom, + 'blockquote', + array( + 'class' => 'amp-wp-fallback amp-wp-video-fallback' + ) + ); + + $fallback_content = $this->dom->createDocumentFragment(); + $fallback_content->appendXML( sprintf( + wp_kses( __( 'Could not load video.', 'amp' ), array( 'a' => array( 'href' => true ) ) ), + esc_url( array_shift( $orig_src ) ) + ) + ); + + $fallback_node->appendChild( $fallback_content ); + + $node->parentNode->replaceChild( $fallback_node, $node ); + } else { + $node->parentNode->removeChild( $node ); + } } else { $node->parentNode->replaceChild( $new_node, $node ); } From 3832b5936a6d2bbf4d627ef64512407104fffe9a Mon Sep 17 00:00:00 2001 From: Corey McKrill Date: Thu, 29 Sep 2016 15:42:16 -0700 Subject: [PATCH 3/9] Fallbacks: update audio/video tests to reflect https is now required --- tests/test-amp-audio-converter.php | 9 ++++++++- tests/test-amp-video-sanitizer.php | 11 +++++++++-- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/tests/test-amp-audio-converter.php b/tests/test-amp-audio-converter.php index 8552265660e..18188e950a0 100644 --- a/tests/test-amp-audio-converter.php +++ b/tests/test-amp-audio-converter.php @@ -70,10 +70,17 @@ public function get_data() { '' ), + /* 'https_not_required' => array( '', '', ), + */ + + 'https_required' => array( + '', + '
Could not load audio.
', + ), ); } @@ -90,7 +97,7 @@ public function test_converter( $source, $expected ) { public function test__https_required() { $source = ''; - $expected = ''; + $expected = '
Could not load audio.
'; $dom = AMP_DOM_Utils::get_dom_from_content( $source ); $sanitizer = new AMP_Audio_Sanitizer( $dom, array( diff --git a/tests/test-amp-video-sanitizer.php b/tests/test-amp-video-sanitizer.php index 481d2d64248..b9826c46cd0 100644 --- a/tests/test-amp-video-sanitizer.php +++ b/tests/test-amp-video-sanitizer.php @@ -71,10 +71,17 @@ public function get_data() { '' ), + /* 'https_not_required' => array( '', '', - ) + ), + */ + + 'https_required' => array( + '', + '
Could not load video.
', + ), ); } @@ -91,7 +98,7 @@ public function test_converter( $source, $expected ) { public function test__https_required() { $source = ''; - $expected = ''; + $expected = '
Could not load video.
'; $dom = AMP_DOM_Utils::get_dom_from_content( $source ); $sanitizer = new AMP_Video_Sanitizer( $dom, array( From 9e02d565db0b510264d34ea95754b0b08f242738 Mon Sep 17 00:00:00 2001 From: Corey McKrill Date: Thu, 29 Sep 2016 16:11:54 -0700 Subject: [PATCH 4/9] Fallbacks: Replace invalid iframe element with fallback element Creates a fallback blockquote element to replace an iframe that doesn't have a valid src attribute. --- .../sanitizers/class-amp-iframe-sanitizer.php | 40 +++++++++++++++---- 1 file changed, 32 insertions(+), 8 deletions(-) diff --git a/includes/sanitizers/class-amp-iframe-sanitizer.php b/includes/sanitizers/class-amp-iframe-sanitizer.php index 95de9680f65..a072781aac2 100644 --- a/includes/sanitizers/class-amp-iframe-sanitizer.php +++ b/includes/sanitizers/class-amp-iframe-sanitizer.php @@ -15,7 +15,8 @@ class AMP_Iframe_Sanitizer extends AMP_Base_Sanitizer { private static $script_src = 'https://cdn.ampproject.org/v0/amp-iframe-0.1.js'; protected $DEFAULT_ARGS = array( - 'add_placeholder' => false, + 'require_https_src' => true, + 'add_placeholder' => false, ); public function get_scripts() { @@ -37,16 +38,39 @@ public function sanitize() { $node = $nodes->item( $i ); $old_attributes = AMP_DOM_Utils::get_node_attributes_as_assoc_array( $node ); + $orig_src = ''; + if ( isset( $old_attributes['src'] ) ) { + $orig_src = $old_attributes['src']; + } + $new_attributes = $this->filter_attributes( $old_attributes ); - // If the src doesn't exist, remove the node. - // This means that it never existed or was invalidated - // while filtering attributes above. - // - // TODO: add a filter to allow for a fallback element in this instance. - // See: https://github.com/ampproject/amphtml/issues/2261 + // If the filtered src doesn't exist, but there's an invalid src, add a fallback element. + // Otherwise remove the node. if ( empty( $new_attributes['src'] ) ) { - $node->parentNode->removeChild( $node ); + if ( ! empty( $orig_src ) ) { + $fallback_node = AMP_DOM_Utils::create_node( + $this->dom, + 'blockquote', + array( + 'class' => 'amp-wp-fallback amp-wp-iframe-fallback' + ) + ); + + $fallback_content = $this->dom->createDocumentFragment(); + $fallback_content->appendXML( sprintf( + wp_kses( __( 'Could not load iframe.', 'amp' ), array( 'a' => array( 'href' => true ) ) ), + esc_url( $orig_src ) + ) + ); + + $fallback_node->appendChild( $fallback_content ); + + $node->parentNode->replaceChild( $fallback_node, $node ); + } else { + $node->parentNode->removeChild( $node ); + } + continue; } From e5a6253b5d148e5be6c70caad584f0c9ff802e9b Mon Sep 17 00:00:00 2001 From: Corey McKrill Date: Thu, 29 Sep 2016 16:30:46 -0700 Subject: [PATCH 5/9] Maybe set https scheme on URLs with relative protocols For a URL with a relative protocol, such as //www.youtube.com/watch?v=dQw4w9WgXcQ ...the assumption is that either http or https will work with it. Since AMP requires https, we should go ahead and add the https scheme to these during filtering, if the sanitizer is configured to require or force the protocol. --- includes/sanitizers/class-amp-base-sanitizer.php | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/includes/sanitizers/class-amp-base-sanitizer.php b/includes/sanitizers/class-amp-base-sanitizer.php index 3a4d78c079d..6d7bbb56731 100644 --- a/includes/sanitizers/class-amp-base-sanitizer.php +++ b/includes/sanitizers/class-amp-base-sanitizer.php @@ -102,14 +102,18 @@ public function add_or_append_attribute( &$attributes, $key, $value, $separator * @return string */ public function maybe_enforce_https_src( $src, $force_https = false ) { + $https_required = isset( $this->args['require_https_src'] ) && true === $this->args['require_https_src']; $protocol = strtok( $src, ':' ); - if ( 'https' !== $protocol ) { + + if ( $protocol === $src && ( $https_required || $force_https ) ) { + // src has relative protocol, ie //example.com/asdf, so add https. + $src = set_url_scheme( $src, 'https' ); + } else if ( 'https' !== $protocol ) { // Check if https is required - if ( isset( $this->args['require_https_src'] ) && true === $this->args['require_https_src'] ) { + if ( $https_required ) { // Remove the src. Let the implementing class decide what do from here. $src = ''; - } elseif ( ( ! isset( $this->args['require_https_src'] ) || false === $this->args['require_https_src'] ) - && true === $force_https ) { + } elseif ( ! $https_required && true === $force_https ) { // Don't remove the src, but force https instead $src = set_url_scheme( $src, 'https' ); } From a689a2c1b51de7723e6817f0de700bb800e7a9b7 Mon Sep 17 00:00:00 2001 From: Corey McKrill Date: Thu, 29 Sep 2016 16:36:23 -0700 Subject: [PATCH 6/9] Fallbacks: update iframe tests to reflect that https is now required --- tests/test-amp-iframe-sanitizer.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test-amp-iframe-sanitizer.php b/tests/test-amp-iframe-sanitizer.php index ae2e535a47e..e22a62997e8 100644 --- a/tests/test-amp-iframe-sanitizer.php +++ b/tests/test-amp-iframe-sanitizer.php @@ -15,7 +15,7 @@ public function get_data() { 'force_https' => array( '', - '', + '
Could not load iframe.
', ), 'iframe_without_dimensions' => array( @@ -116,7 +116,7 @@ public function test_converter( $source, $expected ) { public function test__https_required() { $source = ''; - $expected = ''; + $expected = '
Could not load iframe.
'; $dom = AMP_DOM_Utils::get_dom_from_content( $source ); $sanitizer = new AMP_Iframe_Sanitizer( $dom, array( From 2365364ccd658f2c07e778c53b038352211e845c Mon Sep 17 00:00:00 2001 From: Corey McKrill Date: Thu, 6 Oct 2016 13:26:35 -0700 Subject: [PATCH 7/9] Fallbacks: abstract fallback node creation to a base class method --- .../sanitizers/class-amp-audio-sanitizer.php | 14 ++----------- .../sanitizers/class-amp-base-sanitizer.php | 21 +++++++++++++++++++ .../sanitizers/class-amp-iframe-sanitizer.php | 14 ++----------- .../sanitizers/class-amp-video-sanitizer.php | 14 ++----------- 4 files changed, 27 insertions(+), 36 deletions(-) diff --git a/includes/sanitizers/class-amp-audio-sanitizer.php b/includes/sanitizers/class-amp-audio-sanitizer.php index e038206609f..e204c7e27cb 100644 --- a/includes/sanitizers/class-amp-audio-sanitizer.php +++ b/includes/sanitizers/class-amp-audio-sanitizer.php @@ -64,23 +64,13 @@ public function sanitize() { // Otherwise, just remove the node. if ( 0 === $new_node->childNodes->length && empty( $new_attributes['src'] ) ) { if ( ! empty( $orig_src ) ) { - $fallback_node = AMP_DOM_Utils::create_node( - $this->dom, - 'blockquote', - array( - 'class' => 'amp-wp-fallback amp-wp-audio-fallback' - ) - ); - - $fallback_content = $this->dom->createDocumentFragment(); - $fallback_content->appendXML( sprintf( + $fallback_node = $this->create_fallback_node( + sprintf( wp_kses( __( 'Could not load audio.', 'amp' ), array( 'a' => array( 'href' => true ) ) ), esc_url( array_shift( $orig_src ) ) ) ); - $fallback_node->appendChild( $fallback_content ); - $node->parentNode->replaceChild( $fallback_node, $node ); } else { $node->parentNode->removeChild( $node ); diff --git a/includes/sanitizers/class-amp-base-sanitizer.php b/includes/sanitizers/class-amp-base-sanitizer.php index 6d7bbb56731..481e9353675 100644 --- a/includes/sanitizers/class-amp-base-sanitizer.php +++ b/includes/sanitizers/class-amp-base-sanitizer.php @@ -121,4 +121,25 @@ public function maybe_enforce_https_src( $src, $force_https = false ) { return $src; } + + protected function create_fallback_node( $content, $container_el = 'blockquote', $attributes = array() ) { + $defaults = array( + 'class' => 'amp-wp-fallback', + ); + $attributes = wp_parse_args( $attributes, $defaults ); + + $fallback_node = AMP_DOM_Utils::create_node( + $this->dom, + $container_el, + $attributes + ); + + if ( $content ) { + $fallback_content = $this->dom->createDocumentFragment(); + $fallback_content->appendXML( $content ); + $fallback_node->appendChild( $fallback_content ); + } + + return $fallback_node; + } } diff --git a/includes/sanitizers/class-amp-iframe-sanitizer.php b/includes/sanitizers/class-amp-iframe-sanitizer.php index a072781aac2..0d8edcbc5bd 100644 --- a/includes/sanitizers/class-amp-iframe-sanitizer.php +++ b/includes/sanitizers/class-amp-iframe-sanitizer.php @@ -49,23 +49,13 @@ public function sanitize() { // Otherwise remove the node. if ( empty( $new_attributes['src'] ) ) { if ( ! empty( $orig_src ) ) { - $fallback_node = AMP_DOM_Utils::create_node( - $this->dom, - 'blockquote', - array( - 'class' => 'amp-wp-fallback amp-wp-iframe-fallback' - ) - ); - - $fallback_content = $this->dom->createDocumentFragment(); - $fallback_content->appendXML( sprintf( + $fallback_node = $this->create_fallback_node( + sprintf( wp_kses( __( 'Could not load iframe.', 'amp' ), array( 'a' => array( 'href' => true ) ) ), esc_url( $orig_src ) ) ); - $fallback_node->appendChild( $fallback_content ); - $node->parentNode->replaceChild( $fallback_node, $node ); } else { $node->parentNode->removeChild( $node ); diff --git a/includes/sanitizers/class-amp-video-sanitizer.php b/includes/sanitizers/class-amp-video-sanitizer.php index d6cf51da2ee..cd4b33d9ed9 100644 --- a/includes/sanitizers/class-amp-video-sanitizer.php +++ b/includes/sanitizers/class-amp-video-sanitizer.php @@ -61,23 +61,13 @@ public function sanitize() { // Otherwise, just remove the node. if ( 0 === $new_node->childNodes->length && empty( $new_attributes['src'] ) ) { if ( ! empty( $orig_src ) ) { - $fallback_node = AMP_DOM_Utils::create_node( - $this->dom, - 'blockquote', - array( - 'class' => 'amp-wp-fallback amp-wp-video-fallback' - ) - ); - - $fallback_content = $this->dom->createDocumentFragment(); - $fallback_content->appendXML( sprintf( + $fallback_node = $this->create_fallback_node( + sprintf( wp_kses( __( 'Could not load video.', 'amp' ), array( 'a' => array( 'href' => true ) ) ), esc_url( array_shift( $orig_src ) ) ) ); - $fallback_node->appendChild( $fallback_content ); - $node->parentNode->replaceChild( $fallback_node, $node ); } else { $node->parentNode->removeChild( $node ); From b1077d6c8e8331e019002b4e2f114d0c60ba7979 Mon Sep 17 00:00:00 2001 From: Corey McKrill Date: Thu, 6 Oct 2016 13:30:50 -0700 Subject: [PATCH 8/9] Fallbacks: update tests In the previous commit I removed the embed-specific class name from the abstracted `create_fallback_node` method, but forgot to update the tests --- tests/test-amp-audio-converter.php | 4 ++-- tests/test-amp-iframe-sanitizer.php | 4 ++-- tests/test-amp-video-sanitizer.php | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/test-amp-audio-converter.php b/tests/test-amp-audio-converter.php index 18188e950a0..6fe8f964e21 100644 --- a/tests/test-amp-audio-converter.php +++ b/tests/test-amp-audio-converter.php @@ -79,7 +79,7 @@ public function get_data() { 'https_required' => array( '', - '
Could not load audio.
', + '
Could not load audio.
', ), ); } @@ -97,7 +97,7 @@ public function test_converter( $source, $expected ) { public function test__https_required() { $source = ''; - $expected = '
Could not load audio.
'; + $expected = '
Could not load audio.
'; $dom = AMP_DOM_Utils::get_dom_from_content( $source ); $sanitizer = new AMP_Audio_Sanitizer( $dom, array( diff --git a/tests/test-amp-iframe-sanitizer.php b/tests/test-amp-iframe-sanitizer.php index e22a62997e8..2f2cd94c104 100644 --- a/tests/test-amp-iframe-sanitizer.php +++ b/tests/test-amp-iframe-sanitizer.php @@ -15,7 +15,7 @@ public function get_data() { 'force_https' => array( '', - '
Could not load iframe.
', + '
Could not load iframe.
', ), 'iframe_without_dimensions' => array( @@ -116,7 +116,7 @@ public function test_converter( $source, $expected ) { public function test__https_required() { $source = ''; - $expected = '
Could not load iframe.
'; + $expected = '
Could not load iframe.
'; $dom = AMP_DOM_Utils::get_dom_from_content( $source ); $sanitizer = new AMP_Iframe_Sanitizer( $dom, array( diff --git a/tests/test-amp-video-sanitizer.php b/tests/test-amp-video-sanitizer.php index b9826c46cd0..cd31b84d1c9 100644 --- a/tests/test-amp-video-sanitizer.php +++ b/tests/test-amp-video-sanitizer.php @@ -80,7 +80,7 @@ public function get_data() { 'https_required' => array( '', - '
Could not load video.
', + '
Could not load video.
', ), ); } @@ -98,7 +98,7 @@ public function test_converter( $source, $expected ) { public function test__https_required() { $source = ''; - $expected = '
Could not load video.
'; + $expected = '
Could not load video.
'; $dom = AMP_DOM_Utils::get_dom_from_content( $source ); $sanitizer = new AMP_Video_Sanitizer( $dom, array( From f7dafe58ecf91d8c6c296835390c88727dce3351 Mon Sep 17 00:00:00 2001 From: Corey McKrill Date: Thu, 6 Oct 2016 14:31:27 -0700 Subject: [PATCH 9/9] Detect and handle protocol- and path-relative URLs in base sanitizer --- .../sanitizers/class-amp-base-sanitizer.php | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/includes/sanitizers/class-amp-base-sanitizer.php b/includes/sanitizers/class-amp-base-sanitizer.php index 481e9353675..4ada9b64156 100644 --- a/includes/sanitizers/class-amp-base-sanitizer.php +++ b/includes/sanitizers/class-amp-base-sanitizer.php @@ -106,9 +106,22 @@ public function maybe_enforce_https_src( $src, $force_https = false ) { $protocol = strtok( $src, ':' ); if ( $protocol === $src && ( $https_required || $force_https ) ) { - // src has relative protocol, ie //example.com/asdf, so add https. - $src = set_url_scheme( $src, 'https' ); - } else if ( 'https' !== $protocol ) { + if ( 0 === strpos( $src, '//' ) ) { + // src has relative protocol, ie //example.com/asdf, so add https + $src = set_url_scheme( $src, 'https' ); + } else if ( 0 === strpos( $src, '/' ) ) { + // src is URL relative to the site root + $src = home_url( $src ); + } else { + // src is URL relative to current URI + global $wp; + $src = home_url( trailingslashit( $wp->request ) . $src ); + } + + $protocol = strtok( $src, ':' ); + } + + if ( 'https' !== $protocol ) { // Check if https is required if ( $https_required ) { // Remove the src. Let the implementing class decide what do from here.