Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
t3hk0d3 opened this issue Jul 14, 2024 · 4 comments
Open

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

t3hk0d3 opened this issue Jul 14, 2024 · 4 comments

Comments

@t3hk0d3
Copy link

t3hk0d3 commented Jul 14, 2024

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/[email protected]/body.ts:152:15)
    at eventLoopTick (ext:core/01_core.js:168:7)

See also oakserver/oak#661

@KnorpelSenf
Copy link
Member

The code responsible for oak is here:

/** oak web framework */
const oak: OakAdapter = (ctx) => ({
update: ctx.request.body.json(),
header: ctx.request.headers.get(SECRET_HEADER) || undefined,
end: () => {
ctx.response.status = 200;
},
respond: (json) => {
ctx.response.type = "json";
ctx.response.body = json;
},
unauthorized: () => {
ctx.response.status = 401;
},
});

Are you able to fix this?

@t3hk0d3
Copy link
Author

t3hk0d3 commented Jul 15, 2024

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

@KnorpelSenf
Copy link
Member

Thank you!

@rayz1065
Copy link
Member

rayz1065 commented Jul 27, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants