elysiajs / elysia

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

Cannot match hostnames shorter than 4 chars #748

Closed pi0 closed 1 week ago

pi0 commented 1 month ago

What version of Elysia.JS is running?

1.1.3

What platform is your computer?

Darwin 24.0.0 arm64 arm

What steps can reproduce the bug?

import { Elysia } from "elysia";

const app = new Elysia();

app.get("/", "Hello, World!");

const cases = [
  "http://localhost:3000/",
  "http://localhost",
  "http://abcd",
  "http://abc",
];

for (const url of cases) {
  console.log(url, "~>", await app.fetch(new Request(url)).text());
}

What is the expected behavior?

http://localhost:3000/ ~> Hello, World!
http://localhost ~> Hello, World!
http://abcd ~> Hello, World!
http://abc ~> Hello, World!

What do you see instead?

http://localhost:3000/ ~> Hello, World!
http://localhost ~> Hello, World!
http://abcd ~> Hello, World!
http://abc ~> NOT_FOUND

Additional information

It seems issue rases from here since path parsing starts at char 11 of URL (i think assuming there is always h:p (host port)

SaltyAom commented 1 month ago

The shortest valid minimum HTTP hostname is at least 11 characters long (http://a.bc) which is why the code is hardcoded to start checking a pathname after the first 11 characters for optimization.

Can you give me some uses of the non-standard HTTP hostname?

pi0 commented 1 month ago

Internet hosts can be as short as one character (rfc1123#2.1) and above usage can be at least used in loopback and container-to-container scenarios (where no public DNS resolver is involved) so it is valid.

I've checked and the difference is less than the magnitude of pico-seconds to check more chars, BTW feel free to close the issue if feel it is not a valid case for Elysia.

remorses commented 1 month ago

I agree Elysia should remove these kind of optimizations in favor of correctness. Skipping 11 characters doesn’t make any difference other than in artificial benchmarks

SaltyAom commented 1 week ago

We start an implementation of a non-default standardHostname config to start matching a character after http:// (7 character instead of 11) as an initial implementation on 1678c63 published under 1.1.8.

If you have more use-case, feels free to open a new discussion on discussion tab. We would like to hear to thought, thanks.