From df67e8bd443533cfe57995145ca71036285be254 Mon Sep 17 00:00:00 2001 From: sapphi-red Date: Mon, 18 Jul 2022 01:56:19 +0900 Subject: [PATCH 1/2] fix: fs serve only edit pathname (fixes #9148) --- .../src/node/server/middlewares/static.ts | 48 ++++++++++++------- .../fs-serve/__tests__/fs-serve.spec.ts | 10 ++++ playground/fs-serve/root/src/index.html | 25 ++++++++++ 3 files changed, 65 insertions(+), 18 deletions(-) diff --git a/packages/vite/src/node/server/middlewares/static.ts b/packages/vite/src/node/server/middlewares/static.ts index 3b482068bd6138..b202261ddc2c53 100644 --- a/packages/vite/src/node/server/middlewares/static.ts +++ b/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 = getDecodedPathname(url) // 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 = getDecodedPathname(url) // 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() @@ -152,6 +158,12 @@ export function serveRawFsMiddleware( } } +function getDecodedPathname(url: URL) { + return url.pathname.includes('%') + ? decodeURIComponent(url.pathname) + : url.pathname +} + const _matchOptions = { matchBase: true } export function isFileServingAllowed( diff --git a/playground/fs-serve/__tests__/fs-serve.spec.ts b/playground/fs-serve/__tests__/fs-serve.spec.ts index d65ccdea7368cf..142b4ac9ca3131 100644 --- a/playground/fs-serve/__tests__/fs-serve.spec.ts +++ b/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 diff --git a/playground/fs-serve/root/src/index.html b/playground/fs-serve/root/src/index.html index 68eed69810c7d4..95b31e73d72ea6 100644 --- a/playground/fs-serve/root/src/index.html +++ b/playground/fs-serve/root/src/index.html @@ -7,6 +7,8 @@

Normal Import

Safe Fetch


 

+

+

 
 
Safe Fetch Subdirectory

 

@@ -25,6 +27,8 @@ 
Unsafe Fetch

 
Safe /@fs/ Fetch

 

 

+

+

 

 

 
@@ -58,6 +62,17 @@ 
Denied

     .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 @@ 
Denied

       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) => {

From 3ebdf567169107e44e7a09facb2ac45fdb24fdff Mon Sep 17 00:00:00 2001
From: sapphi-red 
Date: Mon, 18 Jul 2022 19:18:26 +0900
Subject: [PATCH 2/2] refactor: remove getDecodedPathname

---
 packages/vite/src/node/server/middlewares/static.ts | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/packages/vite/src/node/server/middlewares/static.ts b/packages/vite/src/node/server/middlewares/static.ts
index b202261ddc2c53..ed0d162b59c936 100644
--- a/packages/vite/src/node/server/middlewares/static.ts
+++ b/packages/vite/src/node/server/middlewares/static.ts
@@ -81,7 +81,7 @@ export function serveStaticMiddleware(
     }
 
     const url = new URL(req.url!, 'http://example.com')
-    const pathname = getDecodedPathname(url)
+    const pathname = decodeURIComponent(url.pathname)
 
     // apply aliases to static requests as well
     let redirectedPathname: string | undefined
@@ -133,7 +133,7 @@ export function serveRawFsMiddleware(
     // the paths are rewritten to `/@fs/` prefixed paths and must be served by
     // searching based from fs root.
     if (url.pathname.startsWith(FS_PREFIX)) {
-      const pathname = getDecodedPathname(url)
+      const pathname = decodeURIComponent(url.pathname)
       // restrict files outside of `fs.allow`
       if (
         !ensureServingAccess(
@@ -158,12 +158,6 @@ export function serveRawFsMiddleware(
   }
 }
 
-function getDecodedPathname(url: URL) {
-  return url.pathname.includes('%')
-    ? decodeURIComponent(url.pathname)
-    : url.pathname
-}
-
 const _matchOptions = { matchBase: true }
 
 export function isFileServingAllowed(