From 88ea1f9149ffa3b27cca0e47a74a8efa197e046c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Viktor=20Sz=C3=A9pe?= <viktor@szepe.net>
Date: Sun, 31 May 2020 15:04:37 +0200
Subject: [PATCH] Fix invalid Cache-Control header, run [Endpoint] (#5139)

* Fix invalid Cache-Control header
* Update endpoint.tester.js
* Update legacy-request-handler.spec.js

Co-authored-by: chris48s <chris48s@users.noreply.github.com>
---
 core/base-service/cache-headers.js               |  4 ++--
 core/base-service/cache-headers.spec.js          |  4 ++--
 core/base-service/legacy-request-handler.spec.js | 12 ++++++------
 services/endpoint/endpoint.tester.js             |  8 ++++----
 4 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/core/base-service/cache-headers.js b/core/base-service/cache-headers.js
index e8ce9f712f..4b869e716b 100644
--- a/core/base-service/cache-headers.js
+++ b/core/base-service/cache-headers.js
@@ -67,7 +67,7 @@ function setHeadersForCacheLength(res, cacheLengthSeconds) {
     cacheControl = 'no-cache, no-store, must-revalidate'
     expires = nowGMTString
   } else {
-    cacheControl = `max-age=${cacheLengthSeconds} s-maxage=${cacheLengthSeconds}`
+    cacheControl = `max-age=${cacheLengthSeconds}, s-maxage=${cacheLengthSeconds}`
     expires = new Date(now.getTime() + cacheLengthSeconds * 1000).toGMTString()
   }
 
@@ -92,7 +92,7 @@ function setCacheHeaders({
   setHeadersForCacheLength(res, cacheLengthSeconds)
 }
 
-const staticCacheControlHeader = `max-age=${24 * 3600} s-maxage=${24 * 3600}` // 1 day.
+const staticCacheControlHeader = `max-age=${24 * 3600}, s-maxage=${24 * 3600}` // 1 day.
 function setCacheHeadersForStaticResource(res) {
   res.setHeader('Cache-Control', staticCacheControlHeader)
   res.setHeader('Last-Modified', serverStartTimeGMTString)
diff --git a/core/base-service/cache-headers.spec.js b/core/base-service/cache-headers.spec.js
index 65832f50de..c6f5d6afb5 100644
--- a/core/base-service/cache-headers.spec.js
+++ b/core/base-service/cache-headers.spec.js
@@ -147,7 +147,7 @@ describe('Cache header functions', function () {
 
       it('should set the expected Cache-Control header', function () {
         expect(res._headers['cache-control']).to.equal(
-          'max-age=123 s-maxage=123'
+          'max-age=123, s-maxage=123'
         )
       })
 
@@ -187,7 +187,7 @@ describe('Cache header functions', function () {
 
     it('should set the expected Cache-Control header', function () {
       expect(res._headers['cache-control']).to.equal(
-        `max-age=${24 * 3600} s-maxage=${24 * 3600}`
+        `max-age=${24 * 3600}, s-maxage=${24 * 3600}`
       )
     })
 
diff --git a/core/base-service/legacy-request-handler.spec.js b/core/base-service/legacy-request-handler.spec.js
index 580e7ab93d..0e2637b6fd 100644
--- a/core/base-service/legacy-request-handler.spec.js
+++ b/core/base-service/legacy-request-handler.spec.js
@@ -254,7 +254,7 @@ describe('The request handler', function () {
           +new Date(headers.date) + 900000
         ).toGMTString()
         expect(headers.expires).to.equal(expectedExpiry)
-        expect(headers['cache-control']).to.equal('max-age=900 s-maxage=900')
+        expect(headers['cache-control']).to.equal('max-age=900, s-maxage=900')
       })
 
       it('should set the expected cache headers on cached responses', async function () {
@@ -268,7 +268,7 @@ describe('The request handler', function () {
           +new Date(headers.date) + 900000
         ).toGMTString()
         expect(headers.expires).to.equal(expectedExpiry)
-        expect(headers['cache-control']).to.equal('max-age=900 s-maxage=900')
+        expect(headers['cache-control']).to.equal('max-age=900, s-maxage=900')
       })
 
       it('should let live service data override the default cache headers with longer value', async function () {
@@ -289,7 +289,7 @@ describe('The request handler', function () {
         )
 
         const { headers } = await got(`${baseUrl}/testing/123.json`)
-        expect(headers['cache-control']).to.equal('max-age=400 s-maxage=400')
+        expect(headers['cache-control']).to.equal('max-age=400, s-maxage=400')
       })
 
       it('should not let live service data override the default cache headers with shorter value', async function () {
@@ -310,7 +310,7 @@ describe('The request handler', function () {
         )
 
         const { headers } = await got(`${baseUrl}/testing/123.json`)
-        expect(headers['cache-control']).to.equal('max-age=300 s-maxage=300')
+        expect(headers['cache-control']).to.equal('max-age=300, s-maxage=300')
       })
 
       it('should set the expires header to current time + cacheSeconds', async function () {
@@ -322,7 +322,7 @@ describe('The request handler', function () {
           +new Date(headers.date) + 3600000
         ).toGMTString()
         expect(headers.expires).to.equal(expectedExpiry)
-        expect(headers['cache-control']).to.equal('max-age=3600 s-maxage=3600')
+        expect(headers['cache-control']).to.equal('max-age=3600, s-maxage=3600')
       })
 
       it('should ignore cacheSeconds when shorter than defaultCacheLengthSeconds', async function () {
@@ -334,7 +334,7 @@ describe('The request handler', function () {
           +new Date(headers.date) + 600000
         ).toGMTString()
         expect(headers.expires).to.equal(expectedExpiry)
-        expect(headers['cache-control']).to.equal('max-age=600 s-maxage=600')
+        expect(headers['cache-control']).to.equal('max-age=600, s-maxage=600')
       })
 
       it('should set Cache-Control: no-cache, no-store, must-revalidate if cache seconds is 0', async function () {
diff --git a/services/endpoint/endpoint.tester.js b/services/endpoint/endpoint.tester.js
index 26f1a3b167..a121cd0574 100644
--- a/services/endpoint/endpoint.tester.js
+++ b/services/endpoint/endpoint.tester.js
@@ -207,7 +207,7 @@ t.create('cacheSeconds')
       cacheSeconds: 500,
     })
   )
-  .expectHeader('cache-control', 'max-age=500 s-maxage=500')
+  .expectHeader('cache-control', 'max-age=500, s-maxage=500')
 
 t.create('user can override service cacheSeconds')
   .get('.json?url=https://example.com/badge&cacheSeconds=1000')
@@ -219,7 +219,7 @@ t.create('user can override service cacheSeconds')
       cacheSeconds: 500,
     })
   )
-  .expectHeader('cache-control', 'max-age=1000 s-maxage=1000')
+  .expectHeader('cache-control', 'max-age=1000, s-maxage=1000')
 
 t.create('user does not override longer service cacheSeconds')
   .get('.json?url=https://example.com/badge&cacheSeconds=450')
@@ -231,7 +231,7 @@ t.create('user does not override longer service cacheSeconds')
       cacheSeconds: 500,
     })
   )
-  .expectHeader('cache-control', 'max-age=500 s-maxage=500')
+  .expectHeader('cache-control', 'max-age=500, s-maxage=500')
 
 t.create('cacheSeconds does not override longer Shields default')
   .get('.json?url=https://example.com/badge')
@@ -243,7 +243,7 @@ t.create('cacheSeconds does not override longer Shields default')
       cacheSeconds: 10,
     })
   )
-  .expectHeader('cache-control', 'max-age=300 s-maxage=300')
+  .expectHeader('cache-control', 'max-age=300, s-maxage=300')
 
 t.create('Bad scheme')
   .get('.json?url=http://example.com/badge')
-- 
GitLab