Adding error for caching POST requests, Adding tests for checkSWFileC…

…acheHeaders and allows no-store for valid headers (#1336)

infra/testing/sw-env-mocks/Response.js

@@ -28,6 +28,7 @@ class Response {
     this.type = this.status === 0 ? 'opaque' : 'basic';
     this.redirected = false;
     this.url = 'http://example.com/asset';
+    this.method = (options && options.method) || 'GET';
   }
 
   clone() {

packages/workbox-core/_private/cacheWrapper.mjs

@@ -16,6 +16,7 @@
 
 import {logger} from './logger.mjs';
 import {assert} from './assert.mjs';
+import {WorkboxError} from './WorkboxError.mjs';
 import {getFriendlyURL} from '../_private/getFriendlyURL.mjs';
 import pluginEvents from '../models/pluginEvents.mjs';
 import pluginUtils from '../utils/pluginUtils.mjs';
@@ -46,6 +47,15 @@ const putWrapper = async (cacheName, request, response, plugins = []) => {
     return;
   }
 
+  if (process.env.NODE_ENV !== 'production') {
+    if (responseToCache.method && responseToCache.method !== 'GET') {
+      throw new WorkboxError('attempt-to-cache-non-get-request', {
+        url: getFriendlyURL(request.url),
+        method: responseToCache.method,
+      });
+    }
+  }
+
   const cache = await caches.open(cacheName);
 
   const updatePlugins = pluginUtils.filter(

packages/workbox-core/_private/checkSWFileCacheHeaders.mjs

@@ -17,8 +17,6 @@
 import {logger} from './logger.mjs';
 import '../_version.mjs';
 
-const MAX_AGE_REGEX = /max-age\s*=\s*(\d*)/g;
-
 /**
  * Logs a warning to the user recommending changing
  * to max-age=0 or no-cache.
@@ -61,8 +59,7 @@ function checkSWFileCacheHeaders() {
       }
 
       const cacheControlHeader = response.headers.get('cache-control');
-
-      const maxAgeResult = MAX_AGE_REGEX.exec(cacheControlHeader);
+      const maxAgeResult = /max-age\s*=\s*(\d*)/g.exec(cacheControlHeader);
       if (maxAgeResult) {
         if (parseInt(maxAgeResult[1], 10) === 0) {
           return;
@@ -73,6 +70,10 @@ function checkSWFileCacheHeaders() {
         return;
       }
 
+      if (cacheControlHeader.indexOf('no-store') !== -1) {
+        return;
+      }
+
       showWarning(cacheControlHeader);
     } catch (err) {
       // NOOP

packages/workbox-core/models/messages/messages.mjs

@@ -224,4 +224,8 @@ export default {
     return `The start (${start}) and end (${end}) values in the Range are ` +
       `not satisfiable by the cached response, which is ${size} bytes.`;
   },
+  'attempt-to-cache-non-get-request': ({url, method}) => {
+    return `Unable to cache '${url}' because it is a '${method}' request and ` +
+      `only 'GET' requests can be cached.`;
+  },
 };

test/workbox-core/node/_private/test-cacheWrapper.mjs

@@ -2,6 +2,8 @@ import {expect} from 'chai';
 import sinon from 'sinon';
 
 import {cacheWrapper} from '../../../../packages/workbox-core/_private/cacheWrapper.mjs';
+import expectError from '../../../../infra/testing/expectError';
+import {devOnly} from '../../../../infra/testing/env-it';
 
 describe(`workbox-core cacheWrapper`, function() {
   let sandbox;
@@ -58,6 +60,26 @@ describe(`workbox-core cacheWrapper`, function() {
       expect(cachePutStub.callCount).to.equal(0);
     });
 
+    devOnly.it(`should not cache POST responses`, async function() {
+      const testCache = await caches.open('TEST-CACHE');
+      const cacheOpenStub = sandbox.stub(global.caches, 'open');
+      const cachePutStub = sandbox.stub(testCache, 'put');
+      cacheOpenStub.callsFake(async (cacheName) => {
+        return testCache;
+      });
+      const putRequest = new Request('/test/string');
+      const putResponse = new Response('Response for /test/string', {
+        method: 'POST',
+      });
+
+      await expectError(async () => {
+        await cacheWrapper.put('CACHE NAME', putRequest, putResponse);
+      }, 'attempt-to-cache-non-get-request');
+
+      expect(cacheOpenStub.callCount).to.equal(0);
+      expect(cachePutStub.callCount).to.equal(0);
+    });
+
     it(`should call cacheDidUpdate`, async function() {
       const firstPlugin = {
         cacheDidUpdate: () => {},

test/workbox-core/node/_private/test-checkSWFileCacheHeaders.mjs

@@ -0,0 +1,110 @@
+import {expect} from 'chai';
+import sinon from 'sinon';
+
+import {logger} from '../../../../packages/workbox-core/_private/logger.mjs';
+import {checkSWFileCacheHeaders} from '../../../../packages/workbox-core/_private/checkSWFileCacheHeaders.mjs';
+import {devOnly} from '../../../../infra/testing/env-it';
+
+describe(`workbox-core cacheWrapper`, function() {
+  let sandbox;
+
+  before(function() {
+    sandbox = sinon.sandbox.create();
+  });
+
+  afterEach(function() {
+    sandbox.restore();
+  });
+
+  devOnly.it('should handle bad response', async () => {
+    sandbox.stub(self, 'fetch').callsFake(async () => {
+      return new Response('Not Found.', {
+        status: 404,
+      });
+    });
+
+    await checkSWFileCacheHeaders();
+
+    expect(logger.warn.callCount).to.equal(0);
+  });
+
+  devOnly.it('should handle no cache-control header', async () => {
+    sandbox.stub(self, 'fetch').callsFake(async () => {
+      return new Response('OK');
+    });
+
+    await checkSWFileCacheHeaders();
+
+    expect(logger.warn.callCount).to.equal(0);
+  });
+
+  devOnly.it('should log for bad cache-control header', async () => {
+    sandbox.stub(self, 'fetch').callsFake(async () => {
+      return new Response('OK', {
+        headers: {
+          'cache-control': 'max-age=2000',
+        },
+      });
+    });
+
+    await checkSWFileCacheHeaders();
+
+    expect(logger.warn.callCount).to.equal(1);
+  });
+
+  devOnly.it('should handle unexpected max-age cache-control header', async () => {
+    sandbox.stub(self, 'fetch').callsFake(async () => {
+      return new Response('OK', {
+        headers: {
+          'cache-control': 'max-age=abc',
+        },
+      });
+    });
+
+    await checkSWFileCacheHeaders();
+
+    expect(logger.warn.callCount).to.equal(1);
+  });
+
+  devOnly.it('should NOT log for max-age=0 cache-control header', async () => {
+    sandbox.stub(self, 'fetch').callsFake(async () => {
+      return new Response('OK', {
+        headers: {
+          'cache-control': 'max-age=0',
+        },
+      });
+    });
+
+    await checkSWFileCacheHeaders();
+
+    expect(logger.warn.callCount).to.equal(0);
+  });
+
+  devOnly.it('should NOT log for no-cache cache-control header', async () => {
+    sandbox.stub(self, 'fetch').callsFake(async () => {
+      return new Response('OK', {
+        headers: {
+          'cache-control': 'no-cache',
+        },
+      });
+    });
+
+    await checkSWFileCacheHeaders();
+
+    expect(logger.warn.callCount).to.equal(0);
+  });
+
+  devOnly.it('should NOT log for no-store cache-control header', async () => {
+    sandbox.stub(self, 'fetch').callsFake(async () => {
+      return new Response('OK', {
+        headers: {
+          'cache-control': 'no-cache',
+        },
+      });
+    });
+
+    await checkSWFileCacheHeaders();
+
+    expect(logger.warn.callCount).to.equal(0);
+  });
+});