From 5af09eeb1937e885e820b8fe58a3a7f906972966 Mon Sep 17 00:00:00 2001 From: Steven Date: Mon, 9 Nov 2020 10:45:59 -0500 Subject: [PATCH 1/6] Add etag header to optimized images --- packages/next/next-server/server/api-utils.ts | 20 +-------- .../next-server/server/image-optimizer.ts | 43 ++++++++++++------- .../next/next-server/server/send-payload.ts | 30 +++++++++---- 3 files changed, 49 insertions(+), 44 deletions(-) diff --git a/packages/next/next-server/server/api-utils.ts b/packages/next/next-server/server/api-utils.ts index e4e3a1e44ef25..9ffb544a9c141 100644 --- a/packages/next/next-server/server/api-utils.ts +++ b/packages/next/next-server/server/api-utils.ts @@ -1,8 +1,6 @@ import { IncomingMessage, ServerResponse } from 'http' import { parse } from 'next/dist/compiled/content-type' import { CookieSerializeOptions } from 'next/dist/compiled/cookie' -import generateETag from 'etag' -import fresh from 'next/dist/compiled/fresh' import getRawBody from 'raw-body' import { PageConfig } from 'next/types' import { Stream } from 'stream' @@ -10,6 +8,7 @@ import { isResSent, NextApiRequest, NextApiResponse } from '../lib/utils' import { decryptWithSecret, encryptWithSecret } from './crypto-utils' import { interopDefault } from './load-components' import { Params } from './router' +import { sendEtagResponse } from './send-payload' export type NextApiRequestCookies = { [key: string]: string } export type NextApiRequestQuery = { [key: string]: string | string[] } @@ -216,23 +215,6 @@ export function redirect( return res } -function sendEtagResponse( - req: NextApiRequest, - res: NextApiResponse, - body: string | Buffer -): boolean { - const etag = generateETag(body) - - if (fresh(req.headers, { etag })) { - res.statusCode = 304 - res.end() - return true - } - - res.setHeader('ETag', etag) - return false -} - /** * Send `any` body to response * @param req request object diff --git a/packages/next/next-server/server/image-optimizer.ts b/packages/next/next-server/server/image-optimizer.ts index e6f8d05936273..528df6599189a 100644 --- a/packages/next/next-server/server/image-optimizer.ts +++ b/packages/next/next-server/server/image-optimizer.ts @@ -2,7 +2,7 @@ import nodeUrl, { UrlWithParsedQuery } from 'url' import { IncomingMessage, ServerResponse } from 'http' import { join } from 'path' import { mediaType } from '@hapi/accept' -import { createReadStream, promises } from 'fs' +import { promises } from 'fs' import { createHash } from 'crypto' import Server from './next-server' import { getContentType, getExtension } from './serve-static' @@ -10,6 +10,7 @@ import { fileExists } from '../../lib/file-exists' // @ts-ignore no types for is-animated import isAnimated from 'next/dist/compiled/is-animated' import Stream from 'stream' +import { sendEtagResponse } from './send-payload' let sharp: typeof import('sharp') //const AVIF = 'image/avif' @@ -146,14 +147,16 @@ export async function imageOptimizer( const [filename, extension] = file.split('.') const expireAt = Number(filename) const contentType = getContentType(extension) + const fsPath = join(hashDir, file) if (now < expireAt) { if (contentType) { res.setHeader('Content-Type', contentType) } - createReadStream(join(hashDir, file)).pipe(res) + const buffer = await promises.readFile(fsPath) + sendResponse(req, res, contentType, buffer) return { finished: true } } else { - await promises.unlink(join(hashDir, file)) + await promises.unlink(fsPath) } } } @@ -223,8 +226,7 @@ export async function imageOptimizer( const animate = ANIMATABLE_TYPES.includes(upstreamType) && isAnimated(upstreamBuffer) if (vector || animate) { - res.setHeader('Content-Type', upstreamType) - res.end(upstreamBuffer) + sendResponse(req, res, upstreamType, upstreamBuffer) return { finished: true } } } @@ -248,10 +250,8 @@ export async function imageOptimizer( if (error.code === 'MODULE_NOT_FOUND') { error.message += '\n\nLearn more: https://err.sh/next.js/install-sharp' server.logError(error) - if (upstreamType) { - res.setHeader('Content-Type', upstreamType) - } - res.end(upstreamBuffer) + sendResponse(req, res, upstreamType, upstreamBuffer) + return { finished: true } } throw error } @@ -283,19 +283,30 @@ export async function imageOptimizer( const extension = getExtension(contentType) const filename = join(hashDir, `${expireAt}.${extension}`) await promises.writeFile(filename, optimizedBuffer) - res.setHeader('Content-Type', contentType) - res.end(optimizedBuffer) + sendResponse(req, res, contentType, optimizedBuffer) } catch (error) { - server.logError(error) - if (upstreamType) { - res.setHeader('Content-Type', upstreamType) - } - res.end(upstreamBuffer) + sendResponse(req, res, upstreamType, upstreamBuffer) } return { finished: true } } +function sendResponse( + req: IncomingMessage, + res: ServerResponse, + contentType: string | null, + buffer: Buffer +) { + if (contentType) { + res.setHeader('Content-Type', contentType) + } + if (sendEtagResponse(req, res, buffer)) { + return + } + res.setHeader('Cache-Control', 'public, max-age=0, must-revalidate') + res.end(buffer) +} + function getSupportedMimeType(options: string[], accept = ''): string { const mimeType = mediaType(accept, options) return accept.includes(mimeType) ? mimeType : '' diff --git a/packages/next/next-server/server/send-payload.ts b/packages/next/next-server/server/send-payload.ts index c5dd4d7f449ac..c90dbf541f3b8 100644 --- a/packages/next/next-server/server/send-payload.ts +++ b/packages/next/next-server/server/send-payload.ts @@ -25,18 +25,10 @@ export function sendPayload( res.setHeader('X-Powered-By', 'Next.js') } - const etag = generateEtags ? generateETag(payload) : undefined - - if (fresh(req.headers, { etag })) { - res.statusCode = 304 - res.end() + if (sendEtagResponse(req, res, payload, generateEtags)) { return } - if (etag) { - res.setHeader('ETag', etag) - } - if (!res.getHeader('Content-Type')) { res.setHeader( 'Content-Type', @@ -72,3 +64,23 @@ export function sendPayload( } res.end(req.method === 'HEAD' ? null : payload) } + +export function sendEtagResponse( + req: IncomingMessage, + res: ServerResponse, + body: string | Buffer, + generate = true +): boolean { + const etag = generate ? generateETag(body) : undefined + + if (fresh(req.headers, { etag })) { + res.statusCode = 304 + res.end() + return true + } + + if (etag) { + res.setHeader('ETag', etag) + } + return false +} From f1738137c017bc45981ceb6d2e10a0018b9f93bf Mon Sep 17 00:00:00 2001 From: Steven Date: Mon, 9 Nov 2020 11:41:42 -0500 Subject: [PATCH 2/6] Add etag to file name --- .../next-server/server/image-optimizer.ts | 30 +++++++++++++------ 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/packages/next/next-server/server/image-optimizer.ts b/packages/next/next-server/server/image-optimizer.ts index 528df6599189a..5492efbffbc45 100644 --- a/packages/next/next-server/server/image-optimizer.ts +++ b/packages/next/next-server/server/image-optimizer.ts @@ -2,7 +2,7 @@ import nodeUrl, { UrlWithParsedQuery } from 'url' import { IncomingMessage, ServerResponse } from 'http' import { join } from 'path' import { mediaType } from '@hapi/accept' -import { promises } from 'fs' +import { createReadStream, promises } from 'fs' import { createHash } from 'crypto' import Server from './next-server' import { getContentType, getExtension } from './serve-static' @@ -11,6 +11,7 @@ import { fileExists } from '../../lib/file-exists' import isAnimated from 'next/dist/compiled/is-animated' import Stream from 'stream' import { sendEtagResponse } from './send-payload' +import fresh from 'next/dist/compiled/fresh' let sharp: typeof import('sharp') //const AVIF = 'image/avif' @@ -19,7 +20,7 @@ const PNG = 'image/png' const JPEG = 'image/jpeg' const GIF = 'image/gif' const SVG = 'image/svg+xml' -const CACHE_VERSION = 1 +const CACHE_VERSION = 2 const MODERN_TYPES = [/* AVIF, */ WEBP] const ANIMATABLE_TYPES = [WEBP, PNG, GIF] const VECTOR_TYPES = [SVG] @@ -144,7 +145,7 @@ export async function imageOptimizer( if (await fileExists(hashDir, 'directory')) { const files = await promises.readdir(hashDir) for (let file of files) { - const [filename, extension] = file.split('.') + const [filename, etag, extension] = file.split('.') const expireAt = Number(filename) const contentType = getContentType(extension) const fsPath = join(hashDir, file) @@ -152,8 +153,8 @@ export async function imageOptimizer( if (contentType) { res.setHeader('Content-Type', contentType) } - const buffer = await promises.readFile(fsPath) - sendResponse(req, res, contentType, buffer) + res.setHeader('Etag', etag) + createReadStream(fsPath).pipe(res) return { finished: true } } else { await promises.unlink(fsPath) @@ -281,9 +282,17 @@ export async function imageOptimizer( const optimizedBuffer = await transformer.toBuffer() await promises.mkdir(hashDir, { recursive: true }) const extension = getExtension(contentType) - const filename = join(hashDir, `${expireAt}.${extension}`) + const etag = getHash([optimizedBuffer]) + const filename = join(hashDir, `${expireAt}.${etag}.${extension}`) await promises.writeFile(filename, optimizedBuffer) - sendResponse(req, res, contentType, optimizedBuffer) + if (fresh(req.headers, { etag })) { + res.statusCode = 304 + res.end() + return true + } + res.setHeader('ETag', etag) + res.setHeader('Content-Type', contentType) + res.end(optimizedBuffer) } catch (error) { sendResponse(req, res, upstreamType, upstreamBuffer) } @@ -312,10 +321,13 @@ function getSupportedMimeType(options: string[], accept = ''): string { return accept.includes(mimeType) ? mimeType : '' } -function getHash(items: (string | number | undefined)[]) { +function getHash(items: (string | number | Buffer)[]) { const hash = createHash('sha256') for (let item of items) { - hash.update(String(item)) + if (typeof item === 'number') hash.update(String(item)) + else { + hash.update(item) + } } // See https://en.wikipedia.org/wiki/Base64#Filenames return hash.digest('base64').replace(/\//g, '-') From fceda1a681f6acbade53392d9fabf58421d71a68 Mon Sep 17 00:00:00 2001 From: Steven Date: Mon, 9 Nov 2020 11:49:17 -0500 Subject: [PATCH 3/6] Add 304 --- .../next/next-server/server/image-optimizer.ts | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/packages/next/next-server/server/image-optimizer.ts b/packages/next/next-server/server/image-optimizer.ts index 5492efbffbc45..574f28bdc4005 100644 --- a/packages/next/next-server/server/image-optimizer.ts +++ b/packages/next/next-server/server/image-optimizer.ts @@ -145,9 +145,17 @@ export async function imageOptimizer( if (await fileExists(hashDir, 'directory')) { const files = await promises.readdir(hashDir) for (let file of files) { - const [filename, etag, extension] = file.split('.') - const expireAt = Number(filename) + const [prefix, etag, extension] = file.split('.') + const expireAt = Number(prefix) const contentType = getContentType(extension) + if (fresh(req.headers, { etag })) { + if (contentType) { + res.setHeader('Content-Type', contentType) + } + res.statusCode = 304 + res.end() + return { finished: true } + } const fsPath = join(hashDir, file) if (now < expireAt) { if (contentType) { @@ -288,7 +296,7 @@ export async function imageOptimizer( if (fresh(req.headers, { etag })) { res.statusCode = 304 res.end() - return true + return { finished: true } } res.setHeader('ETag', etag) res.setHeader('Content-Type', contentType) From cfcd382daeb4145b6a9e67e7b017ba45fc0c1002 Mon Sep 17 00:00:00 2001 From: Steven Date: Mon, 9 Nov 2020 12:00:42 -0500 Subject: [PATCH 4/6] Refactor etag --- packages/next/next-server/server/api-utils.ts | 5 +++-- .../next-server/server/image-optimizer.ts | 20 ++++--------------- .../next/next-server/server/send-payload.ts | 8 +++----- 3 files changed, 10 insertions(+), 23 deletions(-) diff --git a/packages/next/next-server/server/api-utils.ts b/packages/next/next-server/server/api-utils.ts index 9ffb544a9c141..76eb4095c5822 100644 --- a/packages/next/next-server/server/api-utils.ts +++ b/packages/next/next-server/server/api-utils.ts @@ -9,6 +9,7 @@ import { decryptWithSecret, encryptWithSecret } from './crypto-utils' import { interopDefault } from './load-components' import { Params } from './router' import { sendEtagResponse } from './send-payload' +import generateETag from 'etag' export type NextApiRequestCookies = { [key: string]: string } export type NextApiRequestQuery = { [key: string]: string | string[] } @@ -243,8 +244,8 @@ export function sendData( const isJSONLike = ['object', 'number', 'boolean'].includes(typeof body) const stringifiedBody = isJSONLike ? JSON.stringify(body) : body - - if (sendEtagResponse(req, res, stringifiedBody)) { + const etag = generateETag(stringifiedBody) + if (sendEtagResponse(req, res, etag)) { return } diff --git a/packages/next/next-server/server/image-optimizer.ts b/packages/next/next-server/server/image-optimizer.ts index 574f28bdc4005..aee9dde70bf6d 100644 --- a/packages/next/next-server/server/image-optimizer.ts +++ b/packages/next/next-server/server/image-optimizer.ts @@ -11,7 +11,6 @@ import { fileExists } from '../../lib/file-exists' import isAnimated from 'next/dist/compiled/is-animated' import Stream from 'stream' import { sendEtagResponse } from './send-payload' -import fresh from 'next/dist/compiled/fresh' let sharp: typeof import('sharp') //const AVIF = 'image/avif' @@ -148,12 +147,7 @@ export async function imageOptimizer( const [prefix, etag, extension] = file.split('.') const expireAt = Number(prefix) const contentType = getContentType(extension) - if (fresh(req.headers, { etag })) { - if (contentType) { - res.setHeader('Content-Type', contentType) - } - res.statusCode = 304 - res.end() + if (sendEtagResponse(req, res, etag)) { return { finished: true } } const fsPath = join(hashDir, file) @@ -293,14 +287,7 @@ export async function imageOptimizer( const etag = getHash([optimizedBuffer]) const filename = join(hashDir, `${expireAt}.${etag}.${extension}`) await promises.writeFile(filename, optimizedBuffer) - if (fresh(req.headers, { etag })) { - res.statusCode = 304 - res.end() - return { finished: true } - } - res.setHeader('ETag', etag) - res.setHeader('Content-Type', contentType) - res.end(optimizedBuffer) + sendResponse(req, res, contentType, optimizedBuffer) } catch (error) { sendResponse(req, res, upstreamType, upstreamBuffer) } @@ -317,7 +304,8 @@ function sendResponse( if (contentType) { res.setHeader('Content-Type', contentType) } - if (sendEtagResponse(req, res, buffer)) { + const etag = getHash([buffer]) + if (sendEtagResponse(req, res, etag)) { return } res.setHeader('Cache-Control', 'public, max-age=0, must-revalidate') diff --git a/packages/next/next-server/server/send-payload.ts b/packages/next/next-server/server/send-payload.ts index c90dbf541f3b8..35c0f3b057ba9 100644 --- a/packages/next/next-server/server/send-payload.ts +++ b/packages/next/next-server/server/send-payload.ts @@ -25,7 +25,8 @@ export function sendPayload( res.setHeader('X-Powered-By', 'Next.js') } - if (sendEtagResponse(req, res, payload, generateEtags)) { + const etag = generateEtags ? generateETag(payload) : undefined + if (sendEtagResponse(req, res, etag)) { return } @@ -68,11 +69,8 @@ export function sendPayload( export function sendEtagResponse( req: IncomingMessage, res: ServerResponse, - body: string | Buffer, - generate = true + etag: string | undefined ): boolean { - const etag = generate ? generateETag(body) : undefined - if (fresh(req.headers, { etag })) { res.statusCode = 304 res.end() From 7224b569e1ed30e69e84fb5779019af2cd879468 Mon Sep 17 00:00:00 2001 From: Steven Date: Mon, 9 Nov 2020 13:49:37 -0500 Subject: [PATCH 5/6] Add tests --- .../next-server/server/image-optimizer.ts | 15 +-- .../next/next-server/server/send-payload.ts | 13 ++- .../image-optimizer/test/index.test.js | 93 +++++++++++++++++++ 3 files changed, 111 insertions(+), 10 deletions(-) diff --git a/packages/next/next-server/server/image-optimizer.ts b/packages/next/next-server/server/image-optimizer.ts index aee9dde70bf6d..7562ad2e74552 100644 --- a/packages/next/next-server/server/image-optimizer.ts +++ b/packages/next/next-server/server/image-optimizer.ts @@ -145,17 +145,18 @@ export async function imageOptimizer( const files = await promises.readdir(hashDir) for (let file of files) { const [prefix, etag, extension] = file.split('.') - const expireAt = Number(prefix) - const contentType = getContentType(extension) if (sendEtagResponse(req, res, etag)) { return { finished: true } } + const expireAt = Number(prefix) + const contentType = getContentType(extension) const fsPath = join(hashDir, file) if (now < expireAt) { + res.setHeader('Cache-Control', 'public, max-age=0, must-revalidate') + res.setHeader('Etag', etag) if (contentType) { res.setHeader('Content-Type', contentType) } - res.setHeader('Etag', etag) createReadStream(fsPath).pipe(res) return { finished: true } } else { @@ -301,14 +302,14 @@ function sendResponse( contentType: string | null, buffer: Buffer ) { - if (contentType) { - res.setHeader('Content-Type', contentType) - } const etag = getHash([buffer]) + res.setHeader('Cache-Control', 'public, max-age=0, must-revalidate') if (sendEtagResponse(req, res, etag)) { return } - res.setHeader('Cache-Control', 'public, max-age=0, must-revalidate') + if (contentType) { + res.setHeader('Content-Type', contentType) + } res.end(buffer) } diff --git a/packages/next/next-server/server/send-payload.ts b/packages/next/next-server/server/send-payload.ts index 35c0f3b057ba9..a9868b42b99cd 100644 --- a/packages/next/next-server/server/send-payload.ts +++ b/packages/next/next-server/server/send-payload.ts @@ -71,14 +71,21 @@ export function sendEtagResponse( res: ServerResponse, etag: string | undefined ): boolean { + if (etag) { + /** + * The server generating a 304 response MUST generate any of the + * following header fields that would have been sent in a 200 (OK) + * response to the same request: Cache-Control, Content-Location, Date, + * ETag, Expires, and Vary. https://tools.ietf.org/html/rfc7232#section-4.1 + */ + res.setHeader('ETag', etag) + } + if (fresh(req.headers, { etag })) { res.statusCode = 304 res.end() return true } - if (etag) { - res.setHeader('ETag', etag) - } return false } diff --git a/test/integration/image-optimizer/test/index.test.js b/test/integration/image-optimizer/test/index.test.js index 3eff52b1d8c86..957af45b3527d 100644 --- a/test/integration/image-optimizer/test/index.test.js +++ b/test/integration/image-optimizer/test/index.test.js @@ -55,6 +55,10 @@ function runTests({ w, isDev, domains }) { const res = await fetchViaHTTP(appPort, '/_next/image', query, {}) expect(res.status).toBe(200) expect(res.headers.get('content-type')).toContain('image/gif') + expect(res.headers.get('cache-control')).toBe( + 'public, max-age=0, must-revalidate' + ) + expect(res.headers.get('etag')).toBeTruthy() expect(isAnimated(await res.buffer())).toBe(true) }) @@ -63,6 +67,10 @@ function runTests({ w, isDev, domains }) { const res = await fetchViaHTTP(appPort, '/_next/image', query, {}) expect(res.status).toBe(200) expect(res.headers.get('content-type')).toContain('image/png') + expect(res.headers.get('cache-control')).toBe( + 'public, max-age=0, must-revalidate' + ) + expect(res.headers.get('etag')).toBeTruthy() expect(isAnimated(await res.buffer())).toBe(true) }) @@ -71,6 +79,10 @@ function runTests({ w, isDev, domains }) { const res = await fetchViaHTTP(appPort, '/_next/image', query, {}) expect(res.status).toBe(200) expect(res.headers.get('content-type')).toContain('image/webp') + expect(res.headers.get('cache-control')).toBe( + 'public, max-age=0, must-revalidate' + ) + expect(res.headers.get('etag')).toBeTruthy() expect(isAnimated(await res.buffer())).toBe(true) }) @@ -80,6 +92,10 @@ function runTests({ w, isDev, domains }) { const res = await fetchViaHTTP(appPort, '/_next/image', query, opts) expect(res.status).toBe(200) expect(res.headers.get('Content-Type')).toContain('image/svg+xml') + expect(res.headers.get('cache-control')).toBe( + 'public, max-age=0, must-revalidate' + ) + expect(res.headers.get('etag')).toBeTruthy() const actual = await res.text() const expected = await fs.readFile( join(__dirname, '..', 'public', 'test.svg'), @@ -96,6 +112,10 @@ function runTests({ w, isDev, domains }) { const res = await fetchViaHTTP(appPort, '/_next/image', query, opts) expect(res.status).toBe(200) expect(res.headers.get('Content-Type')).toContain('image/jpeg') + expect(res.headers.get('cache-control')).toBe( + 'public, max-age=0, must-revalidate' + ) + expect(res.headers.get('etag')).toBeTruthy() }) it('should maintain png format for old Safari', async () => { @@ -106,6 +126,10 @@ function runTests({ w, isDev, domains }) { const res = await fetchViaHTTP(appPort, '/_next/image', query, opts) expect(res.status).toBe(200) expect(res.headers.get('Content-Type')).toContain('image/png') + expect(res.headers.get('cache-control')).toBe( + 'public, max-age=0, must-revalidate' + ) + expect(res.headers.get('etag')).toBeTruthy() }) it('should fail when url is missing', async () => { @@ -199,6 +223,10 @@ function runTests({ w, isDev, domains }) { const res = await fetchViaHTTP(appPort, '/_next/image', query, opts) expect(res.status).toBe(200) expect(res.headers.get('Content-Type')).toBe('image/webp') + expect(res.headers.get('cache-control')).toBe( + 'public, max-age=0, must-revalidate' + ) + expect(res.headers.get('etag')).toBeTruthy() await expectWidth(res, w) }) @@ -208,6 +236,10 @@ function runTests({ w, isDev, domains }) { const res = await fetchViaHTTP(appPort, '/_next/image', query, opts) expect(res.status).toBe(200) expect(res.headers.get('Content-Type')).toBe('image/png') + expect(res.headers.get('cache-control')).toBe( + 'public, max-age=0, must-revalidate' + ) + expect(res.headers.get('etag')).toBeTruthy() await expectWidth(res, w) }) @@ -217,6 +249,10 @@ function runTests({ w, isDev, domains }) { const res = await fetchViaHTTP(appPort, '/_next/image', query, opts) expect(res.status).toBe(200) expect(res.headers.get('Content-Type')).toBe('image/png') + expect(res.headers.get('cache-control')).toBe( + 'public, max-age=0, must-revalidate' + ) + expect(res.headers.get('etag')).toBeTruthy() await expectWidth(res, w) }) @@ -226,6 +262,10 @@ function runTests({ w, isDev, domains }) { const res = await fetchViaHTTP(appPort, '/_next/image', query, opts) expect(res.status).toBe(200) expect(res.headers.get('Content-Type')).toBe('image/gif') + expect(res.headers.get('cache-control')).toBe( + 'public, max-age=0, must-revalidate' + ) + expect(res.headers.get('etag')).toBeTruthy() await expectWidth(res, w) }) @@ -235,6 +275,10 @@ function runTests({ w, isDev, domains }) { const res = await fetchViaHTTP(appPort, '/_next/image', query, opts) expect(res.status).toBe(200) expect(res.headers.get('Content-Type')).toBe('image/tiff') + expect(res.headers.get('cache-control')).toBe( + 'public, max-age=0, must-revalidate' + ) + expect(res.headers.get('etag')).toBeTruthy() await expectWidth(res, w) }) @@ -246,6 +290,10 @@ function runTests({ w, isDev, domains }) { const res = await fetchViaHTTP(appPort, '/_next/image', query, opts) expect(res.status).toBe(200) expect(res.headers.get('Content-Type')).toBe('image/webp') + expect(res.headers.get('cache-control')).toBe( + 'public, max-age=0, must-revalidate' + ) + expect(res.headers.get('etag')).toBeTruthy() await expectWidth(res, w) }) @@ -257,6 +305,10 @@ function runTests({ w, isDev, domains }) { const res = await fetchViaHTTP(appPort, '/_next/image', query, opts) expect(res.status).toBe(200) expect(res.headers.get('Content-Type')).toBe('image/webp') + expect(res.headers.get('cache-control')).toBe( + 'public, max-age=0, must-revalidate' + ) + expect(res.headers.get('etag')).toBeTruthy() await expectWidth(res, w) }) } @@ -310,6 +362,39 @@ function runTests({ w, isDev, domains }) { expect(json2).toStrictEqual(json1) }) + it('should set 304 status without body when etag matches if-none-match', async () => { + const query = { url: '/test.jpg', w, q: 80 } + const opts1 = { headers: { accept: 'image/webp' } } + + const res1 = await fetchViaHTTP(appPort, '/_next/image', query, opts1) + expect(res1.status).toBe(200) + expect(res1.headers.get('Content-Type')).toBe('image/webp') + expect(res1.headers.get('Cache-Control')).toBe( + 'public, max-age=0, must-revalidate' + ) + const etag = res1.headers.get('Etag') + expect(etag).toBeTruthy() + await expectWidth(res1, w) + + const opts2 = { headers: { accept: 'image/webp', 'if-none-match': etag } } + const res2 = await fetchViaHTTP(appPort, '/_next/image', query, opts2) + expect(res2.status).toBe(304) + expect(res2.headers.get('Content-Type')).toBeFalsy() + expect(res2.headers.get('Etag')).toBe(etag) + expect((await res2.buffer()).length).toBe(0) + + const query3 = { url: '/test.jpg', w, q: 25 } + const res3 = await fetchViaHTTP(appPort, '/_next/image', query3, opts2) + expect(res3.status).toBe(200) + expect(res3.headers.get('Content-Type')).toBe('image/webp') + expect(res3.headers.get('Cache-Control')).toBe( + 'public, max-age=0, must-revalidate' + ) + expect(res3.headers.get('Etag')).toBeTruthy() + expect(res3.headers.get('Etag')).not.toBe(etag) + await expectWidth(res3, w) + }) + it('should proxy-pass unsupported image types and should not cache file', async () => { const json1 = await fsToJson(imagesDir) expect(json1).toBeTruthy() @@ -319,6 +404,10 @@ function runTests({ w, isDev, domains }) { const res = await fetchViaHTTP(appPort, '/_next/image', query, opts) expect(res.status).toBe(200) expect(res.headers.get('Content-Type')).toBe('image/bmp') + expect(res.headers.get('cache-control')).toBe( + 'public, max-age=0, must-revalidate' + ) + expect(res.headers.get('etag')).toBeTruthy() const json2 = await fsToJson(imagesDir) expect(json2).toStrictEqual(json1) @@ -330,6 +419,10 @@ function runTests({ w, isDev, domains }) { const res = await fetchViaHTTP(appPort, '/_next/image', query, opts) expect(res.status).toBe(200) expect(res.headers.get('Content-Type')).toBe('image/webp') + expect(res.headers.get('cache-control')).toBe( + 'public, max-age=0, must-revalidate' + ) + expect(res.headers.get('etag')).toBeTruthy() await expectWidth(res, 400) }) } From eaca4cd51e210af9de570a80327b0eb4f2133649 Mon Sep 17 00:00:00 2001 From: Steven Date: Mon, 9 Nov 2020 15:15:38 -0500 Subject: [PATCH 6/6] The etag should respect upstream TTL --- packages/next/next-server/server/image-optimizer.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/next/next-server/server/image-optimizer.ts b/packages/next/next-server/server/image-optimizer.ts index 7562ad2e74552..8832cb542e861 100644 --- a/packages/next/next-server/server/image-optimizer.ts +++ b/packages/next/next-server/server/image-optimizer.ts @@ -145,15 +145,14 @@ export async function imageOptimizer( const files = await promises.readdir(hashDir) for (let file of files) { const [prefix, etag, extension] = file.split('.') - if (sendEtagResponse(req, res, etag)) { - return { finished: true } - } const expireAt = Number(prefix) const contentType = getContentType(extension) const fsPath = join(hashDir, file) if (now < expireAt) { res.setHeader('Cache-Control', 'public, max-age=0, must-revalidate') - res.setHeader('Etag', etag) + if (sendEtagResponse(req, res, etag)) { + return { finished: true } + } if (contentType) { res.setHeader('Content-Type', contentType) }