fix: default host to localhost instead of 127.0.0.1
#8543
Conversation
sapphi-red
added
p3-downstream-blocker
| // 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 | ||
| } |
There was a problem hiding this comment.
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' | |||
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
| host === undefined | ||
| ? 'localhost' | ||
| : host | ||
| host === undefined || wildcardHosts.has(host) ? 'localhost' : host |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Yeah, a warning should be good enough
There was a problem hiding this comment.
I implemented like this. (maybe the message text is not friendly... I didn't come up with a good message.)
There was a problem hiding this comment.
@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.
There was a problem hiding this comment.
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.
|
Config: Resolved local host: Why does it still resolve to 127.0.01? |
|
Here is the guide to solve it: https://vitejs.dev/config/server-options.html#server-host |
|
If anyone comes across this issue, like me ^^, it's due to my node version.
So to be sure, check our node version :) |
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:3000Other server:
[::]:3000Log output:
localhost:3000=>[::1]:3000Mismatchafter
Vite:
localhost:3000=>[::1]:3000Other server:
[::]:3000Log output:
localhost:3000=>[::1]:3000Additional context
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123).