fix: preview fallback (#11312)

Author: sapphi-red

package.json

@@ -129,7 +129,8 @@
       }
     },
     "patchedDependencies": {
-      "dotenv-expand@9.0.0": "patches/dotenv-expand@9.0.0.patch"
+      "dotenv-expand@9.0.0": "patches/dotenv-expand@9.0.0.patch",
+      "sirv@2.0.2": "patches/sirv@2.0.2.patch"
     }
   }
 }

packages/vite/src/node/__tests__/utils.spec.ts

@@ -12,7 +12,6 @@ import {
   posToNumber,
   processSrcSetSync,
   resolveHostname,
-  shouldServe,
 } from '../utils'
 
 describe('injectQuery', () => {
@@ -242,12 +241,6 @@ describe('asyncFlatten', () => {
   })
 })
 
-describe('shouldServe', () => {
-  test('returns false for malformed URLs', () => {
-    expect(shouldServe('/%c0%ae%c0%ae/etc/passwd', '/assets/dir')).toBe(false)
-  })
-})
-
 describe('isFileReadable', () => {
   test("file doesn't exist", async () => {
     expect(isFileReadable('/does_not_exist')).toBe(false)

packages/vite/src/node/preview.ts

@@ -15,7 +15,7 @@ import {
 import { openBrowser } from './server/openBrowser'
 import compression from './server/middlewares/compression'
 import { proxyMiddleware } from './server/middlewares/proxy'
-import { resolveHostname, resolveServerUrls, shouldServe } from './utils'
+import { resolveHostname, resolveServerUrls, shouldServeFile } from './utils'
 import { printServerUrls } from './logger'
 import { resolveConfig } from '.'
 import type { InlineConfig, ResolvedConfig } from '.'
@@ -128,13 +128,11 @@ export async function preview(
         }
       }
     },
+    shouldServe(filePath) {
+      return shouldServeFile(filePath, distDir)
+    },
   })
-  app.use(previewBase, async (req, res, next) => {
-    if (shouldServe(req.url!, distDir)) {
-      return assetServer(req, res, next)
-    }
-    next()
-  })
+  app.use(previewBase, assetServer)
 
   // apply post server hooks from plugins
   postHooks.forEach((fn) => fn && fn())

packages/vite/src/node/server/middlewares/static.ts

@@ -14,11 +14,17 @@ import {
   isInternalRequest,
   isParentDirectory,
   isWindows,
-  shouldServe,
+  shouldServeFile,
   slash,
 } from '../../utils'
 
-const sirvOptions = (headers?: OutgoingHttpHeaders): Options => {
+const sirvOptions = ({
+  headers,
+  shouldServe,
+}: {
+  headers?: OutgoingHttpHeaders
+  shouldServe?: (p: string) => void
+}): Options => {
   return {
     dev: true,
     etag: true,
@@ -38,33 +44,42 @@ const sirvOptions = (headers?: OutgoingHttpHeaders): Options => {
         }
       }
     },
+    shouldServe,
   }
 }
 
 export function servePublicMiddleware(
   dir: string,
   headers?: OutgoingHttpHeaders,
 ): Connect.NextHandleFunction {
-  const serve = sirv(dir, sirvOptions(headers))
+  const serve = sirv(
+    dir,
+    sirvOptions({
+      headers,
+      shouldServe: (filePath) => shouldServeFile(filePath, dir),
+    }),
+  )
 
   // Keep the named function. The name is visible in debug logs via `DEBUG=connect:dispatcher ...`
   return function viteServePublicMiddleware(req, res, next) {
     // skip import request and internal requests `/@fs/ /@vite-client` etc...
     if (isImportRequest(req.url!) || isInternalRequest(req.url!)) {
       return next()
     }
-    if (shouldServe(req.url!, dir)) {
-      return serve(req, res, next)
-    }
-    next()
+    serve(req, res, next)
   }
 }
 
 export function serveStaticMiddleware(
   dir: string,
   server: ViteDevServer,
 ): Connect.NextHandleFunction {
-  const serve = sirv(dir, sirvOptions(server.config.server.headers))
+  const serve = sirv(
+    dir,
+    sirvOptions({
+      headers: server.config.server.headers,
+    }),
+  )
 
   // Keep the named function. The name is visible in debug logs via `DEBUG=connect:dispatcher ...`
   return function viteServeStaticMiddleware(req, res, next) {
@@ -124,7 +139,10 @@ export function serveStaticMiddleware(
 export function serveRawFsMiddleware(
   server: ViteDevServer,
 ): Connect.NextHandleFunction {
-  const serveFromRoot = sirv('/', sirvOptions(server.config.server.headers))
+  const serveFromRoot = sirv(
+    '/',
+    sirvOptions({ headers: server.config.server.headers }),
+  )
 
   // Keep the named function. The name is visible in debug logs via `DEBUG=connect:dispatcher ...`
   return function viteServeRawFsMiddleware(req, res, next) {

packages/vite/src/node/utils.ts

@@ -1196,29 +1196,11 @@ export const isNonDriveRelativeAbsolutePath = (p: string): boolean => {
  * Determine if a file is being requested with the correct case, to ensure
  * consistent behaviour between dev and prod and across operating systems.
  */
-export function shouldServe(url: string, assetsDir: string): boolean {
-  try {
-    // viteTestUrl is set to something like http://localhost:4173/ and then many tests make calls
-    // like `await page.goto(viteTestUrl + '/example')` giving us URLs beginning with a double slash
-    const pathname = decodeURI(
-      new URL(
-        url.startsWith('//') ? url.substring(1) : url,
-        'http://example.com',
-      ).pathname,
-    )
-    const file = path.join(assetsDir, pathname)
-    if (
-      !fs.existsSync(file) ||
-      (isCaseInsensitiveFS && // can skip case check on Linux
-        !fs.statSync(file).isDirectory() &&
-        !hasCorrectCase(file, assetsDir))
-    ) {
-      return false
-    }
-    return true
-  } catch (err) {
-    return false
-  }
+export function shouldServeFile(filePath: string, root: string): boolean {
+  // can skip case check on Linux
+  if (!isCaseInsensitiveFS) return true
+
+  return hasCorrectCase(filePath, root)
 }
 
 /**

patches/sirv@2.0.2.patch

@@ -0,0 +1,43 @@
+diff --git a/build.mjs b/build.mjs
+index fe01068d0dd3f788a0978802db1747dd83c5825e..fb099c38cc2cbd59300471e7307625e2fc127f0c 100644
+--- a/build.mjs
++++ b/build.mjs
+@@ -35,7 +35,7 @@ function viaCache(cache, uri, extns) {
+ 	}
+ }
+ 
+-function viaLocal(dir, isEtag, uri, extns) {
++function viaLocal(dir, isEtag, uri, extns, shouldServe) {
+ 	let i=0, arr=toAssume(uri, extns);
+ 	let abs, stats, name, headers;
+ 	for (; i < arr.length; i++) {
+@@ -43,6 +43,7 @@ function viaLocal(dir, isEtag, uri, extns) {
+ 		if (abs.startsWith(dir) && fs.existsSync(abs)) {
+ 			stats = fs.statSync(abs);
+ 			if (stats.isDirectory()) continue;
++			if (shouldServe && !shouldServe(abs)) continue;
+ 			headers = toHeaders(name, stats, isEtag);
+ 			headers['Cache-Control'] = isEtag ? 'no-cache' : 'no-store';
+ 			return { abs, stats, headers };
+@@ -172,7 +173,7 @@ export default function (dir, opts={}) {
+ 			catch (err) { /* malform uri */ }
+ 		}
+ 
+-		let data = lookup(pathname, extns) || isSPA && !isMatch(pathname, ignores) && lookup(fallback, extns);
++		let data = lookup(pathname, extns, opts.shouldServe) || isSPA && !isMatch(pathname, ignores) && lookup(fallback, extns, opts.shouldServe);
+ 		if (!data) return next ? next() : isNotFound(req, res);
+ 
+ 		if (isEtag && req.headers['if-none-match'] === data.headers['ETag']) {
+diff --git a/sirv.d.ts b/sirv.d.ts
+index c05040fc6ec504a1828a7badd39f669981acd0ee..e9597e8b5bf24613a09565f0e13024ae3ca8fa5e 100644
+--- a/sirv.d.ts
++++ b/sirv.d.ts
+@@ -19,6 +19,8 @@ declare module 'sirv' {
+ 		gzip?: boolean;
+ 		onNoMatch?: (req: IncomingMessage, res: ServerResponse) => void;
+ 		setHeaders?: (res: ServerResponse, pathname: string, stats: Stats) => void;
++		/** patched */
++		shouldServe?: (absoluteFilePath: string) => void;
+ 	}
+ 
+ 	export default function(dir?: string, opts?: Options): RequestHandler;

playground/assets/__tests__/assets.spec.ts

@@ -24,19 +24,28 @@ const assetMatch = isBuild
 
 const iconMatch = `/foo/icon.png`
 
+const fetchPath = (p: string) => {
+  return fetch(path.posix.join(viteTestUrl, p), {
+    headers: { Accept: 'text/html,*/*' },
+  })
+}
+
 test('should have no 404s', () => {
   browserLogs.forEach((msg) => {
     expect(msg).not.toMatch('404')
   })
 })
 
 test('should get a 404 when using incorrect case', async () => {
-  expect((await fetch(path.posix.join(viteTestUrl, 'icon.png'))).status).toBe(
-    200,
-  )
-  expect((await fetch(path.posix.join(viteTestUrl, 'ICON.png'))).status).toBe(
-    404,
-  )
+  expect((await fetchPath('icon.png')).status).toBe(200)
+  // won't be wrote to index.html because the url includes `.`
+  expect((await fetchPath('ICON.png')).status).toBe(404)
+
+  expect((await fetchPath('bar')).status).toBe(200)
+  // fallback to index.html
+  const incorrectBarFetch = await fetchPath('BAR')
+  expect(incorrectBarFetch.status).toBe(200)
+  expect(incorrectBarFetch.headers.get('Content-Type')).toContain('text/html')
 })
 
 describe('injected scripts', () => {

playground/assets/static/bar

@@ -0,0 +1 @@
+bar