Merge pull request #1274 from Automattic/fix/convert-video-src-to-https

Convert video src to HTTPS
ampproject · Jul 23, 2018 · e3d036c · e3d036c
2 parents 024e450 + 06e23e0
commit e3d036c
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 12 deletions.
diff --git a/includes/sanitizers/class-amp-video-sanitizer.php b/includes/sanitizers/class-amp-video-sanitizer.php
@@ -47,6 +47,7 @@ public function get_selector_conversion_mapping() {
 	 * Sanitize the <video> elements from the HTML contained in this instance's DOMDocument.
 	 *
 	 * @since 0.2
+	 * @since 1.0 Set the filtered child node's src attribute.
 	 */
 	public function sanitize() {
 		$nodes     = $this->dom->getElementsByTagName( self::$tag );
@@ -95,11 +96,12 @@ public function sanitize() {
 					continue;
 				}
 
+				$this->update_src( $new_child_node, $new_child_attributes['src'], $old_child_attributes['src'] );
+
 				/**
 				 * Only append source tags with a valid src attribute
 				 */
 				$new_node->appendChild( $new_child_node );
-
 			}
 
 			/*
@@ -124,6 +126,7 @@ public function sanitize() {
 	 * Filter video dimensions, try to get width and height from original file if missing.
 	 *
 	 * @param array $new_attributes Attributes.
+	 *
 	 * @return array Modified attributes.
 	 */
 	protected function filter_video_dimensions( $new_attributes ) {
@@ -151,26 +154,28 @@ protected function filter_video_dimensions( $new_attributes ) {
 				}
 			}
 		}
+
 		return $new_attributes;
 	}
 
 	/**
 	 * "Filter" HTML attributes for <amp-audio> elements.
 	 *
 	 * @since 0.2
+	 * @since 1.0 Force HTTPS for the src attribute.
 	 *
 	 * @param string[] $attributes {
 	 *      Attributes.
 	 *
-	 *      @type string $src Video URL - Empty if HTTPS required per $this->args['require_https_src']
-	 *      @type int $width <video> attribute - Set to numeric value if px or %
-	 *      @type int $height <video> attribute - Set to numeric value if px or %
-	 *      @type string $poster <video> attribute - Pass along if found
-	 *      @type string $class <video> attribute - Pass along if found
-	 *      @type bool $controls <video> attribute - Convert 'false' to empty string ''
-	 *      @type bool $loop <video> attribute - Convert 'false' to empty string ''
-	 *      @type bool $muted <video> attribute - Convert 'false' to empty string ''
-	 *      @type bool $autoplay <video> attribute - Convert 'false' to empty string ''
+	 *      @type string    $src        Video URL - Empty if HTTPS required per $this->args['require_https_src']
+	 *      @type int       $width      <video> attribute - Set to numeric value if px or %
+	 *      @type int       $height     <video> attribute - Set to numeric value if px or %
+	 *      @type string    $poster     <video> attribute - Pass along if found
+	 *      @type string    $class      <video> attribute - Pass along if found
+	 *      @type bool      $controls   <video> attribute - Convert 'false' to empty string ''
+	 *      @type bool      $loop       <video> attribute - Convert 'false' to empty string ''
+	 *      @type bool      $muted      <video> attribute - Convert 'false' to empty string ''
+	 *      @type bool      $autoplay   <video> attribute - Convert 'false' to empty string ''
 	 * }
 	 * @return array Returns HTML attributes; removes any not specifically declared above from input.
 	 */
@@ -180,7 +185,7 @@ private function filter_attributes( $attributes ) {
 		foreach ( $attributes as $name => $value ) {
 			switch ( $name ) {
 				case 'src':
-					$out[ $name ] = $this->maybe_enforce_https_src( $value );
+					$out[ $name ] = $this->maybe_enforce_https_src( $value, true );
 					break;
 
 				case 'width':
@@ -218,4 +223,18 @@ private function filter_attributes( $attributes ) {
 
 		return $out;
 	}
+
+	/**
+	 * Update the node's src attribute if it is different from the old src attribute.
+	 *
+	 * @param DOMNode $node    The given DOMNode.
+	 * @param string  $new_src The new src attribute.
+	 * @param string  $old_src The old src attribute.
+	 */
+	protected function update_src( &$node, $new_src, $old_src ) {
+		if ( $old_src === $new_src ) {
+			return;
+		}
+		$node->setAttribute( 'src', $new_src );
+	}
 }
diff --git a/tests/test-amp-video-sanitizer.php b/tests/test-amp-video-sanitizer.php
@@ -90,7 +90,15 @@ public function get_data() {
 
 			'https_not_required' => array(
 				'<video width="300" height="300" src="http://example.com/video.mp4"></video>',
-				'<amp-video width="300" height="300" src="http://example.com/video.mp4" layout="responsive"></amp-video>',
+				'<amp-video width="300" height="300" src="https://example.com/video.mp4" layout="responsive"></amp-video>',
+			),
+
+			'http_video_with_children' => array(
+				'<video width="480" height="300" poster="http://example.com/video-image.gif">
+	<source src="http://example.com/video.mp4" type="video/mp4">
+	<source src="http://example.com/video.ogv" type="video/ogg">
+</video>',
+				'<amp-video width="480" height="300" poster="http://example.com/video-image.gif" layout="responsive"><source src="https://example.com/video.mp4" type="video/mp4"><source src="https://example.com/video.ogv" type="video/ogg"></amp-video>',
 			),
 		);
 	}