FIX: Stop double prepending of window.location.origin on media URLs (#10275)

FIX: Stop double prepending of window.location.origin on media URLs (#10275)

This fixes an issue where sometimes when composing a post and uploading a video/audio file, _loadCachedShortUrls/the uploads controller would return a full URL with origin, instead of just the URL with the host e.g. http://localhost:3000/some/video.mp4 instead of just //localhost:3000/some/video.mp4. We were prepending window.location.origin onto the URL no matter what, and since http://localhost:3000/some/video.mp4 does not match the host URL regex, we were ending up with something like http://localhost:3000http://localhost:3000/some/video.mp4 which broke composer previews. This was only noticed with a video upload in a secure upload environment.

diff --git a/app/assets/javascripts/pretty-text/addon/upload-short-url.js b/app/assets/javascripts/pretty-text/addon/upload-short-url.js
index 070881f..c8f8c72 100644
--- a/app/assets/javascripts/pretty-text/addon/upload-short-url.js
+++ b/app/assets/javascripts/pretty-text/addon/upload-short-url.js
@@ -144,8 +144,8 @@ function _loadCachedShortUrls(uploadElements, siteSettings, opts) {
             let hostRegex = new RegExp("//" + window.location.host, "g");
             url = url.replace(hostRegex, "");
           }
-          let fullUrl = window.location.origin + url;
-          upload.src = fullUrl;
+
+          upload.src = url;
 
           // this is necessary, otherwise because of the src change the
           // video/audio just doesn't bother loading!
@@ -154,8 +154,8 @@ function _loadCachedShortUrls(uploadElements, siteSettings, opts) {
           // set the url and text for the <a> tag within the <video/audio> tag
           const link = upload.parentElement.querySelector("a");
           if (link) {
-            link.href = fullUrl;
-            link.textContent = fullUrl;
+            link.href = url;
+            link.textContent = url;
           }
         });
 
diff --git a/test/javascripts/lib/upload-short-url-test.js b/test/javascripts/lib/upload-short-url-test.js
index 7c9248e..582ddba 100644
--- a/test/javascripts/lib/upload-short-url-test.js
+++ b/test/javascripts/lib/upload-short-url-test.js
@@ -52,6 +52,11 @@ function stubUrls(imageSrcs, attachmentSrcs, otherMediaSrcs) {
         short_url: "upload://e.mp3",
         url: "/uploads/default/original/3X/c/b/5.mp3",
         short_path: "/uploads/short-url/e.mp3"
+      },
+      {
+        short_url: "upload://f.mp4",
+        url: "http://localhost:3000/uploads/default/original/3X/c/b/6.mp4",
+        short_path: "/uploads/short-url/f.mp4"
       }
     ];
   }
@@ -68,7 +73,16 @@ function stubUrls(imageSrcs, attachmentSrcs, otherMediaSrcs) {
             `<a data-orig-href="${src.short_url}">big enterprise contract.pdf</a>`
         )
         .join("") +
-      `<div class="scoped-area"><img data-orig-src="${imageSrcs[2].url}"></div>`
+      `<div class="scoped-area"><img data-orig-src="${imageSrcs[2].url}"></div>` +
+      otherMediaSrcs
+        .map(src => {
+          if (src.short_url.indexOf("mp3") > -1) {
+            return `<audio controls><source data-orig-src="${src.short_url}"></audio>`;
+          } else {
+            return `<video controls><source data-orig-src="${src.short_url}"></video>`;
+          }
+        })
+        .join("")
   );
 }
 QUnit.module("lib:pretty-text/upload-short-url", {
@@ -120,6 +134,12 @@ QUnit.test("resolveAllShortUrls", async assert => {
     url: "/uploads/default/original/3X/c/b/5.mp3",
     short_path: "/uploads/short-url/e.mp3"
   });
+
+  lookup = lookupCachedUploadUrl("upload://f.mp4");
+  assert.deepEqual(lookup, {
+    url: "http://localhost:3000/uploads/default/original/3X/c/b/6.mp4",
+    short_path: "/uploads/short-url/f.mp4"
+  });
 });
 
 QUnit.test(
@@ -135,10 +155,40 @@ QUnit.test(
       .find("img")
       .eq(1);
     let link = fixture().find("a");
+    let audio = fixture()
+      .find("audio")
+      .eq(0);
+    let video = fixture()
+      .find("video")
+      .eq(0);
 
     assert.equal(image1.attr("src"), "/images/avatar.png?a");
     assert.equal(image2.attr("src"), "/images/avatar.png?b");
     assert.equal(link.attr("href"), "/uploads/short-url/c.pdf");
+    assert.equal(
+      video.find("source").attr("src"),
+      "/uploads/default/original/3X/c/b/4.mp4"
+    );
+    assert.equal(
+      audio.find("source").attr("src"),
+      "/uploads/default/original/3X/c/b/5.mp3"
+    );
+  }
+);
+
+QUnit.test(
+  "resolveAllShortUrls - url with full origin replaced correctly",
+  async assert => {
+    stubUrls();
+    await resolveAllShortUrls(ajax, { secure_media: false }, fixture()[0]);
+    let video = fixture()
+      .find("video")
+      .eq(1);
+
+    assert.equal(
+      video.find("source").attr("src"),
+      "http://localhost:3000/uploads/default/original/3X/c/b/6.mp4"
+    );
   }
 );
 

GitHub sha: 62f2e1f9

1 Like

This commit appears in #10275 which was approved by eviltrout. It was merged by martin.