Merge pull request #121 from vector-im/bwindels/fix-safari-sw-fetch-cache

Dont use no-cache in fetch, as it doesn't play well with CORS on Safari

src/matrix/net/common.js

@@ -26,3 +26,25 @@ export function encodeQueryParams(queryParams) {
         })
         .join("&");
 }
+
+export function addCacheBuster(urlStr, random = Math.random) {
+    // XHR doesn't have a good way to disable cache,
+    // so add a random query param
+    // see https://davidtranscend.com/blog/prevent-ie11-cache-ajax-requests/
+    if (urlStr.includes("?")) {
+        urlStr = urlStr + "&";
+    } else {
+        urlStr = urlStr + "?";
+    }
+    return urlStr + `_cacheBuster=${Math.ceil(random() * Number.MAX_SAFE_INTEGER)}`;
+}
+
+export function tests() {
+    return {
+        "add cache buster": assert => {
+            const random = () => 0.5;
+            assert.equal(addCacheBuster("http://foo", random), "http://foo?_cacheBuster=4503599627370496");
+            assert.equal(addCacheBuster("http://foo?bar=baz", random), "http://foo?bar=baz&_cacheBuster=4503599627370496");
+        }
+    }
+}

src/matrix/net/request/fetch.js

@@ -20,6 +20,7 @@ import {
     ConnectionError
 } from "../../error.js";
 import {abortOnTimeout} from "../timeout.js";
+import {addCacheBuster} from "../common.js";
 
 class RequestResult {
     constructor(promise, controller) {
@@ -57,11 +58,23 @@ export function createFetchRequest(createTimeout) {
                 signal: controller.signal
             });
         }
+        url = addCacheBuster(url);
         options = Object.assign(options, {
             mode: "cors",
             credentials: "omit",
             referrer: "no-referrer",
-            cache: "no-cache",
+            // ideally we'd turn off cache here, but Safari interprets
+            // `Access-Control-Allow-Headers` strictly (only when fetch is
+            // intercepted by a service worker strangely enough), in that
+            // it gives a CORS error if Cache-Control is not present
+            // in the list of allowed headers (which it isn't commonly, at least not on matrix.org).
+            // With no-store or no-cache here, it does set `Cache-Control`
+            // so we don't do that, and prevent caching with `addCacheBuster`.
+            // We also hope the server responds with `Cache-Control: no-store` so
+            // we don't fill the http cache with api responses.
+            // 
+            // cache: "no-store",
+            cache: "default",
         });
         if (options.headers) {
             const headers = new Headers();

src/matrix/net/request/xhr.js

@@ -18,6 +18,7 @@ import {
     AbortError,
     ConnectionError
 } from "../../error.js";
+import {addCacheBuster} from "../common.js";
 
 class RequestResult {
     constructor(promise, xhr) {
@@ -60,18 +61,6 @@ function xhrAsPromise(xhr, method, url) {
     });
 }
 
-function addCacheBuster(urlStr, random = Math.random) {
-    // XHR doesn't have a good way to disable cache,
-    // so add a random query param
-    // see https://davidtranscend.com/blog/prevent-ie11-cache-ajax-requests/
-    if (urlStr.includes("?")) {
-        urlStr = urlStr + "&";
-    } else {
-        urlStr = urlStr + "?";
-    }
-    return urlStr + `_cacheBuster=${Math.ceil(random() * Number.MAX_SAFE_INTEGER)}`;
-}
-
 export function xhrRequest(url, options) {
     url = addCacheBuster(url);
     const xhr = send(url, options);
@@ -85,13 +74,3 @@ export function xhrRequest(url, options) {
     });
     return new RequestResult(promise, xhr);
 }
-
-export function tests() {
-    return {
-        "add cache buster": assert => {
-            const random = () => 0.5;
-            assert.equal(addCacheBuster("http://foo", random), "http://foo?_cacheBuster=4503599627370496");
-            assert.equal(addCacheBuster("http://foo?bar=baz", random), "http://foo?bar=baz&_cacheBuster=4503599627370496");
-        }
-    }
-}