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

vite's localhost is now incompatible with node's #7075

Closed
7 tasks done
mrkishi opened this issue Feb 24, 2022 · 2 comments · Fixed by #8543
Closed
7 tasks done

vite's localhost is now incompatible with node's #7075

mrkishi opened this issue Feb 24, 2022 · 2 comments · Fixed by #8543
Labels
p3-downstream-blocker Blocking the downstream ecosystem to work properly (priority)
Milestone

Comments

@mrkishi
Copy link
Contributor

mrkishi commented Feb 24, 2022

Describe the bug

As of v17, node respects the order of IP addresses returned by the system resolver.

This means localhost might resolve to 127.0.0.1 in a system and ::1 in another. As vite defaults to 127.0.0.1 and even translates localhost to it since #2977, it means starting up a vite server without a configured host (or with --host localhost) and doing net.connect(port) or net.connect(port, 'localhost') can fail.

I believe this could be fixed by defaulting to 'localhost' instead of the ipv4 here, but I'm unsure if that has negative complications:

if (
optionsHost === undefined ||
optionsHost === false ||
optionsHost === 'localhost'
) {
// Use a secure default
host = '127.0.0.1'
} else if (optionsHost === true) {

Why was localhost considered insecure? A system might even have reasons to have it resolved to a different, local interface—however unconventional that might be.

If there are no objections, I can open a PR with that change.

Reproduction

I'm not sure how to create a reproduction; I'd have to patch node's dns resolution.

System Info

n/a

Used Package Manager

pnpm

Logs

No response

Validations

@haoqunjiang
Copy link
Member

Also discussed at #5241 (comment)

Using localhost looks good to me.
The only inconvenience is that it's a minor breakage (127.0.0.1 is documented as the default value ).
But considering:

  1. It fixes an inconsistency between the command line output and the documentation
  2. v3.0 is only months away (soon after Node.js v12 reaches EOL).

I think the change is acceptable.

@mrkishi
Copy link
Contributor Author

mrkishi commented Feb 25, 2022

What about a two-step change? We could not override --host localhost to 127.0.0.1 as a first step, since that's not described in the docs and arguably unexpected behavior, and only change the default for v3.

With the override in place, the only alternative is to explicitly specify either --host 127.0.0.1 or --host ::1. While that works on a single system, it breaks down on shared configuration.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
p3-downstream-blocker Blocking the downstream ecosystem to work properly (priority)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants