From ffc241eb253ff426cce07ab39c7ee248ebc1b114 Mon Sep 17 00:00:00 2001
From: Kyle Zhao <kzhao.sw@gmail.com>
Date: Thu, 11 Oct 2018 08:55:36 +0800
Subject: [PATCH] FIX: multiple loadScript to the same url may resolve
 prematurely  (#6474)

This is how `loadScript(url)` currently deals with multiple concurrent requests

1. Check existing `<script>` tags, and mark existing scripts (other than the
   input `url`) as loaded
2. Find "true" `url` of the requested resource (CDN, subfolder path, etc)
3. Check if we have loaded the resource with that "true" `url`, and resolve
   immediately if we have
4. Otherwise insert a `<script>` tag with the "true" `url` to load it

For example, in a subfolder install:

- Input `url` = `/javascripts/script.js`
- "True" `url` = `/subfolder/javascript/script.js`

And the _very_ subtle bug here is that we should use also use the true `url`
for step (1), because:

- Since the input and true `url` are different, we mistakenly mark the true
  `url` as loaded in step one
- After finding the true `url`, and setting `loaded[trueUrl] = true` in (1), we
  resolve the promise prematurely, when the resource could still be loading
---
 .../discourse/lib/load-script.js.es6          | 25 ++++++++-----------
 1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/app/assets/javascripts/discourse/lib/load-script.js.es6 b/app/assets/javascripts/discourse/lib/load-script.js.es6
index 087ec344bc2..34b3f2897b7 100644
--- a/app/assets/javascripts/discourse/lib/load-script.js.es6
+++ b/app/assets/javascripts/discourse/lib/load-script.js.es6
@@ -11,7 +11,6 @@ function loadWithTag(path, cb) {
   if (Ember.Test) {
     Ember.Test.registerWaiter(() => finished);
   }
-  head.appendChild(s);
 
   s.onload = s.onreadystatechange = function(_, abort) {
     finished = true;
@@ -27,6 +26,8 @@ function loadWithTag(path, cb) {
       }
     }
   };
+
+  head.appendChild(s);
 }
 
 export function loadCSS(url) {
@@ -41,17 +42,19 @@ export default function loadScript(url, opts) {
 
   opts = opts || {};
 
+  // Scripts should always load from CDN
+  // CSS is type text, to accept it from a CDN we would need to handle CORS
+  url = opts.css ? Discourse.getURL(url) : Discourse.getURLWithCDN(url);
+
   $("script").each((i, tag) => {
     const src = tag.getAttribute("src");
 
-    if (src && src !== url) {
-      _loaded[tag.getAttribute("src")] = true;
+    if (src && src !== url && !_loading[src]) {
+      _loaded[src] = true;
     }
   });
 
   return new Ember.RSVP.Promise(function(resolve) {
-    url = Discourse.getURL(url);
-
     // If we already loaded this url
     if (_loaded[url]) {
       return resolve();
@@ -78,23 +81,15 @@ export default function loadScript(url, opts) {
       _loaded[url] = true;
     };
 
-    let cdnUrl = url;
-
-    // Scripts should always load from CDN
-    // CSS is type text, to accept it from a CDN we would need to handle CORS
-    if (!opts.css && Discourse.CDN && url[0] === "/" && url[1] !== "/") {
-      cdnUrl = Discourse.CDN.replace(/\/$/, "") + url;
-    }
-
     if (opts.css) {
       ajax({
-        url: cdnUrl,
+        url: url,
         dataType: "text",
         cache: true
       }).then(cb);
     } else {
       // Always load JavaScript with script tag to avoid Content Security Policy inline violations
-      loadWithTag(cdnUrl, cb);
+      loadWithTag(url, cb);
     }
   });
 }