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
This commit is contained in:
Kyle Zhao 2018-10-11 08:55:36 +08:00 committed by GitHub
parent 59be289084
commit ffc241eb25
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

View File

@ -11,7 +11,6 @@ function loadWithTag(path, cb) {
if (Ember.Test) { if (Ember.Test) {
Ember.Test.registerWaiter(() => finished); Ember.Test.registerWaiter(() => finished);
} }
head.appendChild(s);
s.onload = s.onreadystatechange = function(_, abort) { s.onload = s.onreadystatechange = function(_, abort) {
finished = true; finished = true;
@ -27,6 +26,8 @@ function loadWithTag(path, cb) {
} }
} }
}; };
head.appendChild(s);
} }
export function loadCSS(url) { export function loadCSS(url) {
@ -41,17 +42,19 @@ export default function loadScript(url, opts) {
opts = 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) => { $("script").each((i, tag) => {
const src = tag.getAttribute("src"); const src = tag.getAttribute("src");
if (src && src !== url) { if (src && src !== url && !_loading[src]) {
_loaded[tag.getAttribute("src")] = true; _loaded[src] = true;
} }
}); });
return new Ember.RSVP.Promise(function(resolve) { return new Ember.RSVP.Promise(function(resolve) {
url = Discourse.getURL(url);
// If we already loaded this url // If we already loaded this url
if (_loaded[url]) { if (_loaded[url]) {
return resolve(); return resolve();
@ -78,23 +81,15 @@ export default function loadScript(url, opts) {
_loaded[url] = true; _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) { if (opts.css) {
ajax({ ajax({
url: cdnUrl, url: url,
dataType: "text", dataType: "text",
cache: true cache: true
}).then(cb); }).then(cb);
} else { } else {
// Always load JavaScript with script tag to avoid Content Security Policy inline violations // Always load JavaScript with script tag to avoid Content Security Policy inline violations
loadWithTag(cdnUrl, cb); loadWithTag(url, cb);
} }
}); });
} }