elysiajs / elysia

Ergonomic Framework for Humans
https://elysiajs.com
MIT License
9.23k stars 198 forks source link

Accessing ctx.query directly breaks the object #630

Closed SEAPUNK closed 1 month ago

SEAPUNK commented 2 months ago

What version of Elysia.JS is running?

elysia@1.0.15, bun 1.1.7

What platform is your computer?

Linux 6.4.9-arch1-1 x86_64 unknown

What steps can reproduce the bug?

import { Elysia } from "elysia";
import assert from "node:assert";

const hostname = "127.0.0.1";
const port = 8089;
const app = new Elysia();

// using this request:
// http://localhost:8089/foo/bar?foo=bar&bar=baz
app.get("/foo/bar", async (ctx) => {
  console.log(ctx.query);
  // request with just the console.log above (commenting out the console.logs below)
/*
Listening on 127.0.0.1:8089
{
  query: undefined,
}
*/

  // when you add the following three console.log s:
  // console.log(JSON.stringify(ctx.query));
  // console.log(ctx.query.foo);
  // console.log(ctx.query.bar);
/*
Listening on 127.0.0.1:8089
{
  query: undefined,
  "query)": undefined,
  foo: "bar",
  bar: "baz",
}
{"foo":"bar","bar":"baz"}
bar
baz
*/
});

app.listen({
  hostname,
  port,
});
assert.ok(app.server);

console.log(`Listening on ${app.server.hostname}:${app.server.port}`);

What is the expected behavior?

For ctx.query to actually be a correct object containing all of the query parameters in the request.

What do you see instead?

See repro comments.

Additional information

Taking a glance at the codebase, it appears to be because elysia tries to be "smart" with figuring out what the route handler wants to access, and codegens around it. Why? Why is this a thing??? I understand that it may be for "performance reasons" but in place of it you open up a GIGANTIC can of worms causing issues such as above because people can't expect their code to work as it normally does within normal javascript execution. I am very much okay taking a perf hit if it means that the library I use doesn't try to be fancy with metaprogramming like this, potentially exposing my code to critical error paths.

bogeychan commented 2 months ago

You can disable aot (the codegen) as a workaround:

new Elysia({ aot: false })
SEAPUNK commented 2 months ago

Thanks, I wasn't aware of that option. It's still a pretty nasty surprise learning that this was a thing Elysia did, especially given how this appears to not be the only bug relating to it. I would feel a lot more confident in Elysia if it had it disabled by default, instead letting the user opt into it in the docs, after warning about its potential volatility.

SaltyAom commented 1 month ago

Should be fixed with 346134b alongside #656 and #658 published under 1.0.23. Let me know if it works on your end.

bogeychan commented 1 month ago

Works for me, thnx