unjs / ipx

🖼️ High performance, secure and easy-to-use image optimizer.
MIT License
1.87k stars 63 forks source link

ipx v2 returns `204 No Content` instead of `304 Not Modified` #178

Closed aaharu closed 11 months ago

aaharu commented 1 year ago

Environment

ipx v2.0.0-0, v2.0.0-1

Reproduction

minimum h3 app

import { createServer } from 'node:http'
import { createApp, eventHandler, toNodeListener, setResponseStatus } from 'h3'

const app = createApp()
app.use(
  '/',
  eventHandler((event) => {
    setResponseStatus(event, 304)
    return null
  })
)

createServer(toNodeListener(app)).listen(process.env.PORT || 8000)

GET / HTTP/1.1 Host: localhost:8000 Accept: /

< HTTP/1.1 204 No Content < Date: Thu, 05 Oct 2023 13:21:53 GMT < Connection: keep-alive < Keep-Alive: timeout=5

Describe the bug

ipx v2 sets the HTTP status code to 304 in the following two places. https://github.com/unjs/ipx/blob/v2.0.0-1/src/server.ts#L89-L90 https://github.com/unjs/ipx/blob/v2.0.0-1/src/server.ts#L109-L110

However, h3 determines that it is 204, because they return null. https://github.com/unjs/h3/blob/v1.8.2/src/app.ts#L211-L215

Additional context

No response

Logs

No response

EnzoAlbornoz commented 12 months ago

The same is happening to me. I needed to patch (through pnpm patch) the cited lines. So instead of setting the status code, I managed to return sendNoContent(event, 304)

pi0 commented 11 months ago

Thanks for report. It should be fixed via 06820b5ae02bae3403ed233e45bf66dd8d1726ed in next (v2) release.