Skip to content

Commit

Permalink
fix(connection): disable caching when on an insecure connection (apac…
Browse files Browse the repository at this point in the history 
…he-superset#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 apache-superset#193
  • Loading branch information
@schoel-bis @kristw
schoel-bis authored and kristw committed Jul 23, 2019
1 parent 3277a2a commit ca256cd
Show file tree
Hide file tree
Showing 3 changed files with 114 additions and 67 deletions.
6 changes: 5 additions & 1 deletion packages/superset-ui-connection/src/callApi/callApi.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
167 changes: 101 additions & 66 deletions packages/superset-ui-connection/test/callApi/callApi.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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!');
});
});

Expand Down
8 changes: 8 additions & 0 deletions test/setupTests.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,14 @@ class CacheStorage {
resolve(new Cache(key));
});
}
delete(key: string): Promise<boolean> {
const wasPresent = key in caches;
if (wasPresent) {
caches[key] = undefined;
}

return Promise.resolve(wasPresent);
}
};

global.caches = new CacheStorage();

0 comments on commit ca256cd

Please sign in to comment.