Skip to content

Commit 9eeff6c

Browse files
committed
feat: cache etag support
Signed-off-by: flakey5 <73616808+flakey5@users.noreply.github.com>
1 parent 1779440 commit 9eeff6c

File tree

6 files changed

+152
-6
lines changed

6 files changed

+152
-6
lines changed

lib/handler/cache-handler.js

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@ const util = require('../core/util')
44
const DecoratorHandler = require('../handler/decorator-handler')
55
const {
66
parseCacheControlHeader,
7-
parseVaryHeader
7+
parseVaryHeader,
8+
isEtagUsable
89
} = require('../util/cache')
910

1011
function noop () {}
@@ -135,15 +136,24 @@ class CacheHandler extends DecoratorHandler {
135136
cacheControlDirectives
136137
)
137138

138-
this.#writeStream = this.#store.createWriteStream(this.#cacheKey, {
139+
/**
140+
* @type {import('../../types/cache-interceptor.d.ts').default.CachedResponse}
141+
*/
142+
const value = {
139143
statusCode,
140144
statusMessage,
141145
rawHeaders: strippedHeaders,
142146
vary: varyDirectives,
143147
cachedAt: now,
144148
staleAt,
145149
deleteAt
146-
})
150+
}
151+
152+
if (typeof headers.etag === 'string' && isEtagUsable(headers.etag)) {
153+
value.etag = headers.etag
154+
}
155+
156+
this.#writeStream = this.#store.createWriteStream(this.#cacheKey, value)
147157

148158
if (this.#writeStream) {
149159
const handler = this

lib/interceptor/cache.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,8 @@ module.exports = (opts = {}) => {
151151
...opts,
152152
headers: {
153153
...opts.headers,
154-
'if-modified-since': new Date(result.cachedAt).toUTCString()
154+
'if-modified-since': new Date(result.cachedAt).toUTCString(),
155+
etag: result.etag
155156
}
156157
},
157158
new CacheRevalidationHandler(

lib/util/cache.js

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,48 @@ function parseVaryHeader (varyHeader, headers) {
201201
return output
202202
}
203203

204+
/**
205+
* Note: this deviates from the spec a little. Empty etags ("", W/"") are valid,
206+
* however, including them in cached resposnes serves little to no purpose.
207+
*
208+
* @see https://www.rfc-editor.org/rfc/rfc9110.html#name-etag
209+
*
210+
* @param {string} etag
211+
* @returns {boolean}
212+
*/
213+
function isEtagUsable (etag) {
214+
if (etag.length <= 2) {
215+
// Shortest an etag can be is two chars (just ""). This is where we deviate
216+
// from the spec requiring a min of 3 chars however
217+
return false
218+
}
219+
220+
if (etag[0] === '"' && etag[etag.length - 1] === '"') {
221+
if (etag[1] === '"' || etag.startsWith('"W/')) {
222+
// ETag: ""asd123"" or ETag: "W/"asd123"", kinda undefined behavior in the
223+
// spec. Some servers will accept these while others don't.
224+
return false
225+
}
226+
227+
// ETag: "asd123"
228+
return true
229+
}
230+
231+
if (etag.startsWith('W/"') && etag[etag.length - 1] === '"') {
232+
if (etag.length === 4) {
233+
// ETag: W/"", also where we deviate from the spec & require a min of 3
234+
// chars
235+
return false
236+
}
237+
238+
// ETag: for W/"", W/"asd123"
239+
return true
240+
}
241+
242+
// Anything else
243+
return false
244+
}
245+
204246
/**
205247
* @param {unknown} store
206248
* @returns {asserts store is import('../../types/cache-interceptor.d.ts').default.CacheStore}
@@ -244,6 +286,7 @@ module.exports = {
244286
makeCacheKey,
245287
parseCacheControlHeader,
246288
parseVaryHeader,
289+
isEtagUsable,
247290
assertCacheMethods,
248291
assertCacheStore
249292
}

test/cache-interceptor/utils.js

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
'use strict'
22

33
const { describe, test } = require('node:test')
4-
const { deepStrictEqual } = require('node:assert')
5-
const { parseCacheControlHeader, parseVaryHeader } = require('../../lib/util/cache')
4+
const { deepStrictEqual, equal } = require('node:assert')
5+
const { parseCacheControlHeader, parseVaryHeader, isEtagUsable } = require('../../lib/util/cache')
66

77
describe('parseCacheControlHeader', () => {
88
test('all directives are parsed properly when in their correct format', () => {
@@ -215,3 +215,28 @@ describe('parseVaryHeader', () => {
215215
})
216216
})
217217
})
218+
219+
describe('isEtagUsable', () => {
220+
const valuesToTest = {
221+
// Invalid etags
222+
'': false,
223+
asd: false,
224+
'"W/"asd""': false,
225+
'""asd""': false,
226+
227+
// Valid etags
228+
'"asd"': true,
229+
'W/"ads"': true,
230+
231+
// Spec deviations
232+
'""': false,
233+
'W/""': false
234+
}
235+
236+
for (const key in valuesToTest) {
237+
const expectedValue = valuesToTest[key]
238+
test(`\`${key}\` = ${expectedValue}`, () => {
239+
equal(isEtagUsable(key), expectedValue)
240+
})
241+
}
242+
})

test/interceptors/cache.js

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,72 @@ describe('Cache Interceptor', () => {
220220
strictEqual(await response.body.text(), 'asd123')
221221
})
222222

223+
test('revalidates request w/ etag when provided', async (t) => {
224+
let requestsToOrigin = 0
225+
226+
const clock = FakeTimers.install({
227+
shouldClearNativeTimers: true
228+
})
229+
230+
const server = createServer((req, res) => {
231+
res.setHeader('cache-control', 'public, s-maxage=1, stale-while-revalidate=10')
232+
requestsToOrigin++
233+
234+
if (requestsToOrigin > 1) {
235+
equal(req.headers['etag'], '"asd123"')
236+
237+
if (requestsToOrigin === 3) {
238+
res.end('asd123')
239+
} else {
240+
res.statusCode = 304
241+
res.end()
242+
}
243+
} else {
244+
res.setHeader('etag', '"asd123"')
245+
res.end('asd')
246+
}
247+
}).listen(0)
248+
249+
const client = new Client(`http://localhost:${server.address().port}`)
250+
.compose(interceptors.cache())
251+
252+
after(async () => {
253+
server.close()
254+
await client.close()
255+
clock.uninstall()
256+
})
257+
258+
await once(server, 'listening')
259+
260+
strictEqual(requestsToOrigin, 0)
261+
262+
const request = {
263+
origin: 'localhost',
264+
method: 'GET',
265+
path: '/'
266+
}
267+
268+
// Send initial request. This should reach the origin
269+
let response = await client.request(request)
270+
strictEqual(requestsToOrigin, 1)
271+
strictEqual(await response.body.text(), 'asd')
272+
273+
clock.tick(1500)
274+
275+
// Now we send two more requests. Both of these should reach the origin,
276+
// but now with a conditional header asking if the resource has been
277+
// updated. These need to be ran after the response is stale.
278+
// No update for the second request
279+
response = await client.request(request)
280+
strictEqual(requestsToOrigin, 2)
281+
strictEqual(await response.body.text(), 'asd')
282+
283+
// This should be updated, even though the value isn't expired.
284+
response = await client.request(request)
285+
strictEqual(requestsToOrigin, 3)
286+
strictEqual(await response.body.text(), 'asd123')
287+
})
288+
223289
test('respects cache store\'s isFull property', async () => {
224290
const server = createServer((_, res) => {
225291
res.end('asd')

types/cache-interceptor.d.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ declare namespace CacheHandler {
5353
statusCode: number;
5454
statusMessage: string;
5555
rawHeaders: Buffer[];
56+
etag?: string;
5657
/**
5758
* Headers defined by the Vary header and their respective values for
5859
* later comparison

0 commit comments

Comments
 (0)