fix(dev): harden listener edge cases

Author: danielroe

packages/nuxt-cli/src/commands/dev.ts

@@ -15,7 +15,7 @@ import { satisfies } from 'verkit'
 import { initialize } from '../dev'
 
 import { closeInspector, openInspector, resolveInspectOptions } from '../dev/inspect'
-import { isReusePortSupported } from '../dev/listen'
+import { isReusePortSupported, parsePort } from '../dev/listen'
 import { ForkPool } from '../dev/pool'
 import { formatRestartReason } from '../dev/reason'
 import { setupShortcuts } from '../dev/shortcuts'
@@ -158,7 +158,7 @@ const command = defineCommand({
     const listenOverrides = resolveListenOverrides(ctx.args)
 
     const takeover = await takeOverDevServer(resolveDevBuildDir(cwd), {
-      requestedPort: parseRequestedPort(listenOverrides.port),
+      requestedPort: parsePort(listenOverrides.port),
       takeover: ctx.args.takeover,
     })
     if (takeover.action === 'refused') {
@@ -408,11 +408,6 @@ function resolveDevBuildDir(cwd: string): string {
   return resolve(cwd, '.nuxt')
 }
 
-function parseRequestedPort(port: string | number | undefined): number | undefined {
-  const parsed = Number(port)
-  return port === undefined || port === '' || !Number.isInteger(parsed) || parsed <= 0 ? undefined : parsed
-}
-
 function resolveForkPoolSize(): number | undefined {
   const raw = process.env.NUXT_DEV_FORK_POOL_SIZE
   if (!raw) {

packages/nuxt-cli/src/dev/listen.ts

@@ -155,7 +155,7 @@ export async function listen(handler: RequestListener, options: ListenOptions =
   const isolatedEnvironment = options.hostname === undefined && !options.public && detectIsolatedEnvironment()
   const hostname = validateHostname(options.hostname, options.public) ?? (options.public || isolatedEnvironment ? '' : 'localhost')
 
-  const requestedPort = options.port === undefined || options.port === '' ? undefined : Number(options.port)
+  const requestedPort = parsePort(options.port)
   const port = options.handover && requestedPort
     ? requestedPort
     : await resolvePort(requestedPort, hostname, options.strictPort)
@@ -191,15 +191,16 @@ export async function listen(handler: RequestListener, options: ListenOptions =
     tunnel = await startTunnel(`${protocol}://localhost:${address.port}`, !!certificate)
   }
 
+  const tunnelURL = tunnel?.url && tunnel.url + baseURL
   const portless = resolvePortlessURLs()
   const portlessURL = portless.url && portless.url + baseURL
   const portlessShareURL = portless.shareURL && portless.shareURL + baseURL
   const stackblitzURL = resolveStackblitzURL()
 
   function getURLs(): ListenURL[] {
     const urls: ListenURL[] = []
-    if (tunnel) {
-      urls.push({ url: tunnel.url, type: 'tunnel' })
+    if (tunnelURL) {
+      urls.push({ url: tunnelURL, type: 'tunnel' })
     }
     for (const portlessURL of portless.all) {
       urls.push({ url: portlessURL + baseURL, type: 'public' })
@@ -221,7 +222,7 @@ export async function listen(handler: RequestListener, options: ListenOptions =
 
   // The StackBlitz URL points at the editor rather than at a host another
   // device can open, so it is not a QR code candidate.
-  const shareableURL = options.publicURL || tunnel?.url || portlessShareURL || portlessURL
+  const shareableURL = options.publicURL || tunnelURL || portlessShareURL || portlessURL
   const publicURL = shareableURL || stackblitzURL
 
   const qrURL = options.qr === false
@@ -278,31 +279,35 @@ export async function listen(handler: RequestListener, options: ListenOptions =
     https: certificate,
     getURLs,
     showURLs,
-    close: async () => {
-      await tunnel?.close()
-      return new Promise<void>((resolve, reject) => {
-        let forceClose: NodeJS.Timeout | undefined
-        server.close((error) => {
-          if (forceClose) {
-            clearTimeout(forceClose)
-          }
-          if (error) {
-            reject(error)
-          }
-          else {
-            resolve()
-          }
-        })
-        // Sockets waiting on keep-alive are closed at once, so shutdown is only
-        // delayed while a request is actually being served.
-        server.closeIdleConnections?.()
-        forceClose = setTimeout(() => server.closeAllConnections?.(), CONNECTION_DRAIN_TIMEOUT_MS)
-        forceClose.unref()
-      })
-    },
+    close: () => Promise.all([
+      tunnel?.close(),
+      closeServer(server),
+    ]).then(() => {}),
   }
 }
 
+function closeServer(server: HttpServer): Promise<void> {
+  return new Promise<void>((resolve, reject) => {
+    let forceClose: NodeJS.Timeout | undefined
+    server.close((error) => {
+      if (forceClose) {
+        clearTimeout(forceClose)
+      }
+      if (error) {
+        reject(error)
+      }
+      else {
+        resolve()
+      }
+    })
+    // Sockets waiting on keep-alive are closed at once, so shutdown is only
+    // delayed while a request is actually being served.
+    server.closeIdleConnections?.()
+    forceClose = setTimeout(() => server.closeAllConnections?.(), CONNECTION_DRAIN_TIMEOUT_MS)
+    forceClose.unref()
+  })
+}
+
 function bindServer(server: HttpServer, port: number, hostname: string, reusePort: boolean): Promise<void> {
   return new Promise<void>((resolve, reject) => {
     const onError = (error: NodeJS.ErrnoException) => {
@@ -372,6 +377,17 @@ export function isReusePortSupported(): Promise<boolean> {
   return reusePortSupport
 }
 
+export function parsePort(value: string | number | undefined): number | undefined {
+  if (value === undefined || value === '') {
+    return undefined
+  }
+  const port = Number(value)
+  if (!Number.isInteger(port) || port < 0 || port > 65_535) {
+    throw new Error(`Invalid port \`${value}\`; expected an integer between 0 and 65535.`)
+  }
+  return port
+}
+
 async function resolvePort(requestedPort: number | undefined, hostname: string, strictPort?: boolean): Promise<number> {
   if (requestedPort === 0) {
     return getPort({ random: true, host: hostname || undefined })

packages/nuxt-cli/src/dev/utils.ts

@@ -899,8 +899,8 @@ function createConfigDirWatcher(cwd: string, onReload: (path: string) => void) {
 
   fileWatcher.prime(configDir)
   const configDirWatcher = watch(configDir)
-  configDirWatcher.on('change', (_event, file: string) => {
-    if (!fileWatcher.shouldEmitChange(resolve(configDir, file))) {
+  configDirWatcher.on('change', (_event, file: string | null) => {
+    if (!file || !fileWatcher.shouldEmitChange(resolve(configDir, file))) {
       return
     }
 

packages/nuxt-cli/test/unit/listen.spec.ts

@@ -5,7 +5,7 @@ import { networkInterfaces } from 'node:os'
 
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
 
-import { copyURL, formatDisplayURL, getNetworkAddresses, isReusePortSupported, listen, openBrowser, resolveOpenCommand, validateHostname } from '../../src/dev/listen'
+import { copyURL, formatDisplayURL, getNetworkAddresses, isReusePortSupported, listen, openBrowser, parsePort, resolveOpenCommand, validateHostname } from '../../src/dev/listen'
 
 const writeText = vi.hoisted(() => vi.fn())
 const isolatedEnvironment = vi.hoisted(() => ({ current: undefined as string | undefined }))
@@ -115,6 +115,22 @@ describe('validateHostname', () => {
   })
 })
 
+describe('parsePort', () => {
+  it.each([
+    ['0', 0],
+    ['3000', 3000],
+    [65_535, 65_535],
+    ['', undefined],
+    [undefined, undefined],
+  ])('should parse %j as %j', (value, expected) => {
+    expect(parsePort(value)).toBe(expected)
+  })
+
+  it.each(['nope', '-1', '1.5', '65536', 'Infinity'])('should reject %s', (value) => {
+    expect(() => parsePort(value)).toThrow(`Invalid port \`${value}\``)
+  })
+})
+
 describe('resolveOpenCommand', () => {
   const url = 'http://localhost:3000/'
 
@@ -180,6 +196,10 @@ describe('listen', () => {
     expect(listener.url).toBe(`http://localhost:${listener.address.port}/`)
   })
 
+  it('should reject an invalid port before binding', async () => {
+    await expect(start({ port: 'nope' })).rejects.toThrow('Invalid port `nope`; expected an integer between 0 and 65535.')
+  })
+
   it('should fall back to another port by default', async () => {
     const first = await start({ port: 0 })
     const second = await start({ port: first.address.port })
@@ -242,6 +262,29 @@ describe('listen', () => {
 })
 
 describe('listener.close', () => {
+  it('should close the listener while a tunnel is still shutting down', async () => {
+    let closeTunnel: (() => void) | undefined
+    vi.doMock('../../src/dev/tunnel', () => ({
+      startTunnel: async () => ({
+        url: 'https://example.test',
+        close: () => new Promise<void>((resolve) => {
+          closeTunnel = resolve
+        }),
+      }),
+    }))
+
+    const listener = await listen((_req, res) => res.end('ok'), { port: 0, hostname: '127.0.0.1', baseURL: '/dashboard/', showURL: false, tunnel: true })
+    expect(listener.publicURL).toBe('https://example.test/dashboard/')
+    expect(listener.getURLs()).toContainEqual({ url: 'https://example.test/dashboard/', type: 'tunnel' })
+
+    const closed = listener.close()
+
+    await vi.waitFor(() => expect(listener.server.listening).toBe(false))
+    closeTunnel!()
+    await closed
+    vi.doUnmock('../../src/dev/tunnel')
+  })
+
   it('should let an in-flight request finish', async () => {
     let respond: (() => void) | undefined
     const listener = await listen((_req, res) => {