From 22db73f461633ffd365ea2e42ff972c83d6cf6c8 Mon Sep 17 00:00:00 2001 From: Taku Amano Date: Mon, 4 Nov 2024 08:53:56 +0900 Subject: [PATCH] fix(middleware/etag): generate etag hash value from all chunks (#3604) * fix(middleware/etag): generate etag hash value from all chunks * refactor(middleware/etag): returns immediately if digest cannot be generated. * refactor(utils/crypto): remove ReadableStream support * test(middleware/etag): add tests for empty stream and null body Co-authored-by: Yusuke Wada --------- Co-authored-by: Yusuke Wada --- src/middleware/etag/digest.ts | 42 ++++++++++++++++ src/middleware/etag/index.test.ts | 82 +++++++++++++++++++++++++++++++ src/middleware/etag/index.ts | 7 ++- src/utils/crypto.ts | 11 +---- 4 files changed, 130 insertions(+), 12 deletions(-) create mode 100644 src/middleware/etag/digest.ts diff --git a/src/middleware/etag/digest.ts b/src/middleware/etag/digest.ts new file mode 100644 index 00000000..5af9cacc --- /dev/null +++ b/src/middleware/etag/digest.ts @@ -0,0 +1,42 @@ +const mergeBuffers = (buffer1: ArrayBuffer | undefined, buffer2: Uint8Array): Uint8Array => { + if (!buffer1) { + return buffer2 + } + const merged = new Uint8Array(buffer1.byteLength + buffer2.byteLength) + merged.set(new Uint8Array(buffer1), 0) + merged.set(buffer2, buffer1.byteLength) + return merged +} + +export const generateDigest = async ( + stream: ReadableStream | null +): Promise => { + if (!stream || !crypto || !crypto.subtle) { + return null + } + + let result: ArrayBuffer | undefined = undefined + + const reader = stream.getReader() + for (;;) { + const { value, done } = await reader.read() + if (done) { + break + } + + result = await crypto.subtle.digest( + { + name: 'SHA-1', + }, + mergeBuffers(result, value) + ) + } + + if (!result) { + return null + } + + return Array.prototype.map + .call(new Uint8Array(result), (x) => x.toString(16).padStart(2, '0')) + .join('') +} diff --git a/src/middleware/etag/index.test.ts b/src/middleware/etag/index.test.ts index 1fb9c990..a1c67c60 100644 --- a/src/middleware/etag/index.test.ts +++ b/src/middleware/etag/index.test.ts @@ -65,6 +65,63 @@ describe('Etag Middleware', () => { expect(res.headers.get('ETag')).not.toBe(hash) }) + it('Should not be the same etag - ReadableStream', async () => { + const app = new Hono() + app.use('/etag/*', etag()) + app.get('/etag/rs1', (c) => { + return c.body( + new ReadableStream({ + start(controller) { + controller.enqueue(new Uint8Array([1])) + controller.enqueue(new Uint8Array([2])) + controller.close() + }, + }) + ) + }) + app.get('/etag/rs2', (c) => { + return c.body( + new ReadableStream({ + start(controller) { + controller.enqueue(new Uint8Array([1])) + controller.enqueue(new Uint8Array([3])) + controller.close() + }, + }) + ) + }) + + let res = await app.request('http://localhost/etag/rs1') + const hash = res.headers.get('Etag') + res = await app.request('http://localhost/etag/rs2') + expect(res.headers.get('ETag')).not.toBe(hash) + }) + + it('Should not return etag header when the stream is empty', async () => { + const app = new Hono() + app.use('/etag/*', etag()) + app.get('/etag/abc', (c) => { + const stream = new ReadableStream({ + start(controller) { + controller.close() + }, + }) + return c.body(stream) + }) + const res = await app.request('http://localhost/etag/abc') + expect(res.status).toBe(200) + expect(res.headers.get('ETag')).toBeNull() + }) + + it('Should not return etag header when body is null', async () => { + const app = new Hono() + app.use('/etag/*', etag()) + app.get('/etag/abc', () => new Response(null, { status: 500 })) + const res = await app.request('http://localhost/etag/abc') + expect(res.status).toBe(500) + expect(res.headers.get('ETag')).toBeNull() + }) + it('Should return etag header - weak', async () => { const app = new Hono() app.use('/etag/*', etag({ weak: true })) @@ -179,4 +236,29 @@ describe('Etag Middleware', () => { expect(res.headers.get('x-message-retain')).toBe(message) expect(res.headers.get('x-message')).toBeFalsy() }) + + describe('When crypto is not available', () => { + let _crypto: Crypto | undefined + beforeAll(() => { + _crypto = globalThis.crypto + Object.defineProperty(globalThis, 'crypto', { + value: {}, + }) + }) + + afterAll(() => { + Object.defineProperty(globalThis, 'crypto', { + value: _crypto, + }) + }) + + it('Should not generate etag', async () => { + const app = new Hono() + app.use('/etag/*', etag()) + app.get('/etag/no-digest', (c) => c.text('Hono is cool')) + const res = await app.request('/etag/no-digest') + expect(res.status).toBe(200) + expect(res.headers.get('ETag')).toBeNull() + }) + }) }) diff --git a/src/middleware/etag/index.ts b/src/middleware/etag/index.ts index 84ab7044..f8555583 100644 --- a/src/middleware/etag/index.ts +++ b/src/middleware/etag/index.ts @@ -4,7 +4,7 @@ */ import type { MiddlewareHandler } from '../../types' -import { sha1 } from '../../utils/crypto' +import { generateDigest } from './digest' type ETagOptions = { retainedHeaders?: string[] @@ -63,7 +63,10 @@ export const etag = (options?: ETagOptions): MiddlewareHandler => { let etag = res.headers.get('ETag') if (!etag) { - const hash = await sha1(res.clone().body || '') + const hash = await generateDigest(res.clone().body) + if (hash === null) { + return + } etag = weak ? `W/"${hash}"` : `"${hash}"` } diff --git a/src/utils/crypto.ts b/src/utils/crypto.ts index 3e3ad6bd..9ffb8310 100644 --- a/src/utils/crypto.ts +++ b/src/utils/crypto.ts @@ -8,7 +8,7 @@ type Algorithm = { alias: string } -type Data = string | boolean | number | object | ArrayBufferView | ArrayBuffer | ReadableStream +type Data = string | boolean | number | object | ArrayBufferView | ArrayBuffer export const sha256 = async (data: Data): Promise => { const algorithm: Algorithm = { name: 'SHA-256', alias: 'sha256' } @@ -31,15 +31,6 @@ export const md5 = async (data: Data): Promise => { export const createHash = async (data: Data, algorithm: Algorithm): Promise => { let sourceBuffer: ArrayBufferView | ArrayBuffer - if (data instanceof ReadableStream) { - let body = '' - const reader = data.getReader() - await reader?.read().then(async (chuck) => { - const value = await createHash(chuck.value || '', algorithm) - body += value - }) - return body - } if (ArrayBuffer.isView(data) || data instanceof ArrayBuffer) { sourceBuffer = data } else {