From ca256cd5aa8a4b0c9f05e57017814510f444e21e Mon Sep 17 00:00:00 2001 From: schoel-bis Date: Wed, 24 Jul 2019 01:03:40 +0200 Subject: [PATCH] fix(connection): disable caching when on an insecure connection (#194) When the page is served over an insecure connection, some browsers (Firefox) will disable the CacheStorage API for security reasons and will throw an error when an attempt is made to use it. Thus, do not even attempt to use CacheStorage on such connections in the first place. fix #193 --- .../src/callApi/callApi.ts | 6 +- .../test/callApi/callApi.test.ts | 167 +++++++++++------- test/setupTests.ts | 8 + 3 files changed, 114 insertions(+), 67 deletions(-) diff --git a/packages/superset-ui-connection/src/callApi/callApi.ts b/packages/superset-ui-connection/src/callApi/callApi.ts index 4a71ab7e63..9d4fbe0ff2 100644 --- a/packages/superset-ui-connection/src/callApi/callApi.ts +++ b/packages/superset-ui-connection/src/callApi/callApi.ts @@ -28,7 +28,11 @@ export default function callApi({ signal, }; - if (method === 'GET' && CACHE_AVAILABLE) { + if ( + method === 'GET' && + CACHE_AVAILABLE && + (self.location && self.location.protocol) === 'https:' + ) { return caches.open(CACHE_KEY).then(supersetCache => supersetCache .match(url) diff --git a/packages/superset-ui-connection/test/callApi/callApi.test.ts b/packages/superset-ui-connection/test/callApi/callApi.test.ts index f66e088e2b..761de30de6 100644 --- a/packages/superset-ui-connection/test/callApi/callApi.test.ts +++ b/packages/superset-ui-connection/test/callApi/callApi.test.ts @@ -300,83 +300,118 @@ describe('callApi()', () => { }); }); - it('caches requests with ETags', () => - callApi({ url: mockCacheUrl, method: 'GET' }).then(() => { + describe('caching', () => { + const origLocation = self.location; + + beforeAll(() => { + Object.defineProperty(self, 'location', { value: {} }); + }); + + afterAll(() => { + Object.defineProperty(self, 'location', { value: origLocation }); + }); + + beforeEach(() => { + self.location.protocol = 'https:'; + + return caches.delete(constants.CACHE_KEY); + }); + + it('caches requests with ETags', () => + callApi({ url: mockCacheUrl, method: 'GET' }).then(() => { + const calls = fetchMock.calls(mockCacheUrl); + expect(calls).toHaveLength(1); + + return caches.open(constants.CACHE_KEY).then(supersetCache => + supersetCache.match(mockCacheUrl).then(cachedResponse => { + expect(cachedResponse).toBeDefined(); + + return true; + }), + ); + })); + + it('will not use cache when running off an insecure connection', () => { + self.location.protocol = 'http:'; + + return callApi({ url: mockCacheUrl, method: 'GET' }).then(() => { + const calls = fetchMock.calls(mockCacheUrl); + expect(calls).toHaveLength(1); + + return caches.open(constants.CACHE_KEY).then(supersetCache => + supersetCache.match(mockCacheUrl).then(cachedResponse => { + expect(cachedResponse).toBeUndefined(); + + return true; + }), + ); + }); + }); + + it('works when the Cache API is disabled', async () => { + Object.defineProperty(constants, 'CACHE_AVAILABLE', { value: false }); + + const firstResponse = await callApi({ url: mockCacheUrl, method: 'GET' }); const calls = fetchMock.calls(mockCacheUrl); expect(calls).toHaveLength(1); + const firstBody = await firstResponse.text(); + expect(firstBody).toEqual('BODY'); + + const secondResponse = await callApi({ url: mockCacheUrl, method: 'GET' }); + const fetchParams = calls[1][1]; + expect(calls).toHaveLength(2); + // second call should not have If-None-Match header + expect(fetchParams.headers).toBeUndefined(); + const secondBody = await secondResponse.text(); + expect(secondBody).toEqual('BODY'); + + Object.defineProperty(constants, 'CACHE_AVAILABLE', { value: true }); + }); - return caches.open(constants.CACHE_KEY).then(supersetCache => - supersetCache.match(mockCacheUrl).then(cachedResponse => { - expect(cachedResponse).toBeDefined(); + it('sends known ETags in the If-None-Match header', () => + // first call sets the cache + callApi({ url: mockCacheUrl, method: 'GET' }).then(() => { + const calls = fetchMock.calls(mockCacheUrl); + expect(calls).toHaveLength(1); + + // second call sends the Etag in the If-None-Match header + return callApi({ url: mockCacheUrl, method: 'GET' }).then(() => { + const fetchParams = calls[1][1]; + const headers = { 'If-None-Match': 'etag' }; + expect(calls).toHaveLength(2); + expect(fetchParams.headers).toEqual(expect.objectContaining(headers)); return true; - }), - ); - })); - - it('works when the Cache API is disabled', async () => { - Object.defineProperty(constants, 'CACHE_AVAILABLE', { value: false }); - - const firstResponse = await callApi({ url: mockCacheUrl, method: 'GET' }); - const calls = fetchMock.calls(mockCacheUrl); - expect(calls).toHaveLength(1); - const firstBody = await firstResponse.text(); - expect(firstBody).toEqual('BODY'); - - const secondResponse = await callApi({ url: mockCacheUrl, method: 'GET' }); - const fetchParams = calls[1][1]; - expect(calls).toHaveLength(2); - // second call should not have If-None-Match header - expect(fetchParams.headers).toBeUndefined(); - const secondBody = await secondResponse.text(); - expect(secondBody).toEqual('BODY'); - - Object.defineProperty(constants, 'CACHE_AVAILABLE', { value: true }); - }); + }); + })); - it('sends known ETags in the If-None-Match header', () => - // first call sets the cache - callApi({ url: mockCacheUrl, method: 'GET' }).then(() => { + it('reuses cached responses on 304 status', async () => { + // first call sets the cache + await callApi({ url: mockCacheUrl, method: 'GET' }); const calls = fetchMock.calls(mockCacheUrl); expect(calls).toHaveLength(1); + // second call reuses the cached payload on a 304 + const mockCachedPayload = { status: 304 }; + fetchMock.get(mockCacheUrl, mockCachedPayload, { overwriteRoutes: true }); + + const secondResponse = await callApi({ url: mockCacheUrl, method: 'GET' }); + expect(calls).toHaveLength(2); + const secondBody = await secondResponse.text(); + expect(secondBody).toEqual('BODY'); + }); - // second call sends the Etag in the If-None-Match header - return callApi({ url: mockCacheUrl, method: 'GET' }).then(() => { - const fetchParams = calls[1][1]; - const headers = { 'If-None-Match': 'etag' }; - expect(calls).toHaveLength(2); - expect(fetchParams.headers).toEqual(expect.objectContaining(headers)); + it('throws error when cache fails on 304', () => { + // this should never happen, since a 304 is only returned if we have + // the cached response and sent the If-None-Match header + const mockUncachedUrl = '/mock/uncached/url'; + const mockCachedPayload = { status: 304 }; + fetchMock.get(mockUncachedUrl, mockCachedPayload); - return true; + return callApi({ url: mockUncachedUrl, method: 'GET' }).catch(error => { + const calls = fetchMock.calls(mockUncachedUrl); + expect(calls).toHaveLength(1); + expect(error.message).toEqual('Received 304 but no content is cached!'); }); - })); - - it('reuses cached responses on 304 status', async () => { - // first call sets the cache - await callApi({ url: mockCacheUrl, method: 'GET' }); - const calls = fetchMock.calls(mockCacheUrl); - expect(calls).toHaveLength(1); - // second call reuses the cached payload on a 304 - const mockCachedPayload = { status: 304 }; - fetchMock.get(mockCacheUrl, mockCachedPayload, { overwriteRoutes: true }); - - const secondResponse = await callApi({ url: mockCacheUrl, method: 'GET' }); - expect(calls).toHaveLength(2); - const secondBody = await secondResponse.text(); - expect(secondBody).toEqual('BODY'); - }); - - it('throws error when cache fails on 304', () => { - // this should never happen, since a 304 is only returned if we have - // the cached response and sent the If-None-Match header - const mockUncachedUrl = '/mock/uncached/url'; - const mockCachedPayload = { status: 304 }; - fetchMock.get(mockUncachedUrl, mockCachedPayload); - - return callApi({ url: mockUncachedUrl, method: 'GET' }).catch(error => { - const calls = fetchMock.calls(mockUncachedUrl); - expect(calls).toHaveLength(1); - expect(error.message).toEqual('Received 304 but no content is cached!'); }); }); diff --git a/test/setupTests.ts b/test/setupTests.ts index f2a648d7f1..f568acb35b 100644 --- a/test/setupTests.ts +++ b/test/setupTests.ts @@ -25,6 +25,14 @@ class CacheStorage { resolve(new Cache(key)); }); } + delete(key: string): Promise { + const wasPresent = key in caches; + if (wasPresent) { + caches[key] = undefined; + } + + return Promise.resolve(wasPresent); + } }; global.caches = new CacheStorage();