FIX: Stop infinite lookup-urls issue for video/audio on page (#9096)

Meta report: https://meta.discourse.org/t/excessive-requests-to-uploads-lookup-urls-leading-to-429-response/143119

* The data-orig-src attribute was not being removed from cooked
video and audio so the composer was infinitely trying to get the
URLs for them, which would never resolve to anything
* Also the code that retrieved the short URL was unscoped, and was
getting everything on the page. if running from the composer we
now scope to the preview window
* Also fixed a minor issue where the element href for the video
and audio tags was not being set when the short URL was found

app/assets/javascripts/discourse/components/composer-editor.js.es6

@@ -1014,7 +1014,7 @@ export default Component.extend({
       );
 
       // Short upload urls need resolution
-      resolveAllShortUrls(ajax);
+      resolveAllShortUrls(ajax, ".d-editor-preview-wrapper");
 
       if (this._enableAdvancedEditorPreviewSync()) {
         this._syncScroll(

app/assets/javascripts/pretty-text/engines/discourse-markdown-it.js.es6

@@ -145,9 +145,10 @@ function videoHTML(token, opts) {
   const src = token.attrGet("src");
   const origSrc = token.attrGet("data-orig-src");
   const preloadType = opts.secureMedia ? "none" : "metadata";
+  const dataOrigSrcAttr = origSrc !== null ? `data-orig-src="${origSrc}"` : "";
   return `<div class="video-container">
     <video width="100%" height="100%" preload="${preloadType}" controls>
-      <source src="${src}" data-orig-src="${origSrc}">
+      <source src="${src}" ${dataOrigSrcAttr}>
       <a href="${src}">${src}</a>
     </video>
   </div>`;
@@ -157,8 +158,9 @@ function audioHTML(token, opts) {
   const src = token.attrGet("src");
   const origSrc = token.attrGet("data-orig-src");
   const preloadType = opts.secureMedia ? "none" : "metadata";
+  const dataOrigSrcAttr = origSrc !== null ? `data-orig-src="${origSrc}"` : "";
   return `<audio preload="${preloadType}" controls>
-    <source src="${src}" data-orig-src="${origSrc}">
+    <source src="${src}" ${dataOrigSrcAttr}>
     <a href="${src}">${src}</a>
   </audio>`;
 }

app/assets/javascripts/pretty-text/upload-short-url.js.es6

@@ -9,6 +9,11 @@ export function lookupCachedUploadUrl(shortUrl) {
 const MISSING = "missing";
 
 export function lookupUncachedUploadUrls(urls, ajax) {
+  urls = _.compact(urls);
+  if (urls.length === 0) {
+    return;
+  }
+
   return ajax("/uploads/lookup-urls", {
     method: "POST",
     data: { short_urls: urls }
@@ -77,7 +82,7 @@ function _loadCachedShortUrls($uploads) {
         });
 
         break;
-      case "SOURCE": // video tag > source tag
+      case "SOURCE": // video/audio tag > source tag
         retrieveCachedUrl($upload, "orig-src", url => {
           $upload.attr("src", url);
 
@@ -85,11 +90,19 @@ function _loadCachedShortUrls($uploads) {
             let hostRegex = new RegExp("//" + window.location.host, "g");
             url = url.replace(hostRegex, "");
           }
-          $upload.attr("src", window.location.origin + url);
+          let fullUrl = window.location.origin + url;
+          $upload.attr("src", fullUrl);
 
           // this is necessary, otherwise because of the src change the
-          // video just doesn't bother loading!
+          // video/audio just doesn't bother loading!
-          $upload.parent()[0].load();
+          let $parent = $upload.parent();
+          $parent[0].load();
+
+          // set the url and text for the <a> tag within the <video/audio> tag
+          $parent
+            .find("a")
+            .attr("href", fullUrl)
+            .text(fullUrl);
         });
 
         break;
@@ -98,7 +111,7 @@ function _loadCachedShortUrls($uploads) {
 }
 
 function _loadShortUrls($uploads, ajax) {
-  const urls = $uploads.toArray().map(upload => {
+  let urls = $uploads.toArray().map(upload => {
     const $upload = $(upload);
     return $upload.data("orig-src") || $upload.data("orig-href");
   });
@@ -108,15 +121,15 @@ function _loadShortUrls($uploads, ajax) {
   );
 }
 
-export function resolveAllShortUrls(ajax) {
+export function resolveAllShortUrls(ajax, scope = null) {
   const attributes =
     "img[data-orig-src], a[data-orig-href], source[data-orig-src]";
-  let $shortUploadUrls = $(attributes);
+  let $shortUploadUrls = $(scope || document).find(attributes);
 
   if ($shortUploadUrls.length > 0) {
     _loadCachedShortUrls($shortUploadUrls);
 
-    $shortUploadUrls = $(attributes);
+    $shortUploadUrls = $(scope || document).find(attributes);
     if ($shortUploadUrls.length > 0) {
       // this is carefully batched so we can do a leading debounce (trigger right away)
       return debounce(

test/javascripts/lib/pretty-text-test.js.es6

@@ -1012,6 +1012,32 @@ QUnit.test("video", assert => {
   );
 });
 
+QUnit.test("video - mapped url - secure media enabled", assert => {
+  function lookupUploadUrls() {
+    let cache = {};
+    cache["upload://eyPnj7UzkU0AkGkx2dx8G4YM1Jx.mp4"] = {
+      short_url: "upload://eyPnj7UzkU0AkGkx2dx8G4YM1Jx.mp4",
+      url: "/secure-media-uploads/original/3X/c/b/test.mp4",
+      short_path: "/uploads/short-url/blah"
+    };
+    return cache;
+  }
+  assert.cookedOptions(
+    "![baby shark|video](upload://eyPnj7UzkU0AkGkx2dx8G4YM1Jx.mp4)",
+    {
+      siteSettings: { secure_media: true },
+      lookupUploadUrls: lookupUploadUrls
+    },
+    `<p><div class="video-container">
+    <video width="100%" height="100%" preload="none" controls>
+      <source src="/secure-media-uploads/original/3X/c/b/test.mp4">
+      <a href="/secure-media-uploads/original/3X/c/b/test.mp4">/secure-media-uploads/original/3X/c/b/test.mp4</a>
+    </video>
+  </div></p>`,
+    "It returns the correct video HTML when the URL is mapped with secure media, removing data-orig-src"
+  );
+});
+
 QUnit.test("audio", assert => {
   assert.cooked(
     "![young americans|audio](upload://eyPnj7UzkU0AkGkx2dx8G4YM1Jx.mp3)",
@@ -1023,6 +1049,30 @@ QUnit.test("audio", assert => {
   );
 });
 
+QUnit.test("audio - mapped url - secure media enabled", assert => {
+  function lookupUploadUrls() {
+    let cache = {};
+    cache["upload://eyPnj7UzkU0AkGkx2dx8G4YM1Jx.mp3"] = {
+      short_url: "upload://eyPnj7UzkU0AkGkx2dx8G4YM1Jx.mp3",
+      url: "/secure-media-uploads/original/3X/c/b/test.mp3",
+      short_path: "/uploads/short-url/blah"
+    };
+    return cache;
+  }
+  assert.cookedOptions(
+    "![baby shark|audio](upload://eyPnj7UzkU0AkGkx2dx8G4YM1Jx.mp3)",
+    {
+      siteSettings: { secure_media: true },
+      lookupUploadUrls: lookupUploadUrls
+    },
+    `<p><audio preload="none" controls>
+    <source src="/secure-media-uploads/original/3X/c/b/test.mp3">
+    <a href="/secure-media-uploads/original/3X/c/b/test.mp3">/secure-media-uploads/original/3X/c/b/test.mp3</a>
+  </audio></p>`,
+    "It returns the correct audio HTML when the URL is mapped with secure media, removing data-orig-src"
+  );
+});
+
 QUnit.test("censoring", assert => {
   assert.cookedOptions(
     "Pleased to meet you, but pleeeease call me later, xyz123",

test/javascripts/lib/upload-short-url-test.js.es6

@@ -23,6 +23,11 @@ QUnit.module("lib:pretty-text/upload-short-url", {
         short_url: "upload://b.jpeg",
         url: "/uploads/default/original/3X/c/b/2.jpeg",
         short_path: "/uploads/short-url/b.jpeg"
+      },
+      {
+        short_url: "upload://z.jpeg",
+        url: "/uploads/default/original/3X/c/b/9.jpeg",
+        short_path: "/uploads/short-url/z.jpeg"
       }
     ];
 
@@ -53,7 +58,8 @@ QUnit.module("lib:pretty-text/upload-short-url", {
 
     fixture().html(
       imageSrcs.map(src => `<img data-orig-src="${src.url}">`).join("") +
-        attachmentSrcs.map(src => `<a data-orig-href="${src.url}">`).join("")
+        attachmentSrcs.map(src => `<a data-orig-href="${src.url}">`).join("") +
+        `<div class="scoped-area"><img data-orig-src="${imageSrcs[2].url}"></div>`
     );
   },
 
@@ -105,3 +111,23 @@ QUnit.test("resolveAllShortUrls", async assert => {
     short_path: "/uploads/short-url/e.mp3"
   });
 });
+
+QUnit.test("resolveAllShortUrls - scoped", async assert => {
+  let lookup;
+  await resolveAllShortUrls(ajax, ".scoped-area");
+
+  lookup = lookupCachedUploadUrl("upload://z.jpeg");
+
+  assert.deepEqual(lookup, {
+    url: "/uploads/default/original/3X/c/b/9.jpeg",
+    short_path: "/uploads/short-url/z.jpeg"
+  });
+
+  // do this because the pretender caches ALL the urls, not
+  // just the ones being looked up (like the normal behaviour)
+  resetCache();
+  await resolveAllShortUrls(ajax, ".scoped-area");
+
+  lookup = lookupCachedUploadUrl("upload://a.jpeg");
+  assert.deepEqual(lookup, {});
+});