From 5c61318592d3ef818b033343b4a38728c6bddbb6 Mon Sep 17 00:00:00 2001
From: Joe Previte <jjprevite@gmail.com>
Date: Mon, 19 Jul 2021 14:22:42 -0700
Subject: [PATCH] refactor: only accept string in pathToFsPath

CodeQL caught a path where we were passing in req.query.path
to pathToFsPath, which may not have been a string.

So we refactored some things to ensure we only pass it a string
which also let us change the parameter type to string
instead of string | string[].
---
 src/node/routes/vscode.ts   | 14 ++++++++------
 src/node/util.ts            | 10 ++--------
 test/unit/node/util.test.ts | 13 +------------
 3 files changed, 11 insertions(+), 26 deletions(-)

diff --git a/src/node/routes/vscode.ts b/src/node/routes/vscode.ts
index ffd66ab9c..160f5a432 100644
--- a/src/node/routes/vscode.ts
+++ b/src/node/routes/vscode.ts
@@ -63,9 +63,10 @@ router.get("/", async (req, res) => {
  * TODO: Might currently be unused.
  */
 router.get("/resource(/*)?", ensureAuthenticated, async (req, res) => {
-  if (typeof req.query.path === "string") {
-    res.set("Content-Type", getMediaMime(req.query.path))
-    res.send(await fs.readFile(pathToFsPath(req.query.path)))
+  const path = getFirstString(req.query.path)
+  if (path) {
+    res.set("Content-Type", getMediaMime(path))
+    res.send(await fs.readFile(pathToFsPath(path)))
   }
 })
 
@@ -73,9 +74,10 @@ router.get("/resource(/*)?", ensureAuthenticated, async (req, res) => {
  * Used by VS Code to load files.
  */
 router.get("/vscode-remote-resource(/*)?", ensureAuthenticated, async (req, res) => {
-  if (typeof req.query.path === "string") {
-    res.set("Content-Type", getMediaMime(req.query.path))
-    res.send(await fs.readFile(pathToFsPath(req.query.path)))
+  const path = getFirstString(req.query.path)
+  if (path) {
+    res.set("Content-Type", getMediaMime(path))
+    res.send(await fs.readFile(pathToFsPath(path)))
   }
 })
 
diff --git a/src/node/util.ts b/src/node/util.ts
index 2a010f0e9..1216601ef 100644
--- a/src/node/util.ts
+++ b/src/node/util.ts
@@ -458,17 +458,11 @@ enum CharCode {
  * Taken from vs/base/common/uri.ts. It's not imported to avoid also importing
  * everything that file imports.
  */
-export function pathToFsPath(path: string | string[], keepDriveLetterCasing = false): string {
+export function pathToFsPath(path: string, keepDriveLetterCasing = false): string {
   const isWindows = process.platform === "win32"
-  const uri = { authority: undefined, path: getFirstString(path), scheme: "file" }
+  const uri = { authority: undefined, path: getFirstString(path) || "", scheme: "file" }
   let value: string
 
-  if (typeof uri.path !== "string") {
-    throw new Error(
-      `Could not compute fsPath from given uri. Expected path to be of type string, but was of type ${typeof uri.path}.`,
-    )
-  }
-
   if (uri.authority && uri.path.length > 1 && uri.scheme === "file") {
     // unc path: file://shares/c$/far/boo
     value = `//${uri.authority}${uri.path}`
diff --git a/test/unit/node/util.test.ts b/test/unit/node/util.test.ts
index 8b3923fb4..30cd4672c 100644
--- a/test/unit/node/util.test.ts
+++ b/test/unit/node/util.test.ts
@@ -464,19 +464,8 @@ describe("pathToFsPath", () => {
   it("should keep drive letter casing when set to true", () => {
     expect(util.pathToFsPath("/C:/far/bo", true)).toBe("C:/far/bo")
   })
-  it("should throw an error if a non-string is passed in for path", () => {
-    expect(() =>
-      util
-        // @ts-expect-error We need to check other types
-        .pathToFsPath({}),
-    ).toThrow(`Could not compute fsPath from given uri. Expected path to be of type string, but was of type undefined.`)
-  })
-  it("should not throw an error for a string array", () => {
-    // @ts-expect-error We need to check other types
-    expect(() => util.pathToFsPath(["/hello/foo", "/hello/bar"]).not.toThrow())
-  })
   it("should use the first string in a string array", () => {
-    expect(util.pathToFsPath(["/hello/foo", "/hello/bar"])).toBe("/hello/foo")
+    expect(util.pathToFsPath("/hello/foo")).toBe("/hello/foo")
   })
   it("should replace / with \\ on Windows", () => {
     let ORIGINAL_PLATFORM = process.platform