From 7ff58f17877cd5b5ce6e96def47aa7719e843977 Mon Sep 17 00:00:00 2001
From: Martin Brennan <mjrbrennan@gmail.com>
Date: Tue, 11 Feb 2020 11:49:58 +1000
Subject: [PATCH] FIX: Disable preloading audio + video when secure media
 enabled (#8922)

Meta topic: https://meta.discourse.org/t/secure-media-uploads-expire/140894

This fixes the issue where if secure media was enabled, audio
and video files would do an initial load using the presigned
URL for the media to get metadata information e.g. duration of
track/video. However this started the expiry countdown for the
URL, so when a user pressed play on the media after 15 seconds
the media would be expired and AWS would return a 403 error.

We do not preload media if secure media is enabled. Otherwise
we just set the preload type to "metadata" which is the browser
default anyway.
---
 .../engines/discourse-markdown-it.js.es6      | 22 +++++++++-----
 .../pretty-text/white-lister.js.es6           |  4 ++-
 test/javascripts/lib/pretty-text-test.js.es6  | 30 +++++++++++++++++--
 3 files changed, 46 insertions(+), 10 deletions(-)

diff --git a/app/assets/javascripts/pretty-text/engines/discourse-markdown-it.js.es6 b/app/assets/javascripts/pretty-text/engines/discourse-markdown-it.js.es6
index 4439455f12c..6e6392d7745 100644
--- a/app/assets/javascripts/pretty-text/engines/discourse-markdown-it.js.es6
+++ b/app/assets/javascripts/pretty-text/engines/discourse-markdown-it.js.es6
@@ -141,21 +141,23 @@ export function extractDataAttribute(str) {
 
 // videoHTML and audioHTML follow the same HTML syntax
 // as oneboxer.rb when dealing with these formats
-function videoHTML(token) {
+function videoHTML(token, opts) {
   const src = token.attrGet("src");
   const origSrc = token.attrGet("data-orig-src");
+  const preloadType = opts.secureMedia ? "none" : "metadata";
   return `<div class="video-container">
-    <video width="100%" height="100%" controls>
+    <video width="100%" height="100%" preload="${preloadType}" controls>
       <source src="${src}" data-orig-src="${origSrc}">
       <a href="${src}">${src}</a>
     </video>
   </div>`;
 }
 
-function audioHTML(token) {
+function audioHTML(token, opts) {
   const src = token.attrGet("src");
   const origSrc = token.attrGet("data-orig-src");
-  return `<audio controls>
+  const preloadType = opts.secureMedia ? "none" : "metadata";
+  return `<audio preload="${preloadType}" controls>
     <source src="${src}" data-orig-src="${origSrc}">
     <a href="${src}">${src}</a>
   </audio>`;
@@ -165,17 +167,19 @@ const IMG_SIZE_REGEX = /^([1-9]+[0-9]*)x([1-9]+[0-9]*)(\s*,\s*(x?)([1-9][0-9]{0,
 function renderImageOrPlayableMedia(tokens, idx, options, env, slf) {
   const token = tokens[idx];
   const alt = slf.renderInlineAsText(token.children, options, env);
-
   const split = alt.split("|");
   const altSplit = [];
 
   // markdown-it supports returning HTML instead of continuing to render the current token
   // see https://github.com/markdown-it/markdown-it/blob/master/docs/architecture.md#renderer
   // handles |video and |audio alt transformations for image tags
+  const mediaOpts = {
+    secureMedia: options.discourse.limitedSiteSettings.secureMedia
+  };
   if (split[1] === "video") {
-    return videoHTML(token);
+    return videoHTML(token, mediaOpts);
   } else if (split[1] === "audio") {
-    return audioHTML(token);
+    return audioHTML(token, mediaOpts);
   }
 
   // parsing ![myimage|500x300]() or ![myimage|75%]() or ![myimage|500x300, 75%]
@@ -334,6 +338,10 @@ export function setup(opts, siteSettings, state) {
   opts.discourse = copy;
   getOptions.f = () => opts.discourse;
 
+  opts.discourse.limitedSiteSettings = {
+    secureMedia: siteSettings.secure_media
+  };
+
   opts.engine = window.markdownit({
     discourse: opts.discourse,
     html: true,
diff --git a/app/assets/javascripts/pretty-text/white-lister.js.es6 b/app/assets/javascripts/pretty-text/white-lister.js.es6
index c90a3ac6eee..8a3a1140f1e 100644
--- a/app/assets/javascripts/pretty-text/white-lister.js.es6
+++ b/app/assets/javascripts/pretty-text/white-lister.js.es6
@@ -134,6 +134,7 @@ export const DEFAULT_LIST = [
   "aside[data-*]",
   "audio",
   "audio[controls]",
+  "audio[preload]",
   "b",
   "big",
   "blockquote",
@@ -211,8 +212,9 @@ export const DEFAULT_LIST = [
   "video[crossorigin]",
   "video[height]",
   "video[poster]",
-  "video[preload]",
   "video[width]",
+  "video[controls]",
+  "video[preload]",
   "ruby",
   "ruby[lang]",
   "rb",
diff --git a/test/javascripts/lib/pretty-text-test.js.es6 b/test/javascripts/lib/pretty-text-test.js.es6
index d796eb6eddf..bab2d71d3a3 100644
--- a/test/javascripts/lib/pretty-text-test.js.es6
+++ b/test/javascripts/lib/pretty-text-test.js.es6
@@ -973,11 +973,37 @@ QUnit.test("images", assert => {
   );
 });
 
+QUnit.test("video - secure media enabled", assert => {
+  assert.cookedOptions(
+    "![baby shark|video](upload://eyPnj7UzkU0AkGkx2dx8G4YM1Jx.mp4)",
+    { siteSettings: { secure_media: true } },
+    `<p><div class="video-container">
+    <video width="100%" height="100%" preload="none" controls>
+      <source src="/404" data-orig-src="upload://eyPnj7UzkU0AkGkx2dx8G4YM1Jx.mp4">
+      <a href="/404">/404</a>
+    </video>
+  </div></p>`,
+    "It returns the correct video player HTML"
+  );
+});
+
+QUnit.test("audio - secure media enabled", assert => {
+  assert.cookedOptions(
+    "![young americans|audio](upload://eyPnj7UzkU0AkGkx2dx8G4YM1Jx.mp3)",
+    { siteSettings: { secure_media: true } },
+    `<p><audio preload="none" controls>
+    <source src="/404" data-orig-src="upload://eyPnj7UzkU0AkGkx2dx8G4YM1Jx.mp3">
+    <a href="/404">/404</a>
+  </audio></p>`,
+    "It returns the correct audio player HTML"
+  );
+});
+
 QUnit.test("video", assert => {
   assert.cooked(
     "![baby shark|video](upload://eyPnj7UzkU0AkGkx2dx8G4YM1Jx.mp4)",
     `<p><div class="video-container">
-    <video width="100%" height="100%" controls>
+    <video width="100%" height="100%" preload="metadata" controls>
       <source src="/404" data-orig-src="upload://eyPnj7UzkU0AkGkx2dx8G4YM1Jx.mp4">
       <a href="/404">/404</a>
     </video>
@@ -989,7 +1015,7 @@ QUnit.test("video", assert => {
 QUnit.test("audio", assert => {
   assert.cooked(
     "![young americans|audio](upload://eyPnj7UzkU0AkGkx2dx8G4YM1Jx.mp3)",
-    `<p><audio controls>
+    `<p><audio preload="metadata" controls>
     <source src="/404" data-orig-src="upload://eyPnj7UzkU0AkGkx2dx8G4YM1Jx.mp3">
     <a href="/404">/404</a>
   </audio></p>`,