fix: ensure cdn cache control only set for get and head methods (#137)

* fix: ensure cdn cache control only for get and head

* test: pass requests in tests and add HEAD and POST test cases

---------

Co-authored-by: Michal Piechowiak <misiek.piechowiak@gmail.com>

Author: orinokai

src/run/handlers/server.ts

@@ -1,5 +1,4 @@
 import { toComputeResponse, toReqRes } from '@fastly/http-compute-js'
-import { HeadersSentEvent } from '@fastly/http-compute-js/dist/http-compute-js/http-outgoing.js'
 import type { NextConfigComplete } from 'next/dist/server/config-shared.js'
 import type { WorkerRequestHandler } from 'next/dist/server/lib/types.js'
 
@@ -55,10 +54,9 @@ export default async (request: Request) => {
 
   await adjustDateHeader(response.headers, request)
 
-  setCacheControlHeaders(response.headers)
+  setCacheControlHeaders(response.headers, request)
   setCacheTagsHeaders(response.headers, request, tagsManifest)
   setVaryHeaders(response.headers, request, nextConfig)
 
-
   return response
 }

src/run/headers.test.ts

@@ -144,11 +144,14 @@ describe('headers', () => {
   })
 
   describe('setCacheControlHeaders', () => {
+    const defaultUrl = 'https://example.com'
+
     test('should not set any headers if "cache-control" is not set', () => {
       const headers = new Headers()
+      const request = new Request(defaultUrl)
       vi.spyOn(headers, 'set')
 
-      setCacheControlHeaders(headers)
+      setCacheControlHeaders(headers, request)
 
       expect(headers.set).toHaveBeenCalledTimes(0)
     })
@@ -159,9 +162,10 @@ describe('headers', () => {
         'cdn-cache-control': 'public, max-age=0, must-revalidate',
       }
       const headers = new Headers(givenHeaders)
+      const request = new Request(defaultUrl)
       vi.spyOn(headers, 'set')
 
-      setCacheControlHeaders(headers)
+      setCacheControlHeaders(headers, request)
 
       expect(headers.set).toHaveBeenCalledTimes(0)
     })
@@ -172,21 +176,23 @@ describe('headers', () => {
         'netlify-cdn-cache-control': 'public, max-age=0, must-revalidate',
       }
       const headers = new Headers(givenHeaders)
+      const request = new Request(defaultUrl)
       vi.spyOn(headers, 'set')
 
-      setCacheControlHeaders(headers)
+      setCacheControlHeaders(headers, request)
 
       expect(headers.set).toHaveBeenCalledTimes(0)
     })
 
-    test('should set expected headers if "cache-control" is set and "cdn-cache-control" and "netlify-cdn-cache-control" are not present', () => {
+    test('should set expected headers if "cache-control" is set and "cdn-cache-control" and "netlify-cdn-cache-control" are not present (GET request)', () => {
       const givenHeaders = {
         'cache-control': 'public, max-age=0, must-revalidate',
       }
       const headers = new Headers(givenHeaders)
+      const request = new Request(defaultUrl)
       vi.spyOn(headers, 'set')
 
-      setCacheControlHeaders(headers)
+      setCacheControlHeaders(headers, request)
 
       expect(headers.set).toHaveBeenNthCalledWith(
         1,
@@ -200,14 +206,50 @@ describe('headers', () => {
       )
     })
 
+    test('should set expected headers if "cache-control" is set and "cdn-cache-control" and "netlify-cdn-cache-control" are not present (HEAD request)', () => {
+      const givenHeaders = {
+        'cache-control': 'public, max-age=0, must-revalidate',
+      }
+      const headers = new Headers(givenHeaders)
+      const request = new Request(defaultUrl, { method: 'HEAD' })
+      vi.spyOn(headers, 'set')
+
+      setCacheControlHeaders(headers, request)
+
+      expect(headers.set).toHaveBeenNthCalledWith(
+        1,
+        'cache-control',
+        'public, max-age=0, must-revalidate',
+      )
+      expect(headers.set).toHaveBeenNthCalledWith(
+        2,
+        'netlify-cdn-cache-control',
+        'public, max-age=0, must-revalidate',
+      )
+    })
+
+    test('should not set any headers on POST request', () => {
+      const givenHeaders = {
+        'cache-control': 'public, max-age=0, must-revalidate',
+      }
+      const headers = new Headers(givenHeaders)
+      const request = new Request(defaultUrl, { method: 'POST' })
+      vi.spyOn(headers, 'set')
+
+      setCacheControlHeaders(headers, request)
+
+      expect(headers.set).toHaveBeenCalledTimes(0)
+    })
+
     test('should remove "s-maxage" from "cache-control" header', () => {
       const givenHeaders = {
         'cache-control': 'public, s-maxage=604800',
       }
       const headers = new Headers(givenHeaders)
+      const request = new Request(defaultUrl)
       vi.spyOn(headers, 'set')
 
-      setCacheControlHeaders(headers)
+      setCacheControlHeaders(headers, request)
 
       expect(headers.set).toHaveBeenNthCalledWith(1, 'cache-control', 'public')
       expect(headers.set).toHaveBeenNthCalledWith(
@@ -222,9 +264,10 @@ describe('headers', () => {
         'cache-control': 'max-age=604800, stale-while-revalidate=86400',
       }
       const headers = new Headers(givenHeaders)
+      const request = new Request(defaultUrl)
       vi.spyOn(headers, 'set')
 
-      setCacheControlHeaders(headers)
+      setCacheControlHeaders(headers, request)
 
       expect(headers.set).toHaveBeenNthCalledWith(1, 'cache-control', 'max-age=604800')
       expect(headers.set).toHaveBeenNthCalledWith(
@@ -239,9 +282,10 @@ describe('headers', () => {
         'cache-control': 's-maxage=604800, stale-while-revalidate=86400',
       }
       const headers = new Headers(givenHeaders)
+      const request = new Request(defaultUrl)
       vi.spyOn(headers, 'set')
 
-      setCacheControlHeaders(headers)
+      setCacheControlHeaders(headers, request)
 
       expect(headers.set).toHaveBeenNthCalledWith(
         1,

src/run/headers.ts

@@ -109,10 +109,11 @@ export const adjustDateHeader = async (headers: Headers, request: Request) => {
  * Ensure stale-while-revalidate and s-maxage don't leak to the client, but
  * assume the user knows what they are doing if CDN cache controls are set
  */
-export const setCacheControlHeaders = (headers: Headers) => {
+export const setCacheControlHeaders = (headers: Headers, request: Request) => {
   const cacheControl = headers.get('cache-control')
   if (
     cacheControl !== null &&
+    ['GET', 'HEAD'].includes(request.method) &&
     !headers.has('cdn-cache-control') &&
     !headers.has('netlify-cdn-cache-control')
   ) {