fix: fs serve only edit pathname (fixes #9148) (#9173)

Author: sapphi-red

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

@@ -80,36 +80,40 @@ export function serveStaticMiddleware(
       return next()
     }
 
-    const url = decodeURIComponent(req.url!)
+    const url = new URL(req.url!, 'http://example.com')
+    const pathname = decodeURIComponent(url.pathname)
 
     // apply aliases to static requests as well
-    let redirected: string | undefined
+    let redirectedPathname: string | undefined
     for (const { find, replacement } of server.config.resolve.alias) {
       const matches =
-        typeof find === 'string' ? url.startsWith(find) : find.test(url)
+        typeof find === 'string'
+          ? pathname.startsWith(find)
+          : find.test(pathname)
       if (matches) {
-        redirected = url.replace(find, replacement)
+        redirectedPathname = pathname.replace(find, replacement)
         break
       }
     }
-    if (redirected) {
+    if (redirectedPathname) {
       // dir is pre-normalized to posix style
-      if (redirected.startsWith(dir)) {
-        redirected = redirected.slice(dir.length)
+      if (redirectedPathname.startsWith(dir)) {
+        redirectedPathname = redirectedPathname.slice(dir.length)
       }
     }
 
-    const resolvedUrl = redirected || url
-    let fileUrl = path.resolve(dir, resolvedUrl.replace(/^\//, ''))
-    if (resolvedUrl.endsWith('/') && !fileUrl.endsWith('/')) {
+    const resolvedPathname = redirectedPathname || pathname
+    let fileUrl = path.resolve(dir, resolvedPathname.replace(/^\//, ''))
+    if (resolvedPathname.endsWith('/') && !fileUrl.endsWith('/')) {
       fileUrl = fileUrl + '/'
     }
     if (!ensureServingAccess(fileUrl, server, res, next)) {
       return
     }
 
-    if (redirected) {
-      req.url = encodeURIComponent(redirected)
+    if (redirectedPathname) {
+      url.pathname = encodeURIComponent(redirectedPathname)
+      req.url = url.href.slice(url.origin.length)
     }
 
     serve(req, res, next)
@@ -123,16 +127,17 @@ export function serveRawFsMiddleware(
 
   // Keep the named function. The name is visible in debug logs via `DEBUG=connect:dispatcher ...`
   return function viteServeRawFsMiddleware(req, res, next) {
-    let url = decodeURIComponent(req.url!)
+    const url = new URL(req.url!, 'http://example.com')
     // In some cases (e.g. linked monorepos) files outside of root will
     // reference assets that are also out of served root. In such cases
     // the paths are rewritten to `/@fs/` prefixed paths and must be served by
     // searching based from fs root.
-    if (url.startsWith(FS_PREFIX)) {
+    if (url.pathname.startsWith(FS_PREFIX)) {
+      const pathname = decodeURIComponent(url.pathname)
       // restrict files outside of `fs.allow`
       if (
         !ensureServingAccess(
-          slash(path.resolve(fsPathFromId(url))),
+          slash(path.resolve(fsPathFromId(pathname))),
           server,
           res,
           next
@@ -141,10 +146,11 @@ export function serveRawFsMiddleware(
         return
       }
 
-      url = url.slice(FS_PREFIX.length)
-      if (isWindows) url = url.replace(/^[A-Z]:/i, '')
+      let newPathname = pathname.slice(FS_PREFIX.length)
+      if (isWindows) newPathname = newPathname.replace(/^[A-Z]:/i, '')
 
-      req.url = encodeURIComponent(url)
+      url.pathname = encodeURIComponent(newPathname)
+      req.url = url.href.slice(url.origin.length)
       serveFromRoot(req, res, next)
     } else {
       next()

playground/fs-serve/__tests__/fs-serve.spec.ts

@@ -21,6 +21,11 @@ describe.runIf(isServe)('main', () => {
     expect(await page.textContent('.safe-fetch-status')).toBe('200')
   })
 
+  test('safe fetch with query', async () => {
+    expect(await page.textContent('.safe-fetch-query')).toMatch('KEY=safe')
+    expect(await page.textContent('.safe-fetch-query-status')).toBe('200')
+  })
+
   test('safe fetch with special characters', async () => {
     expect(
       await page.textContent('.safe-fetch-subdir-special-characters')
@@ -52,6 +57,11 @@ describe.runIf(isServe)('main', () => {
     expect(await page.textContent('.safe-fs-fetch-status')).toBe('200')
   })
 
+  test('safe fs fetch', async () => {
+    expect(await page.textContent('.safe-fs-fetch-query')).toBe(stringified)
+    expect(await page.textContent('.safe-fs-fetch-query-status')).toBe('200')
+  })
+
   test('safe fs fetch with special characters', async () => {
     expect(await page.textContent('.safe-fs-fetch-special-characters')).toBe(
       stringified

playground/fs-serve/root/src/index.html

@@ -7,6 +7,8 @@ <h2>Normal Import</h2>
 <h2>Safe Fetch</h2>
 <pre class="safe-fetch-status"></pre>
 <pre class="safe-fetch"></pre>
+<pre class="safe-fetch-query-status"></pre>
+<pre class="safe-fetch-query"></pre>
 
 <h2>Safe Fetch Subdirectory</h2>
 <pre class="safe-fetch-subdir-status"></pre>
@@ -25,6 +27,8 @@ <h2>Unsafe Fetch</h2>
 <h2>Safe /@fs/ Fetch</h2>
 <pre class="safe-fs-fetch-status"></pre>
 <pre class="safe-fs-fetch"></pre>
+<pre class="safe-fs-fetch-query-status"></pre>
+<pre class="safe-fs-fetch-query"></pre>
 <pre class="safe-fs-fetch-special-characters-status"></pre>
 <pre class="safe-fs-fetch-special-characters"></pre>
 
@@ -58,6 +62,17 @@ <h2>Denied</h2>
     .then((data) => {
       text('.safe-fetch', JSON.stringify(data))
     })
+
+  // inside allowed dir with query, safe fetch
+  fetch('/src/safe.txt?query')
+    .then((r) => {
+      text('.safe-fetch-query-status', r.status)
+      return r.text()
+    })
+    .then((data) => {
+      text('.safe-fetch-query', JSON.stringify(data))
+    })
+
   // inside allowed dir, safe fetch
   fetch('/src/subdir/safe.txt')
     .then((r) => {
@@ -127,6 +142,16 @@ <h2>Denied</h2>
       text('.safe-fs-fetch', JSON.stringify(data))
     })
 
+  // imported before with query, should be treated as safe
+  fetch('/@fs/' + ROOT + '/safe.json?query')
+    .then((r) => {
+      text('.safe-fs-fetch-query-status', r.status)
+      return r.json()
+    })
+    .then((data) => {
+      text('.safe-fs-fetch-query', JSON.stringify(data))
+    })
+
   // not imported before, outside of root, treated as unsafe
   fetch('/@fs/' + ROOT + '/unsafe.json')
     .then((r) => {