Make media pool assert HTTPs-ness of URLs to align with amp-video (#2…

…7875) Fixes #27874
ampproject · Apr 19, 2020 · dd9b5b2 · dd9b5b2
1 parent b86998f
commit dd9b5b2
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 7 deletions.
diff --git a/extensions/amp-story/1.0/sources.js b/extensions/amp-story/1.0/sources.js
@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+import {Services} from '../../../src/services';
 import {ampMediaElementFor} from './utils';
 import {removeElement} from '../../../src/dom';
 import {toArray} from '../../../src/types';
@@ -132,6 +133,10 @@ export class Sources {
     // was specified.
     // cf: https://html.spec.whatwg.org/#concept-media-load-algorithm
     const sourcesToUse = srcEl ? [srcEl] : srcEls;
+    const urlService = Services.urlForDoc(win.document.documentElement);
+    sourcesToUse.forEach((el) =>
+      urlService.assertHttpsUrl(el.getAttribute('src'), el)
+    );
 
     return new Sources(null /** srcAttr */, sourcesToUse, trackEls);
   }

diff --git a/extensions/amp-story/1.0/test/test-media-pool.js b/extensions/amp-story/1.0/test/test-media-pool.js
@@ -19,7 +19,7 @@ import {findIndex} from '../../../../src/utils/array';
 
 const NOOP = () => {};
 
-describes.realWin('media-pool', {}, (env) => {
+describes.realWin('media-pool', {amp: true}, (env) => {
   let win;
   let mediaPool;
   let distanceFnStub;
@@ -42,11 +42,12 @@ describes.realWin('media-pool', {}, (env) => {
 
   /**
    * @param {string} tagName
+   * @param {string=} opt_url
    * @return {!HTMLMediaElement}
    */
-  function createMediaElement(tagName) {
+  function createMediaElement(tagName, opt_url) {
     const el = env.win.document.createElement(tagName);
-    el.src = `http://example.com/${tagName}.xyz`;
+    el.src = opt_url || `https://example.com/${tagName}.xyz`;
     env.win.document.body.appendChild(el);
     return el;
   }
@@ -136,6 +137,15 @@ describes.realWin('media-pool', {}, (env) => {
     expect(mediaPool.unallocated['video'].length).to.equal(1);
   });
 
+  it('should fail with http src', () => {
+    mediaPool = new MediaPool(win, {'video': 2}, (unusedEl) => 0);
+
+    const videoEl = createMediaElement('video', 'http://example.com/video.mp4');
+    expect(() => {
+      mediaPool.register(videoEl);
+    }).to.throw(/from either https/);
+  });
+
   it.skip('should evict the element with the highest distance first', () => {
     const elements = createMediaElements('video', 3);
     mediaPool = new MediaPool(

diff --git a/extensions/amp-story/1.0/test/test-media-tasks.js b/extensions/amp-story/1.0/test/test-media-tasks.js
@@ -27,14 +27,15 @@ import {
 import {Sources} from '../sources';
 import {toArray} from '../../../../src/types';
 
-describes.realWin('media-tasks', {}, (env) => {
+describes.realWin('media-tasks', {amp: true}, (env) => {
   let win;
   let el;
   let vsyncApi;
 
   beforeEach(() => {
     win = env.win;
-    el = document.createElement('video');
+    el = win.document.createElement('video');
+    win.document.body.appendChild(el);
 
     // Mock vsync
     vsyncApi = {
@@ -132,15 +133,15 @@ describes.realWin('media-tasks', {}, (env) => {
      * @return {string}
      */
     function getFakeVideoUrl(index) {
-      return `http://example.com/video${index}.mp4`;
+      return `https://example.com/video${index}.mp4`;
     }
 
     /**
      * @param {number} index
      * @return {!Element}
      */
     function getFakeSource(index) {
-      const source = document.createElement('source');
+      const source = env.win.document.createElement('source');
       source.src = getFakeVideoUrl(index);
       return source;
     }