Skip to content

Commit

Permalink
feat: cache etag support
Browse files Browse the repository at this point in the history
Signed-off-by: flakey5 <[email protected]>
  • Loading branch information
flakey5 committed Nov 17, 2024
1 parent 1779440 commit d914909
Show file tree
Hide file tree
Showing 6 changed files with 152 additions and 6 deletions.
16 changes: 13 additions & 3 deletions lib/handler/cache-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ const util = require('../core/util')
const DecoratorHandler = require('../handler/decorator-handler')
const {
parseCacheControlHeader,
parseVaryHeader
parseVaryHeader,
isEtagValid
} = require('../util/cache')

function noop () {}
Expand Down Expand Up @@ -135,15 +136,24 @@ class CacheHandler extends DecoratorHandler {
cacheControlDirectives
)

this.#writeStream = this.#store.createWriteStream(this.#cacheKey, {
/**
* @type {import('../../types/cache-interceptor.d.ts').default.CachedResponse}
*/
const value = {
statusCode,
statusMessage,
rawHeaders: strippedHeaders,
vary: varyDirectives,
cachedAt: now,
staleAt,
deleteAt
})
}

if (typeof headers.etag === 'string' && isEtagValid(headers.etag)) {
value.etag = headers.etag
}

this.#writeStream = this.#store.createWriteStream(this.#cacheKey, value)

if (this.#writeStream) {
const handler = this
Expand Down
3 changes: 2 additions & 1 deletion lib/interceptor/cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,8 @@ module.exports = (opts = {}) => {
...opts,
headers: {
...opts.headers,
'if-modified-since': new Date(result.cachedAt).toUTCString()
'if-modified-since': new Date(result.cachedAt).toUTCString(),
etag: result.etag
}
},
new CacheRevalidationHandler(
Expand Down
43 changes: 43 additions & 0 deletions lib/util/cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,48 @@ function parseVaryHeader (varyHeader, headers) {
return output
}

/**
* Note: this deviates from the spec a little. Empty etags ("", W/"") are valid,
* however, including them in cached resposnes serves little to no purpose.
*
* @see https://www.rfc-editor.org/rfc/rfc9110.html#name-etag
*
* @param {string} etag
* @returns {boolean}
*/
function isEtagValid (etag) {
if (etag.length <= 2) {
// Shortest an etag can be is two chars (just ""). This is where we deviate
// from the spec requiring a min of 3 chars however
return false
}

if (etag[0] === '"' && etag[etag.length - 1] === '"') {
if (etag[1] === '"' || etag.startsWith('"W/')) {
// ETag: ""asd123"" or ETag: "W/"asd123"", kinda undefined behavior in the
// spec. Some servers will accept these while others don't.
return false
}

// ETag: "asd123"
return true
}

if (etag.startsWith('W/"') && etag[etag.length - 1] === '"') {
if (etag.length === 4) {
// ETag: W/"", also where we deviate from the spec & require a min of 3
// chars
return false
}

// ETag: for W/"", W/"asd123"
return true
}

// Anything else
return false
}

/**
* @param {unknown} store
* @returns {asserts store is import('../../types/cache-interceptor.d.ts').default.CacheStore}
Expand Down Expand Up @@ -244,6 +286,7 @@ module.exports = {
makeCacheKey,
parseCacheControlHeader,
parseVaryHeader,
isEtagValid,
assertCacheMethods,
assertCacheStore
}
29 changes: 27 additions & 2 deletions test/cache-interceptor/utils.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
'use strict'

const { describe, test } = require('node:test')
const { deepStrictEqual } = require('node:assert')
const { parseCacheControlHeader, parseVaryHeader } = require('../../lib/util/cache')
const { deepStrictEqual, equal } = require('node:assert')
const { parseCacheControlHeader, parseVaryHeader, isEtagValid } = require('../../lib/util/cache')

describe('parseCacheControlHeader', () => {
test('all directives are parsed properly when in their correct format', () => {
Expand Down Expand Up @@ -215,3 +215,28 @@ describe('parseVaryHeader', () => {
})
})
})

describe('isEtagValid', () => {
const valuesToTest = {
// Invalid etags
'': false,
asd: false,
'"W/"asd""': false,
'""asd""': false,

// Valid etags
'"asd"': true,
'W/"ads"': true,

// Spec deviations
'""': false,
'W/""': false
}

for (const key in valuesToTest) {
const expectedValue = valuesToTest[key]
test(`\`${key}\` = ${expectedValue}`, () => {
equal(isEtagValid(key), expectedValue)
})
}
})
66 changes: 66 additions & 0 deletions test/interceptors/cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,72 @@ describe('Cache Interceptor', () => {
strictEqual(await response.body.text(), 'asd123')
})

test('revalidates request w/ etag when provided', async (t) => {
let requestsToOrigin = 0

const clock = FakeTimers.install({
shouldClearNativeTimers: true
})

const server = createServer((req, res) => {
res.setHeader('cache-control', 'public, s-maxage=1, stale-while-revalidate=10')
requestsToOrigin++

if (requestsToOrigin > 1) {
equal(req.headers['etag'], '"asd123"')

if (requestsToOrigin === 3) {
res.end('asd123')
} else {
res.statusCode = 304
res.end()
}
} else {
res.setHeader('etag', '"asd123"')
res.end('asd')
}
}).listen(0)

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

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

await once(server, 'listening')

strictEqual(requestsToOrigin, 0)

const request = {
origin: 'localhost',
method: 'GET',
path: '/'
}

// Send initial request. This should reach the origin
let response = await client.request(request)
strictEqual(requestsToOrigin, 1)
strictEqual(await response.body.text(), 'asd')

clock.tick(1500)

// Now we send two more requests. Both of these should reach the origin,
// but now with a conditional header asking if the resource has been
// updated. These need to be ran after the response is stale.
// No update for the second request
response = await client.request(request)
strictEqual(requestsToOrigin, 2)
strictEqual(await response.body.text(), 'asd')

// This should be updated, even though the value isn't expired.
response = await client.request(request)
strictEqual(requestsToOrigin, 3)
strictEqual(await response.body.text(), 'asd123')
})

test('respects cache store\'s isFull property', async () => {
const server = createServer((_, res) => {
res.end('asd')
Expand Down
1 change: 1 addition & 0 deletions types/cache-interceptor.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ declare namespace CacheHandler {
statusCode: number;
statusMessage: string;
rawHeaders: Buffer[];
etag?: string;
/**
* Headers defined by the Vary header and their respective values for
* later comparison
Expand Down

0 comments on commit d914909

Please sign in to comment.