From eb498b0d6d8e87aa5ad1dfb473964078c1821e0f Mon Sep 17 00:00:00 2001 From: Asher Date: Tue, 29 Aug 2023 15:25:24 -0700 Subject: [PATCH] Remove humanPath (#6404) The tilde is ambiguous and it can be helpful to know exactly what paths code-server is trying to use, especially if it is running as a different user than you expected. --- src/node/cli.ts | 5 ++--- src/node/main.ts | 13 ++++++------- src/node/routes/login.ts | 5 ++--- src/node/util.ts | 14 -------------- test/unit/node/util.test.ts | 16 ---------------- 5 files changed, 10 insertions(+), 43 deletions(-) diff --git a/src/node/cli.ts b/src/node/cli.ts index 8978490c1..3e5a642e9 100644 --- a/src/node/cli.ts +++ b/src/node/cli.ts @@ -1,9 +1,8 @@ import { field, Level, logger } from "@coder/logger" import { promises as fs } from "fs" import { load } from "js-yaml" -import * as os from "os" import * as path from "path" -import { generateCertificate, generatePassword, humanPath, paths, splitOnFirstEquals } from "./util" +import { generateCertificate, generatePassword, paths, splitOnFirstEquals } from "./util" import { EditorSessionManagerClient } from "./vscodeSocket" export enum Feature { @@ -663,7 +662,7 @@ export async function readConfigFile(configPath?: string): Promise { await fs.writeFile(configPath, defaultConfigFile(generatedPassword), { flag: "wx", // wx means to fail if the path exists. }) - logger.info(`Wrote default config file to ${humanPath(os.homedir(), configPath)}`) + logger.info(`Wrote default config file to ${configPath}`) } catch (error: any) { // EEXIST is fine; we don't want to overwrite existing configurations. if (error.code !== "EEXIST") { diff --git a/src/node/main.ts b/src/node/main.ts index 76f5f2560..f61ca2bb4 100644 --- a/src/node/main.ts +++ b/src/node/main.ts @@ -1,13 +1,12 @@ import { field, logger } from "@coder/logger" import http from "http" -import * as os from "os" import { Disposable } from "../common/emitter" import { plural } from "../common/util" import { createApp, ensureAddress } from "./app" import { AuthType, DefaultedArgs, Feature, SpawnCodeCli, toCodeArgs, UserProvidedArgs } from "./cli" import { commit, version } from "./constants" import { register } from "./routes" -import { humanPath, isDirectory, loadAMDModule, open } from "./util" +import { isDirectory, loadAMDModule, open } from "./util" /** * Return true if the user passed an extension-related VS Code flag. @@ -109,8 +108,8 @@ export const runCodeServer = async ( ): Promise<{ dispose: Disposable["dispose"]; server: http.Server }> => { logger.info(`code-server ${version} ${commit}`) - logger.info(`Using user-data-dir ${humanPath(os.homedir(), args["user-data-dir"])}`) - logger.trace(`Using extensions-dir ${humanPath(os.homedir(), args["extensions-dir"])}`) + logger.info(`Using user-data-dir ${args["user-data-dir"]}`) + logger.trace(`Using extensions-dir ${args["extensions-dir"]}`) if (args.auth === AuthType.Password && !args.password && !args["hashed-password"]) { throw new Error( @@ -123,7 +122,7 @@ export const runCodeServer = async ( const serverAddress = ensureAddress(app.server, protocol) const disposeRoutes = await register(app, args) - logger.info(`Using config file ${humanPath(os.homedir(), args.config)}`) + logger.info(`Using config file ${args.config}`) logger.info(`${protocol.toUpperCase()} server listening on ${serverAddress.toString()}`) if (args.auth === AuthType.Password) { logger.info(" - Authentication is enabled") @@ -132,14 +131,14 @@ export const runCodeServer = async ( } else if (args.usingEnvHashedPassword) { logger.info(" - Using password from $HASHED_PASSWORD") } else { - logger.info(` - Using password from ${humanPath(os.homedir(), args.config)}`) + logger.info(` - Using password from ${args.config}`) } } else { logger.info(" - Authentication is disabled") } if (args.cert) { - logger.info(` - Using certificate for HTTPS: ${humanPath(os.homedir(), args.cert.value)}`) + logger.info(` - Using certificate for HTTPS: ${args.cert.value}`) } else { logger.info(" - Not serving HTTPS") } diff --git a/src/node/routes/login.ts b/src/node/routes/login.ts index b97b7647b..db796343d 100644 --- a/src/node/routes/login.ts +++ b/src/node/routes/login.ts @@ -1,12 +1,11 @@ import { Router, Request } from "express" import { promises as fs } from "fs" import { RateLimiter as Limiter } from "limiter" -import * as os from "os" import * as path from "path" import { CookieKeys } from "../../common/http" import { rootPath } from "../constants" import { authenticated, getCookieOptions, redirect, replaceTemplates } from "../http" -import { getPasswordMethod, handlePasswordValidation, humanPath, sanitizeString, escapeHtml } from "../util" +import { getPasswordMethod, handlePasswordValidation, sanitizeString, escapeHtml } from "../util" import i18n from "../i18n" // RateLimiter wraps around the limiter library for logins. @@ -33,7 +32,7 @@ const getRoot = async (req: Request, error?: Error): Promise => { i18n.changeLanguage(locale) const appName = req.args["app-name"] || "code-server" const welcomeText = req.args["welcome-text"] || (i18n.t("WELCOME", { app: appName }) as string) - let passwordMsg = i18n.t("LOGIN_PASSWORD", { configFile: humanPath(os.homedir(), req.args.config) }) + let passwordMsg = i18n.t("LOGIN_PASSWORD", { configFile: req.args.config }) if (req.args.usingEnvPassword) { passwordMsg = i18n.t("LOGIN_USING_ENV_PASSWORD") } else if (req.args.usingEnvHashedPassword) { diff --git a/src/node/util.ts b/src/node/util.ts index 6abb3805b..6d2993e2c 100644 --- a/src/node/util.ts +++ b/src/node/util.ts @@ -87,20 +87,6 @@ export function getEnvPaths(platform = process.platform): Paths { } } -/** - * humanPath replaces the home directory in path with ~. - * Makes it more readable. - * - * @param homedir - the home directory(i.e. `os.homedir()`) - * @param path - a file path - */ -export function humanPath(homedir: string, path?: string): string { - if (!path) { - return "" - } - return path.replace(homedir, "~") -} - export const generateCertificate = async (hostname: string): Promise<{ cert: string; certKey: string }> => { const certPath = path.join(paths.data, `${hostname.replace(/\./g, "_")}.crt`) const certKeyPath = path.join(paths.data, `${hostname.replace(/\./g, "_")}.key`) diff --git a/test/unit/node/util.test.ts b/test/unit/node/util.test.ts index dcb790209..572c512a0 100644 --- a/test/unit/node/util.test.ts +++ b/test/unit/node/util.test.ts @@ -491,22 +491,6 @@ describe("isDirectory", () => { }) }) -describe("humanPath", () => { - it("should return an empty string if no path provided", () => { - const mockHomedir = "/home/coder" - const actual = util.humanPath(mockHomedir) - const expected = "" - expect(actual).toBe(expected) - }) - it("should replace the homedir with ~", () => { - const mockHomedir = "/home/coder" - const path = `${mockHomedir}/code-server` - const actual = util.humanPath(mockHomedir, path) - const expected = "~/code-server" - expect(actual).toBe(expected) - }) -}) - describe("isWsl", () => { const testName = "wsl"