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

fix: default host to localhost instead of 127.0.0.1 #8543

Merged
merged 9 commits into from Jun 14, 2022

Conversation

sapphi-red
Copy link
Member

@sapphi-red sapphi-red commented Jun 11, 2022

BREAKING CHANGE: default host is now localhost.

Description

This PR changes default host to localhost.
Related discussions: #5241 (comment), #7075 (comment)

I confirmed #5241 will be fixed by this PR. (Node v16.15.1 + Windows)

fixes #5241
fixes #6519
fixes #7016
fixes #7075
close #6523
refs #7271
refs #7274

Why does this solve them?

#5241 (comment)

before
Vite: 127.0.0.1:3000
Other server: [::]:3000
Log output: localhost:3000 => [::1]:3000 Mismatch

after
Vite: localhost:3000 => [::1]:3000
Other server: [::]:3000
Log output: localhost:3000 => [::1]:3000

Additional context


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the Commit Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@sapphi-red sapphi-red added p3-downstream-blocker Blocking the downstream ecosystem to work properly (priority) breaking change labels Jun 11, 2022
@sapphi-red sapphi-red added this to the 3.0 milestone Jun 11, 2022
Comment on lines +190 to +195
// This could be removed when Vite only supports Node 17+ because verbatim=true is default
// https://github.com/nodejs/node/pull/39987
if (host === 'localhost') {
const addr = await dns.lookup('localhost', { verbatim: true })
host = addr.address
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I confirmed this part with Node v18.3.0 + Windows.

@@ -5,6 +5,7 @@ import type {
OutgoingHttpHeaders as HttpServerHeaders
} from 'http'
import type { ServerOptions as HttpsServerOptions } from 'https'
import { promises as dns } from 'dns'
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here I'm using dns.promises since dns/promises is supported from Node 15+.
https://nodejs.org/dist/latest-v16.x/docs/api/dns.html#dns-promises-api

@sapphi-red sapphi-red marked this pull request as ready for review June 11, 2022 18:49
patak-dev
patak-dev previously approved these changes Jun 12, 2022
Copy link
Member

@patak-dev patak-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing work here @sapphi-red!

@sodatea you may also want to review this one. I think the best would be to merge and release another alpha so we can get feedback from more people about the change.

bluwy
bluwy previously approved these changes Jun 12, 2022
host === undefined
? 'localhost'
: host
host === undefined || wildcardHosts.has(host) ? 'localhost' : host
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not safe.
When you listen to 0.0.0.0:3000 in Vite, it is still possible to create a server that listens to 127.0.0.1:3000, and localhost:3000 will resolve to that server instead.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!
Though I'm not sure how to fix this.
Opening http://0.0.0.0:3000 in browsers does not work and any other network ip's could also be overridden by other servers.

Maybe adding a warning that denotes ports might be overridden?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, a warning should be good enough

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image
I implemented like this. (maybe the message text is not friendly... I didn't come up with a good message.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sapphi-red I was confused by the message, as you may know. As a Vite user, I would expect it to say "security" and be listed as a warning (not informational). Also while the "ports might be overridden" makes sense in the context of this thread, it lead me in a wrong direction.

I know this already shipped, but a suggestion:
"You are using a wildcard host. This means a service listening to 127.0.0.1:port on your system may hijack your traffic. More information: ".

The "more information" should include a way to permanently silence the warning.

My 2c... Just that you know, anyone running Vite under Docker Compose will need to enable the host: true and thus will see the message. Docker Compose port forwarding does not work without it.

Disclaimer. I don't claim to understand the whole change in play here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion!

This message is not intended as a security warning. The purpose of this message is to inform the user that a server other than Vite might respond when accessing the output address. Because it is a confusing behavior.

I'll create a PR to improve this message.

@hegelstad
Copy link

Config:
Screenshot 2023-02-21 at 09 27 51

Resolved local host:
Screenshot 2023-02-21 at 09 27 38

Why does it still resolve to 127.0.01?

@hegelstad
Copy link

Here is the guide to solve it: https://vitejs.dev/config/server-options.html#server-host

@Horsty80
Copy link

Horsty80 commented Apr 6, 2023

If anyone comes across this issue, like me ^^, it's due to my node version.
I switch between node16 and node18 due to some dependencies.
And when you use vite in :

  • node16 you get http://127.0.0.1:5137
  • node18 you get https://localhost:5137

So to be sure, check our node version :)

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