serveStatic w/ compress middleware issue

[luwes]

In my testing the compress middleware made a serveStatic HTML page return plain text text/plain;charset=UTF-8.
https://hono.dev/middleware/builtin/compress

[sebastianwessel]

I can confirm, that there is an issue with the response content-type header, as soon as compression is enabled.
It also happens when you simply send a JSON response.

Minimal example:

import { serve } from '@hono/node-server'
import { Hono } from 'hono'
import { compress } from 'hono/compress'

const app = new Hono()

app.use('*', compress())

app.get('/', (c) =>
  c.json({
    some: 'value',
  }),
)

serve({
  fetch: app.fetch,
  port: 3000,
})

response:
content-encoding: gzip
content-type: text/plain;charset=UTF-8

Tried it with node 20.8 and 20.10

The issue seems not to be present when with Deno.
As Bun isn't supporting compress right now, I couldn't test it.

I had a quick look into compress middleware and I didn't recognize something, that might be the root cause.
So, I think the issue is somewhere in the node-server package.

From quick look into the code, I guess the code, the issue relies on this code here:

node-server/src/response.ts

Line 24 in e2648a4

if (typeof body === 'string' || body instanceof ReadableStream) {

and here:

node-server/src/utils.ts

Line 41 in e2648a4

export const buildOutgoingHttpHeaders = (headers: Headers): OutgoingHttpHeaders => {

The mapping from the current headers to the final headers is messed up in compression with:
https://github.com/honojs/hono/blob/75dbd4dc229335d473b809b0f2190e3b4044b9bc/src/middleware/compress/index.ts#L21

I guess the headers simply have different types, but I'm too unfamiliar with hono code base, to provide a propper fix atm.

[yusukebe]

@luwes @sebastianwessel Thank you for raising the issue.

Hi @usualoma, in the fowllowing case, test will fail. Can you see it?

describe('content-type', () => {
  const app = new Hono()
  app.use('*', async (c, next) => {
    await next()
    c.res = new Response('', c.res)
    c.res.headers // If this is set, test fails
  })

  app.get('/json', async (c) => {
    return c.json({})
  })

  it('Should return 200 response - GET /json', async () => {
    const server = createAdaptorServer(app)
    const res = await request(server).get('/json')
    expect(res.status).toBe(200)
    expect(res.headers['content-type']).toMatch(/application\/json/)
  })
})

[usualoma]

It appears to be a bug in the Response object. It can be fixed. Please wait a moment.

[yusukebe]

@usualoma

Thanks!

[yusukebe]

Hi @luwes @sebastianwessel

Thanks to @usualoma , this issue is fixed in latest version v1.3.2. Give it a try!

[sebastianwessel]

I did a quick test, and it seems to work as expected.
Thanks for the quick fix 🙏.

[luwes]

Thank you for the fix!

I tested it out but not all statics are compressed it seems.

Using

  app.use('*', serveStatic({ root: dir }));
  app.use('*', compress());

/favicon.ico is gzipped

/test/test.js is not

test/index.html is not

If I try to reverse the middleware:

  app.use('*', compress());
  app.use('*', serveStatic({ root: dir }));

I get an error

[yusukebe]

It can be reproduced by this minimal code:

app.get('/', () => {
  const stream = new ReadableStream({
    start(controller) {
      const encoder = new TextEncoder()
      controller.enqueue(encoder.encode('hello'))
      controller.close()
    },
  })
  const originalResponse = new Response(stream)
  const compStream = new CompressionStream('gzip')
  const res = new Response(originalResponse.body!.pipeThrough(compStream), originalResponse)
  res.headers.set('Content-Encoding', 'gzip')
  return res
})

@usualoma When you have time, please take a look.

[yusukebe]

@luwes

It's fixed thanks to @usualoma and I've released new version v1.3.3 which includes the fix. Take a look!