Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions packages/cache/src/bootstrap/cache.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,54 @@ describe('Cache API', () => {
expect(missResponse).toBeUndefined()
})

test('retains request headers, discarding any forbidden ones', async () => {
const headers = new Headers()
headers.set('content-type', 'text/html')
headers.set('x-custom-header', 'foobar')

const response = new Response('<h1>Hello world</h1>', { headers })
const mockFetch = new MockFetch()
.get({
url: 'https://example.netlify/.netlify/cache/https%3A%2F%2Fnetlify.com%2F',
headers: (headers) => {
expect(headers.Authorization).toBe(`Bearer ${token}`)
expect(headers['netlify-forwarded-host']).toBe(host)
expect(headers['x-custom-header']).toBe('foo')
expect(headers['netlify-programmable-store']).toBe('my-cache')
expect(headers['netlify-programmable-headers']).toBeUndefined()
},
response,
})
.inject()
const cache = new NetlifyCache({
getContext: ({ operation }) => {
expect(operation).toBe(Operation.Read)

return { host, token, url }
},
name: 'my-cache',
userAgent,
})

const cached = await cache.match(
new Request('https://netlify.com', {
headers: {
'X-Custom-Header': 'foo',
'Netlify-Programmable-Headers': 'forbidden',
'Netlify-Programmable-Store': 'not the right store',
'Netlify-Forwarded-Host': 'this would break things',
},
}),
)

mockFetch.restore()

expect(mockFetch.requests.length).toBe(1)

expect(cached?.status).toBe(200)
expect(await cached?.text()).toBe('<h1>Hello world</h1>')
})

test('is a no-op when the `getContext` callback returns `null`', async () => {
const mockFetch = new MockFetch().inject()
const cache = new NetlifyCache({
Expand Down
35 changes: 31 additions & 4 deletions packages/cache/src/bootstrap/cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,17 @@ const allowedProtocols = new Set(['http:', 'https:'])
// sent to the API.
const discardedHeaders = new Set(['cookie', 'content-encoding', 'content-length'])

// Headers with these prefixes will be discarded from requests when doing cache
// lookups; allowing clients to set them could lead to unexpected behavior.
const forbiddenHeaderPrefixes = ['netlify-programmable-', 'x-nf-']

type NetlifyCacheOptions = EnvironmentOptions & {
name: string
}

const getInternalHeaders = Symbol('getInternalHeaders')
const serializeResourceHeaders = Symbol('serializeResourceHeaders')
const serializeRequestHeaders = Symbol('serializeRequestHeaders')
const serializeResponseHeaders = Symbol('serializeResponseHeaders')

export class NetlifyCache implements Cache {
#getContext: RequestContextFactory
Expand Down Expand Up @@ -47,7 +52,26 @@ export class NetlifyCache implements Cache {
return headers
}

private [serializeResourceHeaders](headers: Headers) {
private [serializeRequestHeaders](headers: Headers) {
const headersMap: Record<string, string> = {}

headers.forEach((value, key) => {
const normalizedKey = key.toLowerCase()

// Discard any internal headers that the client should not be setting.
for (const prefix of forbiddenHeaderPrefixes) {
if (normalizedKey.startsWith(prefix)) {
return
}
}

headersMap[normalizedKey] = value
})

return headersMap
}

private [serializeResponseHeaders](headers: Headers) {
const headersMap: Record<string, string[]> = {}

headers.forEach((value, key) => {
Expand Down Expand Up @@ -110,7 +134,10 @@ export class NetlifyCache implements Cache {
const resourceURL = extractAndValidateURL(request)
const cacheURL = `${context.url}/${toCacheKey(resourceURL)}`
const response = await fetch(cacheURL, {
headers: this[getInternalHeaders](context),
headers: {
...(request instanceof Request ? this[serializeRequestHeaders](request.headers) : {}),
...this[getInternalHeaders](context),
},
method: 'GET',
})

Expand Down Expand Up @@ -164,7 +191,7 @@ export class NetlifyCache implements Cache {
body: response.body,
headers: {
...this[getInternalHeaders](context),
[HEADERS.ResourceHeaders]: this[serializeResourceHeaders](response.headers),
[HEADERS.ResourceHeaders]: this[serializeResponseHeaders](response.headers),
[HEADERS.ResourceStatus]: response.status.toString(),
},
// @ts-expect-error https://github.com/whatwg/fetch/pull/1457
Expand Down
45 changes: 41 additions & 4 deletions packages/cache/src/fetchwithcache.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,43 @@ describe('`fetchWithCache`', () => {
expect(mockFetch.requests.length).toBe(2)
})

test('Respects request headers', async () => {
const headers = new Headers()
headers.set('content-type', 'text/html')
headers.set('vary', 'x-custom-header')

const cachedResponse = new Response('<h1>Hello world</h1>', { headers })
const mockFetch = new MockFetch()
.post({
response: new Response(null, { status: 201 }),
url: 'https://example.netlify/.netlify/cache/https%3A%2F%2Fnetlify.com%2F',
})
.get({
headers: (headers) => {
expect(headers.vary).toBe('x-custom-header')
},
response: () => cachedResponse,
url: 'https://example.netlify/.netlify/cache/https%3A%2F%2Fnetlify.com%2F',
})
.inject()
const resourceURL = 'https://netlify.com'

const cache = await caches.open('')
await cache.put(resourceURL, cachedResponse)

const response = await fetchWithCache(resourceURL, {
headers: {
vary: 'x-custom-header',
},
})

expect(await response.text()).toBe('<h1>Hello world</h1>')

mockFetch.restore()

expect(mockFetch.requests.length).toBe(2)
})

test('Throws when used with a method other than GET', async () => {
const mockFetch = new MockFetch().inject()
const resourceURL = 'https://netlify.com'
Expand Down Expand Up @@ -103,7 +140,7 @@ describe('`fetchWithCache`', () => {
url: 'https://example.netlify/.netlify/cache/https%3A%2F%2Fnetlify.com%2F',
response: () => new Response('<h1>Hello world</h1>', { headers }),
})
.get({ url: 'https://netlify.com', response: new Response('<h1>Hello world</h1>', { headers }) })
.get({ url: 'https://netlify.com/', response: new Response('<h1>Hello world</h1>', { headers }) })
.inject()
const resourceURL = 'https://netlify.com'

Expand Down Expand Up @@ -152,7 +189,7 @@ describe('`fetchWithCache`', () => {
response: new Response('<h1>Hello world</h1>', { headers }),
})
.get({
url: 'https://netlify.com',
url: 'https://netlify.com/',
response: new Response('<h1>Hello world</h1>', { headers }),
})
.inject()
Expand Down Expand Up @@ -218,7 +255,7 @@ describe('`fetchWithCache`', () => {
response: new Response('<h1>Hello world</h1>', { headers }),
})
.get({
url: 'https://netlify.com',
url: 'https://netlify.com/',
response: new Response('<h1>Hello world</h1>', { headers }),
})
.inject()
Expand Down Expand Up @@ -253,7 +290,7 @@ describe('`fetchWithCache`', () => {
response: new Response(html),
})
.get({
url: 'https://netlify.com',
url: 'https://netlify.com/',
response: new Response(html),
})
.inject()
Expand Down
14 changes: 4 additions & 10 deletions packages/cache/src/fetchwithcache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ type FetchWithCache = {
* whether it comes from the cache or from the network.
*/
export const fetchWithCache: FetchWithCache = async (
request: string | URL | Request,
requestOrURL: string | URL | Request,
optionsOrCacheSettings?: RequestInit | CacheSettings,
cacheOptionsParam?: CacheOptions,
) => {
Expand All @@ -87,15 +87,9 @@ export const fetchWithCache: FetchWithCache = async (
requestInit = {}
}

let method: string | undefined
const request = new Request(requestOrURL, requestInit)

if (request instanceof Request) {
method = request.method
} else {
method = requestInit?.method
}

if (method && method?.toLowerCase() !== 'get') {
if (request.method.toLowerCase() !== 'get') {
throw new TypeError('`fetchWithCache` only supports GET requests.')
}

Expand All @@ -121,7 +115,7 @@ export const fetchWithCache: FetchWithCache = async (
return cached
}

const fresh = await fetch(request, requestInit)
const fresh = await fetch(request)
if (!fresh.body) {
return fresh
}
Expand Down
Loading