-
Notifications
You must be signed in to change notification settings - Fork 594
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
[http-cache]: handle cache invalidation of location and content-location headers #3735
Closed
Closed
Changes from 11 commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
c65844d
followup http-cache
Uzlopak 10755d3
fix methods check
Uzlopak 9a8b0b4
avoid instantiation in case of error
Uzlopak bc5cfe7
improve
Uzlopak 310c2e5
rename assertCacheStoreType to assertCacheStore
Uzlopak a5fd832
remove validation
Uzlopak 85727cf
remove fallback
Uzlopak bff7c42
fix
Uzlopak 6a099bb
fix
Uzlopak 8ae39f0
[http-cache] follow up 2
Uzlopak b89a32f
handle cache invalidation of location and content-location headers
Uzlopak 4456152
Merge branch 'main' into followup-http-cache-2
Uzlopak File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,53 +2,54 @@ | |
|
||
const util = require('../core/util') | ||
const DecoratorHandler = require('../handler/decorator-handler') | ||
const { parseCacheControlHeader, parseVaryHeader, UNSAFE_METHODS, assertCacheStoreType } = require('../util/cache') | ||
const { | ||
parseCacheControlHeader, | ||
parseVaryHeader | ||
} = require('../util/cache') | ||
const { sameOrigin } = require('../web/fetch/util.js') | ||
|
||
const noop = () => {} | ||
|
||
/** | ||
* Writes a response to a CacheStore and then passes it on to the next handler | ||
*/ | ||
class CacheHandler extends DecoratorHandler { | ||
/** | ||
* @type {import('../../types/cache-interceptor.d.ts').default.CacheOptions | null} | ||
*/ | ||
#opts = null | ||
* @type {import('../../types/cache-interceptor.d.ts').default.CacheStore} | ||
*/ | ||
#store | ||
|
||
/** | ||
* @type {import('../../types/cache-interceptor.d.ts').default.CacheMethods} | ||
*/ | ||
#methods | ||
|
||
/** | ||
* @type {import('../../types/dispatcher.d.ts').default.RequestOptions | null} | ||
* @type {import('../../types/dispatcher.d.ts').default.RequestOptions} | ||
*/ | ||
#req = null | ||
#requestOptions | ||
|
||
/** | ||
* @type {import('../../types/dispatcher.d.ts').default.DispatchHandlers | null} | ||
* @type {import('../../types/dispatcher.d.ts').default.DispatchHandlers} | ||
*/ | ||
#handler = null | ||
#handler | ||
|
||
/** | ||
* @type {import('../../types/cache-interceptor.d.ts').default.CacheStoreWriteable | undefined} | ||
*/ | ||
#writeStream | ||
|
||
/** | ||
* @param {import('../../types/cache-interceptor.d.ts').default.CacheOptions} opts | ||
* @param {import('../../types/dispatcher.d.ts').default.RequestOptions} req | ||
* @param {import('../../types/dispatcher.d.ts').default.RequestOptions} requestOptions | ||
* @param {import('../../types/dispatcher.d.ts').default.DispatchHandlers} handler | ||
*/ | ||
constructor (opts, req, handler) { | ||
constructor (opts, requestOptions, handler) { | ||
super(handler) | ||
|
||
if (typeof opts !== 'object') { | ||
throw new TypeError(`expected opts to be an object, got type ${typeof opts}`) | ||
} | ||
|
||
assertCacheStoreType(opts.store) | ||
|
||
if (typeof req !== 'object') { | ||
throw new TypeError(`expected req to be an object, got type ${typeof opts}`) | ||
} | ||
|
||
if (typeof handler !== 'object') { | ||
throw new TypeError(`expected handler to be an object, got type ${typeof opts}`) | ||
} | ||
|
||
this.#opts = opts | ||
this.#req = req | ||
this.#store = opts.store | ||
this.#methods = opts.methods | ||
this.#requestOptions = requestOptions | ||
this.#handler = handler | ||
} | ||
|
||
|
@@ -74,36 +75,60 @@ class CacheHandler extends DecoratorHandler { | |
statusMessage | ||
) | ||
|
||
const headers = util.parseHeaders(rawHeaders) | ||
|
||
// 4.4. Invalidating Stored Responses | ||
// | ||
// https://www.rfc-editor.org/rfc/rfc9111.html#name-invalidating-stored-respons | ||
// Because unsafe request methods (Section 9.2.1 of [HTTP]) such as PUT, | ||
// POST, or DELETE have the potential for changing state on the origin | ||
// server, intervening caches are required to invalidate stored responses | ||
// to keep their contents up to date. | ||
|
||
if ( | ||
UNSAFE_METHODS.includes(this.#req.method) && | ||
statusCode >= 200 && | ||
statusCode <= 399 | ||
// A cache MUST invalidate the target URI (Section 7.1 of [HTTP]) when it | ||
// receives a non-error status code in response to an unsafe request | ||
// method (including methods whose safety is unknown). | ||
!this.#methods.includes(this.#requestOptions.method) && | ||
isNonErrorStatusCode(statusCode) | ||
) { | ||
// https://www.rfc-editor.org/rfc/rfc9111.html#name-invalidating-stored-respons | ||
// Try/catch for if it's synchronous | ||
try { | ||
const result = this.#opts.store.deleteByOrigin(this.#req.origin) | ||
if ( | ||
result && | ||
typeof result.catch === 'function' && | ||
typeof this.#handler.onError === 'function' | ||
) { | ||
// Fail silently | ||
result.catch(_ => {}) | ||
const result = this.#store.deleteByOrigin(this.#requestOptions.origin) | ||
// Fail silently if it is a promise/async function | ||
result && result.catch && result.catch(noop) | ||
} catch (err) { /* Fail silently */ } | ||
|
||
// A cache MAY invalidate other URIs when it receives a non-error status code | ||
// in response to an unsafe request method (including methods whose safety is | ||
// unknown). In particular, the URI(s) in the Location and Content-Location | ||
// response header fields (if present) are candidates for invalidation; other | ||
// URIs might be discovered through mechanisms not specified in this document. | ||
// However, a cache MUST NOT trigger an invalidation under these conditions if | ||
// the origin (Section 4.3.1 of [HTTP]) of the URI to be invalidated differs | ||
// from that of the target URI (Section 7.1 of [HTTP]). This helps prevent | ||
// denial-of-service attacks. | ||
['location', 'content-location'].forEach((headerName) => { | ||
if (headers[headerName]) { | ||
const targetURL = new URL(headers[headerName], this.#requestOptions.origin) | ||
if (sameOrigin(targetURL, this.#requestOptions.url)) { | ||
// Try/catch for if it's synchronous | ||
try { | ||
const result = this.#store.deleteByOriginAndPath(this.#requestOptions.origin, targetURL.pathname) | ||
// Fail silently if it is a promise/async function | ||
result && result.catch && result.catch(noop) | ||
} catch (err) { /* Fail silently */ } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this should be awaited if it's a promise. |
||
} | ||
} | ||
} catch (err) { | ||
// Fail silently | ||
} | ||
}) | ||
|
||
return downstreamOnHeaders() | ||
} | ||
|
||
const headers = util.parseHeaders(rawHeaders) | ||
|
||
const cacheControlHeader = headers['cache-control'] | ||
const contentLengthHeader = headers['content-length'] | ||
|
||
if (!cacheControlHeader || !contentLengthHeader || this.#opts.store.isFull) { | ||
if (!cacheControlHeader || !contentLengthHeader || this.#store.isFull) { | ||
// Don't have the headers we need, can't cache | ||
return downstreamOnHeaders() | ||
} | ||
|
@@ -122,7 +147,7 @@ class CacheHandler extends DecoratorHandler { | |
const staleAt = determineStaleAt(now, headers, cacheControlDirectives) | ||
if (staleAt) { | ||
const varyDirectives = headers.vary | ||
? parseVaryHeader(headers.vary, this.#req.headers) | ||
? parseVaryHeader(headers.vary, this.#requestOptions.headers) | ||
: undefined | ||
const deleteAt = determineDeleteAt(now, cacheControlDirectives, staleAt) | ||
|
||
|
@@ -132,7 +157,7 @@ class CacheHandler extends DecoratorHandler { | |
cacheControlDirectives | ||
) | ||
|
||
this.#writeStream = this.#opts.store.createWriteStream(this.#req, { | ||
this.#writeStream = this.#store.createWriteStream(this.#requestOptions, { | ||
statusCode, | ||
statusMessage, | ||
rawHeaders: strippedHeaders, | ||
|
@@ -362,4 +387,9 @@ function stripNecessaryHeaders (rawHeaders, parsedHeaders, cacheControlDirective | |
return strippedHeaders ? util.encodeHeaders(strippedHeaders) : rawHeaders | ||
} | ||
|
||
function isNonErrorStatusCode (statusCode) { | ||
// A "non-error response" is one with a 2xx (Successful) or 3xx (Redirection) status code | ||
return statusCode >= 200 && statusCode <= 399 | ||
} | ||
|
||
module.exports = CacheHandler |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would split the
key
here by the:
because theMemoryCacheStore
indeed stores keys in a formatpath:method
, but this might be not true for other types of stores. So if someone will rely on this it might be a breaking change in the future.