Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cache: make vary headers case-insensitive #3990

Closed
Closed
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
8 changes: 5 additions & 3 deletions lib/handler/cache-handler.js
Original file line number Diff line number Diff line change
@@ -4,7 +4,8 @@
const {
parseCacheControlHeader,
parseVaryHeader,
isEtagUsable
isEtagUsable,
makeHeaderNamesLowercase

Check failure on line 8 in lib/handler/cache-handler.js

GitHub Actions / Lint

'makeHeaderNamesLowercase' is assigned a value but never used
} = require('../util/cache')
const { parseHttpDate } = require('../util/date.js')

@@ -96,7 +97,7 @@
statusMessage
)

if (
if (

Check failure on line 100 in lib/handler/cache-handler.js

GitHub Actions / Lint

Expected indentation of 4 spaces but found 6
!util.safeHTTPMethods.includes(this.#cacheKey.method) &&
statusCode >= 200 &&
statusCode <= 399
@@ -115,7 +116,7 @@
const heuristicallyCacheable = resHeaders['last-modified'] && HEURISTICALLY_CACHEABLE_STATUS_CODES.includes(statusCode)
if (
!cacheControlHeader &&
!resHeaders['expires'] &&
!resHeaders.expires &&
!heuristicallyCacheable &&
!this.#cacheByDefault
) {
@@ -157,6 +158,7 @@
let varyDirectives
if (this.#cacheKey.headers && resHeaders.vary) {
varyDirectives = parseVaryHeader(resHeaders.vary, this.#cacheKey.headers)

if (!varyDirectives) {
// Parse error
return downstreamOnHeaders()
2 changes: 1 addition & 1 deletion lib/interceptor/cache.js
Original file line number Diff line number Diff line change
@@ -318,7 +318,7 @@
// Not a method we want to cache or we don't have the origin, skip
return dispatch(opts, handler)
}

console.log('reqHeaders=', opts.headers)

Check failure on line 321 in lib/interceptor/cache.js

GitHub Actions / Lint

Expected indentation of 6 spaces but found 0
const reqCacheControl = opts.headers?.['cache-control']
? parseCacheControlHeader(opts.headers['cache-control'])
: undefined
19 changes: 17 additions & 2 deletions lib/util/cache.js
Original file line number Diff line number Diff line change
@@ -38,7 +38,7 @@ function makeCacheKey (opts) {
origin: opts.origin.toString(),
method: opts.method,
path: opts.path,
headers
headers: makeHeaderNamesLowercase(headers)
}
}

@@ -347,6 +347,20 @@ function assertCacheMethods (methods, name = 'CacheMethods') {
}
}

/**
* @param {import('../../types/header.d.ts').IncomingHttpHeaders} headers
* @returns {import('../../types/header.d.ts').IncomingHttpHeaders}
*/
function makeHeaderNamesLowercase (headers) {
const lowercased = {}

for (const header of Object.keys(headers)) {
lowercased[header.toLowerCase()] = headers[header]
}

return lowercased
}

module.exports = {
makeCacheKey,
assertCacheKey,
@@ -355,5 +369,6 @@ module.exports = {
parseVaryHeader,
isEtagUsable,
assertCacheMethods,
assertCacheStore
assertCacheStore,
makeHeaderNamesLowercase
}
45 changes: 45 additions & 0 deletions test/interceptors/cache.js
Original file line number Diff line number Diff line change
@@ -128,6 +128,51 @@ describe('Cache Interceptor', () => {
}
})

test('vary directives are case-insensitive', async () => {
let requestsToOrigin = 0
const server = createServer((_, res) => {
requestsToOrigin++

res.setHeader('date', 0)
res.setHeader('cache-control', 'max-age=5000')
res.setHeader('vary', 'FoO, bar, bAZ')

res.end('asd')
}).listen(0)

const client = new Client(`http://localhost:${server.address().port}`)
.compose(interceptors.cache())

after(async () => {
server.close()
await client.close()
})

await once(server, 'listening')

strictEqual(requestsToOrigin, 0)

/**
* @type {import('../../types/dispatcher').default.RequestOptions}
*/
const request = {
origin: 'localhost',
method: 'GET',
path: '/',
headers: {
Foo: '1',
BAr: 'abc',
BAZ: '789'
}
}

await client.request(request)
equal(requestsToOrigin, 1)

await client.request(request)
equal(requestsToOrigin, 1)
})

test('stale responses are revalidated before deleteAt (if-modified-since)', async () => {
const clock = FakeTimers.install({
shouldClearNativeTimers: true
5 changes: 5 additions & 0 deletions tmp.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { Client, interceptors } from './index.js'

Check failure on line 1 in tmp.mjs

GitHub Actions / Lint

Multiple spaces found before 'from'

const client = new Client('https://google.com').compose(interceptors.cache())

await client.request({ path: '/', method: 'GET', origin: 'google.com' })