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(server): Listen only to 127.0.0.1 by default #2977

Merged
merged 12 commits into from
May 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion docs/config/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -328,8 +328,12 @@ export default async ({ command, mode }) => {
### server.host

- **Type:** `string`
- **Default:** `'127.0.0.1'`

Specify server hostname.
Specify which IP addresses the server should listen on.
Set this to `0.0.0.0` to listen on all addresses, including LAN and public addresses.

This can be set via the CLI using `--host 0.0.0.0` or `--host`.

### server.port

Expand Down
2 changes: 1 addition & 1 deletion packages/vite/src/node/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ cli
cli
.command('[root]') // default command
.alias('serve')
.option('--host <host>', `[string] specify hostname`)
.option('--host [host]', `[string] specify hostname`)
.option('--port <port>', `[number] specify port`)
.option('--https', `[boolean] use TLS + HTTP/2`)
.option('--open [path]', `[boolean | string] open browser on startup`)
Expand Down
53 changes: 34 additions & 19 deletions packages/vite/src/node/preview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,33 +41,48 @@ export async function preview(
)

const options = config.server || {}
const hostname = options.host || 'localhost'
let hostname: string | undefined
if (options.host === undefined || options.host === 'localhost') {
// Use a secure default
hostname = '127.0.0.1'
} else if (options.host === true) {
// The user probably passed --host in the CLI, without arguments
hostname = undefined // undefined typically means 0.0.0.0 or :: (listen on all IPs)
} else {
hostname = options.host as string
}
const protocol = options.https ? 'https' : 'http'
const logger = config.logger
const base = config.base

httpServer.listen(port, () => {
httpServer.listen(port, hostname, () => {
logger.info(
chalk.cyan(`\n vite v${require('vite/package.json').version}`) +
chalk.green(` build preview server running at:\n`)
)
const interfaces = os.networkInterfaces()
Object.keys(interfaces).forEach((key) =>
(interfaces[key] || [])
.filter((details) => details.family === 'IPv4')
.map((detail) => {
return {
type: detail.address.includes('127.0.0.1')
? 'Local: '
: 'Network: ',
host: detail.address.replace('127.0.0.1', hostname)
}
})
.forEach(({ type, host }) => {
const url = `${protocol}://${host}:${chalk.bold(port)}${base}`
logger.info(` > ${type} ${chalk.cyan(url)}`)
})
)
if (hostname === '127.0.0.1') {
const url = `${protocol}://localhost:${chalk.bold(port)}${base}`
logger.info(` > Local: ${chalk.cyan(url)}`)
logger.info(` > Network: ${chalk.dim('use `--host` to expose')}`)
} else {
const interfaces = os.networkInterfaces()
Object.keys(interfaces).forEach((key) =>
(interfaces[key] || [])
Shinigami92 marked this conversation as resolved.
Show resolved Hide resolved
.filter((details) => details.family === 'IPv4')
.map((detail) => {
return {
type: detail.address.includes('127.0.0.1')
? 'Local: '
: 'Network: ',
host: detail.address
}
})
Shinigami92 marked this conversation as resolved.
Show resolved Hide resolved
.forEach(({ type, host }) => {
const url = `${protocol}://${host}:${chalk.bold(port)}${base}`
Shinigami92 marked this conversation as resolved.
Show resolved Hide resolved
logger.info(` > ${type} ${chalk.cyan(url)}`)
})
)
}
Comment on lines +63 to +85
Copy link
Member

Choose a reason for hiding this comment

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

This can be extracted too

Copy link
Member

Choose a reason for hiding this comment

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

Agreed, but I think it is better if we do these refactorings in a future PR.

Copy link
Member

Choose a reason for hiding this comment

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

👍🏼 Okay to leave this one out, but it would be good to extract the added code (previous comment)


if (options.open) {
const path = typeof options.open === 'string' ? options.open : base
Expand Down
60 changes: 38 additions & 22 deletions packages/vite/src/node/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ import { ssrRewriteStacktrace } from '../ssr/ssrStacktrace'
import { createMissingImporterRegisterFn } from '../optimizer/registerMissing'

export interface ServerOptions {
host?: string
host?: string | boolean
port?: number
/**
* Enable TLS + HTTP/2.
Expand Down Expand Up @@ -531,8 +531,17 @@ async function startServer(

const options = server.config.server || {}
let port = inlinePort || options.port || 3000
let hostname = options.host || 'localhost'
if (hostname === '0.0.0.0') hostname = 'localhost'
let hostname: string | undefined
if (options.host === undefined || options.host === 'localhost') {
// Use a secure default
hostname = '127.0.0.1'
} else if (options.host === true) {
// probably passed --host in the CLI, without arguments
hostname = undefined // undefined typically means 0.0.0.0 or :: (listen on all IPs)
} else {
hostname = options.host as string
Comment on lines +541 to +542
Copy link
Member

Choose a reason for hiding this comment

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

question: Is this also the case if options.host is set to false?
Cause I see that host is of type string | boolean | undefined

Copy link
Member

Choose a reason for hiding this comment

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

@patak-js as I mentioned here, what is with the options.host being false case?

image

The as string just hides the TypeScript safeness here ⚠️

}

const protocol = options.https ? 'https' : 'http'
const info = server.config.logger.info
const base = server.config.base
Expand All @@ -545,7 +554,7 @@ async function startServer(
reject(new Error(`Port ${port} is already in use`))
} else {
info(`Port ${port} is in use, trying another one...`)
httpServer.listen(++port)
httpServer.listen(++port, hostname)
}
} else {
httpServer.removeListener('error', onError)
Expand All @@ -555,7 +564,7 @@ async function startServer(

httpServer.on('error', onError)

httpServer.listen(port, options.host, () => {
httpServer.listen(port, hostname, () => {
httpServer.removeListener('error', onError)

info(
Expand All @@ -565,23 +574,30 @@ async function startServer(
clear: !server.config.logger.hasWarned
}
)
const interfaces = os.networkInterfaces()
Object.keys(interfaces).forEach((key) =>
(interfaces[key] || [])
.filter((details) => details.family === 'IPv4')
.map((detail) => {
return {
type: detail.address.includes('127.0.0.1')
? 'Local: '
: 'Network: ',
host: detail.address.replace('127.0.0.1', hostname)
}
})
.forEach(({ type, host }) => {
const url = `${protocol}://${host}:${chalk.bold(port)}${base}`
info(` > ${type} ${chalk.cyan(url)}`)
})
)

if (hostname === '127.0.0.1') {
const url = `${protocol}://localhost:${chalk.bold(port)}${base}`
info(` > Local: ${chalk.cyan(url)}`)
info(` > Network: ${chalk.dim('use `--host` to expose')}`)
} else {
const interfaces = os.networkInterfaces()
Object.keys(interfaces).forEach((key) =>
(interfaces[key] || [])
.filter((details) => details.family === 'IPv4')
.map((detail) => {
return {
type: detail.address.includes('127.0.0.1')
? 'Local: '
: 'Network: ',
host: detail.address
}
})
.forEach(({ type, host }) => {
const url = `${protocol}://${host}:${chalk.bold(port)}${base}`
info(` > ${type} ${chalk.cyan(url)}`)
})
)
}

// @ts-ignore
if (global.__vite_start_time) {
Expand Down
3 changes: 2 additions & 1 deletion scripts/jestPerTestSetup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@ beforeAll(async () => {
// misses change events, so enforce polling for consistency
usePolling: true,
interval: 100
}
},
host: true
},
build: {
// skip transpilation and dynamic import polyfills during tests to
Expand Down