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

Cannot match hostnames shorter than 4 chars #748

Closed
pi0 opened this issue Jul 22, 2024 · 4 comments
Closed

Cannot match hostnames shorter than 4 chars #748

pi0 opened this issue Jul 22, 2024 · 4 comments
Labels
bug Something isn't working

Comments

@pi0
Copy link

pi0 commented Jul 22, 2024

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)

@pi0 pi0 added the bug Something isn't working label Jul 22, 2024
@SaltyAom
Copy link
Member

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
Copy link
Author

pi0 commented Jul 23, 2024

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
Copy link

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
Copy link
Member

SaltyAom commented Aug 30, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants