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 <yusuke@kamawada.com>

---------

Co-authored-by: Yusuke Wada <yusuke@kamawada.com>

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<Uint8Array> | null
+): Promise<string | null> => {
+  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('')
+}

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()
+    })
+  })
 })

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}"`
     }
 

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<string | null> => {
   const algorithm: Algorithm = { name: 'SHA-256', alias: 'sha256' }
@@ -31,15 +31,6 @@ export const md5 = async (data: Data): Promise<string | null> => {
 export const createHash = async (data: Data, algorithm: Algorithm): Promise<string | null> => {
   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 {