honojs / hono

Web framework built on Web Standards
https://hono.dev
MIT License
20.12k stars 574 forks source link

(bug): Unable to read raw request body after hono 3.5.0 #1387

Closed AdiRishi closed 1 year ago

AdiRishi commented 1 year ago

What version of Hono are you using?

3.5.6

What runtime/platform is your app running on?

Cloudflare Workers

What steps can reproduce the bug?

  1. I've made a minimal reproduction repository, please clone it from here - https://github.com/AdiRishi/hono-body-read-bug
  2. Run pnpm install
  3. Run pnpm dev. This will start a http server at port 8787
  4. Make a curl POST request to http://localhost:8787/read-body. Here is a sample request
    curl --location 'http://localhost:8787/read-body' \
    --header 'Content-Type: application/json' \
    --data '{}'

    For ease, here is the code snippet that the /read-body endpoint runs

    app.post('/read-body', zValidator('json', z.object({})), async (c) => {
    const rawBody = await c.req.text(); // the same happens with c.req.raw.text()
    return c.json({ rawText: rawBody });
    })
  5. On the console, you will see the endpoint return a 500 error response rather than 200, triggering the bug.

What is the expected behavior?

The expected behaviour is that this request returns a successful 200 response and succeeds.

NOTE This piece of code worked in hono 3.4.3. From what I can tell this behaviour changed in 3.5.0 and still persists in the latest version. To see a working version of this, I've made a branch with Hono 3.4.3 in the reproduction repository. You can see the exact same code working just fine.

What do you see instead?

TypeError: Body has already been used. It can only be used once. Use tee() first if you need to read it twice.
    at HonoRequest.cachedBody (index.js:596:39)
    at HonoRequest.text (index.js:654:17)
    at index.js:5348:31
    at dispatch (index.js:512:17)
    at index.js:513:33
    at index.js:1684:11
    at async index.js:1591:7
    at async index.js:938:62
    at async jsonError (index.js:5370:12) {
  stack: TypeError: Body has already been used. It can only…:938:62
    at async jsonError (index.js:5370:12),
  message: Body has already been used. It can only be used once. Use tee() first if you need to read it twice.
}

Additional information

Now, this may not be considered a "bug", since I'm sure at some point before we handle the request Hono's zodValidator reads the request body to parse it. However, this leads to it being impossible for us to read the raw request body again in the endpoint definition.

I have a specific use case my code was running before, which broke in 3.5.0 onwards. Specifically, my project had an endpoint that would read the contents of c.req.raw.text() and run it through HMAC hashing, checking the result against a signature header. e.g x-razorpay-signature. This is a common pattern needed to validate requests by payment providers, which often provide an integrity header to check the body against.

In the latest version of Hono, there seems to be no way of doing this due to the exception I've shown above.

yusukebe commented 1 year ago

Hi @AdiRishi !

Sorry for the trouble. I'll fix it later. Thanks.

yusukebe commented 1 year ago

Hi @AdiRishi!

I've released new version v3.5.7, which should address the issue you've encountered. Let me provide some background on what was happening:

Prior to v3.5.0, our validation process involved cloning the Request object:

value = await c.req.raw.clone().json()

But, this approach raised warnings in the Cloudflare environment if the cloned object wasn't utilized. Related https://github.com/honojs/middleware/issues/62.

As a solution, v3.5.0 was designed to eliminate the need to clone the Response. Instead, it caches the body content for reuse. Related https://github.com/honojs/hono/pull/1333.

Unfortunately, this implementation had its downsides. Specifically, it didn't properly handle scenarios where one needed to execute c.req.text() after validating a JSON object, leading to the issue you've experienced.

The latest PR, #1393 , included in the new release, rectifies this by caching the body content as arrayBuffer. This allows for the retrieval of cached arrayBuffer and its subsequent conversion to text via c.req.text() after JSON validation. It's vital to note that you should invoke c.req.text() and avoid c.req.raw.text() in this context.

I hope this resolves your concerns. Should you face any further issues, please don't hesitate to reopen the thread. Thanks!

AdiRishi commented 1 year ago

Thank you for the prompt fix yusukebe πŸ˜„

Finally found the time to upgrade hono to 3.5.8 in my project and everything is working great. Your fix works πŸ‘ πŸŽ‰