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')
  ) {