grammyjs / grammY

The Telegram Bot Framework.
https://grammy.dev
MIT License
2.28k stars 112 forks source link

Crashing on calling webhook with empty JSON body using Oak #613

Open t3hk0d3 opened 3 months ago

t3hk0d3 commented 3 months ago

If somebody occasionally do an GET request to webhook endpoint it will result in entire server crashing.

error: Uncaught (in promise) BadRequestError: Unexpected end of JSON input
  return new errors[Status[status] as ErrorStatusKeys](message, options);
         ^
    at createHttpError (https://jsr.io/@oak/commons/0.11.0/http_errors.ts:211:10)
    at Body.json (https://deno.land/x/oak@v16.1.0/body.ts:152:15)
    at eventLoopTick (ext:core/01_core.js:168:7)

See also https://github.com/oakserver/oak/issues/661

KnorpelSenf commented 3 months ago

The code responsible for oak is here: https://github.com/grammyjs/grammY/blob/fa10509af2d2daf7f3070024785bbc6d334eae17/src/convenience/frameworks.ts#L467-L481 Are you able to fix this?

t3hk0d3 commented 3 months ago

I've got an idea how to fix it. I'll try fixing when i'll get some spare time.

KnorpelSenf commented 3 months ago

Thank you!

rayz1065 commented 2 months ago

I noticed a crash when sending a POST request with an empty body to the webhook using Hono. I figured out it was due to an uncaught exception from a promise. As a test, changing the code as follows resolves the crash:

update: (async () => {
  try {
    return await c.req.json();
  } catch (error) {
    return {} as any;
  }
})(),

Such a request would likely be refused due to the header !== token check (assuming the secret is set correctly) before reaching await update, but this can't happen since the update promise throws before that. I believe this issue could be widespread, as we've observed it in two frameworks already.

A potential solution is changing the order in which webhookCallback handles promises:

const { update, respond, unauthorized, end, handlerReturn, header } =
  server(...args);
const receivedUpdate = await update;

// ...

await timeoutIfNecessary(
  bot.handleUpdate(receivedUpdate, webhookReplyEnvelope),
  typeof timeout === 'function' ? () => timeout(...args) : timeout,
  ms
);

This should fix the error for all frameworks.