From 1b60ef418c6343e00ec5fe5d6489f825c5ae02a3 Mon Sep 17 00:00:00 2001 From: Teffen Date: Wed, 10 Nov 2021 00:28:31 -0500 Subject: [PATCH] Use upstream server (#4414) * Flesh out fixes to align with upstream. * Update route handlers to better reflect fallback behavior. * Add platform to vscode-reh-web task Our strategy has been to build once and then recompile native modules for individual platforms. It looks like VS Code builds from scratch for each platform. But we can target any platform, grab the pre-packaged folder, then continue with own packaging. In the future we may want to rework to match upstream. * Fix issue where workspace args are not parsed. * Fix issues surrounding opening files within code-server's terminal. * Readd parent wrapper for hot reload. * Allow more errors. * Fix issues surrounding Coder link. * Add dir creation and fix cli It seems VS Code explodes when certain directories do not exist so import the reh agent instead of the server component since it creates the directories (require patching thus the VS Code update). Also the CLI (for installing extensions) did not seem to be working so point that to the same place since it also exports a function for running that part of the CLI. * Remove hardcoded VSCODE_DEV=1 This causes VS Code to use the development HTML file. Move this to the watch command instead. I deleted the other stuff before it as well since in the latest main.js they do not have this code so I figure we should be safe to omit it. * Fix mismatching commit between client and server * Mostly restore command-line parity Restore most everything and remove the added server arguments. This will let us add and remove options after later so we can contain the number of breaking changes. To accomplish this a hard separation is added between the CLI arguments and the server arguments. The separation between user-provided arguments and arguments with defaults is also made more clear. The extra directory flags have been left out as they were buggy and should be implemented upstream although I think there are better solutions anyway. locale and install-source are unsupported with the web remote and are left removed. It is unclear whether they were used before anyway. Some restored flags still need to have their behavior re-implemented. * Fix static endpoint not emitting 404s This fixes the last failing unit test. Fix a missing dependency, add some generic reverse proxy support for the protocol, and add back a missing nfpm fix. * Import missing logError * Fix 403 errors * Add code-server version to about dialog * Use user settings to disable welcome page The workspace setting seems to be recognized but if so it is having no effect. * Update VS Code cache step with new build directories Co-authored-by: Asher --- .github/workflows/ci.yaml | 6 +- ci/build/build-release.sh | 4 +- ci/build/build-vscode.sh | 5 +- package.json | 2 +- src/node/cli.ts | 147 +++++++++++++++--------- src/node/constants.ts | 1 + src/node/entry.ts | 29 ++++- src/node/main.ts | 69 +++++------- src/node/routes/errors.ts | 2 +- src/node/routes/index.ts | 1 + src/node/routes/vscode.ts | 62 ++++------- src/node/util.ts | 8 ++ test/e2e/models/CodeServer.ts | 7 +- test/unit/node/app.test.ts | 5 - test/unit/node/cli.test.ts | 204 ++++++++++++++-------------------- test/unit/node/plugin.test.ts | 2 + vendor/package.json | 2 +- vendor/yarn.lock | 4 +- 18 files changed, 285 insertions(+), 275 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index c0b1d6508..6f69b6383 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -142,9 +142,9 @@ jobs: path: | vendor/modules/code-oss-dev/.build vendor/modules/code-oss-dev/out-build - vendor/modules/code-oss-dev/out-vscode-server - vendor/modules/code-oss-dev/out-vscode-server-min - key: vscode-server-build-${{ steps.vscode-rev.outputs.rev }} + vendor/modules/code-oss-dev/out-vscode-reh-web + vendor/modules/code-oss-dev/out-vscode-reh-web-min + key: vscode-reh-build-${{ steps.vscode-rev.outputs.rev }} - name: Build vscode if: steps.cache-vscode.outputs.cache-hit != 'true' diff --git a/ci/build/build-release.sh b/ci/build/build-release.sh index 1981215ac..7a13ded94 100755 --- a/ci/build/build-release.sh +++ b/ci/build/build-release.sh @@ -67,7 +67,7 @@ EOF bundle_vscode() { mkdir -p "$VSCODE_OUT_PATH" rsync "$VSCODE_SRC_PATH/yarn.lock" "$VSCODE_OUT_PATH" - rsync "$VSCODE_SRC_PATH/out-vscode-server${MINIFY:+-min}/" "$VSCODE_OUT_PATH/out" + rsync "$VSCODE_SRC_PATH/out-vscode-reh-web${MINIFY:+-min}/" "$VSCODE_OUT_PATH/out" rsync "$VSCODE_SRC_PATH/.build/extensions/" "$VSCODE_OUT_PATH/extensions" if [ "$KEEP_MODULES" = 0 ]; then @@ -88,7 +88,7 @@ bundle_vscode() { cat << EOF { "enableTelemetry": true, - "commit": "$(git rev-parse HEAD)", + "commit": "$(cd "$VSCODE_SRC_PATH" && git rev-parse HEAD)", "quality": "stable", "date": $(jq -n 'now | todate') } diff --git a/ci/build/build-vscode.sh b/ci/build/build-vscode.sh index 59bd6759b..be996fcee 100755 --- a/ci/build/build-vscode.sh +++ b/ci/build/build-vscode.sh @@ -11,9 +11,8 @@ main() { cd vendor/modules/code-oss-dev - # extensions-ci compiles extensions and includes their media. - # compile-web compiles web extensions. TODO: Unsure if used. - yarn gulp extensions-ci compile-web "vscode-server${MINIFY:+-min}" + # Any platform works since we have our own packaging step (for now). + yarn gulp "vscode-reh-web-linux-x64${MINIFY:+-min}" } main "$@" diff --git a/package.json b/package.json index dd7796df6..2ed79453d 100644 --- a/package.json +++ b/package.json @@ -28,7 +28,7 @@ "lint": "./ci/dev/lint.sh", "test": "echo 'Run yarn test:unit or yarn test:e2e' && exit 1", "ci": "./ci/dev/ci.sh", - "watch": "VSCODE_IPC_HOOK_CLI= NODE_OPTIONS='--max_old_space_size=32384 --trace-warnings' ts-node ./ci/dev/watch.ts", + "watch": "VSCODE_DEV=1 VSCODE_IPC_HOOK_CLI= NODE_OPTIONS='--max_old_space_size=32384 --trace-warnings' ts-node ./ci/dev/watch.ts", "icons": "./ci/dev/gen_icons.sh", "coverage": "codecov" }, diff --git a/src/node/cli.ts b/src/node/cli.ts index 3122fb004..d9bbca0e2 100644 --- a/src/node/cli.ts +++ b/src/node/cli.ts @@ -3,7 +3,15 @@ import { promises as fs } from "fs" import yaml from "js-yaml" import * as os from "os" import * as path from "path" -import { canConnect, generateCertificate, generatePassword, humanPath, paths, isNodeJSErrnoException } from "./util" +import { + canConnect, + generateCertificate, + generatePassword, + humanPath, + paths, + isNodeJSErrnoException, + isFile, +} from "./util" const DEFAULT_SOCKET_PATH = path.join(os.tmpdir(), "vscode-ipc") @@ -31,27 +39,13 @@ export enum LogLevel { export class OptionalString extends Optional {} -export interface Args - extends Pick< - CodeServerLib.NativeParsedArgs, - | "_" - | "user-data-dir" - | "enable-proposed-api" - | "extensions-dir" - | "builtin-extensions-dir" - | "extra-extensions-dir" - | "extra-builtin-extensions-dir" - | "ignore-last-opened" - | "locale" - | "log" - | "verbose" - | "install-source" - | "list-extensions" - | "install-extension" - | "uninstall-extension" - | "locate-extension" - // | "telemetry" - > { +/** + * Arguments that the user explicitly provided on the command line. All + * arguments must be optional. + * + * For arguments with defaults see DefaultedArgs. + */ +export interface UserProvidedArgs { config?: string auth?: AuthType password?: string @@ -59,25 +53,39 @@ export interface Args cert?: OptionalString "cert-host"?: string "cert-key"?: string - "disable-telemetry"?: boolean "disable-update-check"?: boolean enable?: string[] help?: boolean host?: string + port?: number json?: boolean log?: LogLevel open?: boolean - port?: number "bind-addr"?: string socket?: string version?: boolean - force?: boolean - "show-versions"?: boolean "proxy-domain"?: string[] "reuse-window"?: boolean "new-window"?: boolean - + "ignore-last-opened"?: boolean link?: OptionalString + verbose?: boolean + /* Positional arguments. */ + _?: string[] + + // VS Code flags. + "disable-telemetry"?: boolean + force?: boolean + "user-data-dir"?: string + "enable-proposed-api"?: string[] + "extensions-dir"?: string + "builtin-extensions-dir"?: string + "install-extension"?: string[] + "uninstall-extension"?: string[] + "list-extensions"?: boolean + "locate-extension"?: string[] + "show-versions"?: boolean + category?: string } interface Option { @@ -121,7 +129,7 @@ type Options = { [P in keyof T]: Option> } -const options: Options> = { +const options: Options> = { auth: { type: AuthType, description: "The type of authentication to use." }, password: { type: "string", @@ -178,12 +186,10 @@ const options: Options> = { "user-data-dir": { type: "string", path: true, description: "Path to the user data directory." }, "extensions-dir": { type: "string", path: true, description: "Path to the extensions directory." }, "builtin-extensions-dir": { type: "string", path: true }, - "extra-extensions-dir": { type: "string[]", path: true }, - "extra-builtin-extensions-dir": { type: "string[]", path: true }, "list-extensions": { type: "boolean", description: "List installed VS Code extensions." }, force: { type: "boolean", description: "Avoid prompts when installing VS Code extensions." }, - "install-source": { type: "string" }, "locate-extension": { type: "string[]" }, + category: { type: "string" }, "install-extension": { type: "string[]", description: @@ -214,7 +220,6 @@ const options: Options> = { description: "Force to open a file or folder in an already opened window.", }, - locale: { type: "string" }, log: { type: LogLevel }, verbose: { type: "boolean", short: "vvv", description: "Enable verbose logging." }, @@ -271,12 +276,16 @@ export function splitOnFirstEquals(str: string): string[] { return split } +/** + * Parse arguments into UserProvidedArgs. This should not go beyond checking + * that arguments are valid types and have values when required. + */ export const parse = ( argv: string[], opts?: { configFile?: string }, -): Args => { +): UserProvidedArgs => { const error = (msg: string): Error => { if (opts?.configFile) { msg = `error reading ${opts.configFile}: ${msg}` @@ -285,7 +294,7 @@ export const parse = ( return new Error(msg) } - const args: Args = { _: [] } + const args: UserProvidedArgs = {} let ended = false for (let i = 0; i < argv.length; ++i) { @@ -299,17 +308,17 @@ export const parse = ( // Options start with a dash and require a value if non-boolean. if (!ended && arg.startsWith("-")) { - let key: keyof Args | undefined + let key: keyof UserProvidedArgs | undefined let value: string | undefined if (arg.startsWith("--")) { const split = splitOnFirstEquals(arg.replace(/^--/, "")) - key = split[0] as keyof Args + key = split[0] as keyof UserProvidedArgs value = split[1] } else { const short = arg.replace(/^-/, "") const pair = Object.entries(options).find(([, v]) => v.short === short) if (pair) { - key = pair[0] as keyof Args + key = pair[0] as keyof UserProvidedArgs } } @@ -384,6 +393,10 @@ export const parse = ( } // Everything else goes into _. + if (typeof args._ === "undefined") { + args._ = [] + } + args._.push(arg) } @@ -397,6 +410,11 @@ export const parse = ( return args } +/** + * User-provided arguments with defaults. The distinction between user-provided + * args and defaulted args exists so we can tell the difference between end + * values and what the user actually provided on the command line. + */ export interface DefaultedArgs extends ConfigArgs { auth: AuthType cert?: { @@ -410,6 +428,10 @@ export interface DefaultedArgs extends ConfigArgs { usingEnvHashedPassword: boolean "extensions-dir": string "user-data-dir": string + /* Positional arguments. */ + _: [] + folder: string + workspace: string } /** @@ -417,7 +439,7 @@ export interface DefaultedArgs extends ConfigArgs { * with the defaults set. Arguments from the CLI are prioritized over config * arguments. */ -export async function setDefaults(cliArgs: Args, configArgs?: ConfigArgs): Promise { +export async function setDefaults(cliArgs: UserProvidedArgs, configArgs?: ConfigArgs): Promise { const args = Object.assign({}, configArgs || {}, cliArgs) if (!args["user-data-dir"]) { @@ -472,7 +494,7 @@ export async function setDefaults(cliArgs: Args, configArgs?: ConfigArgs): Promi args.auth = AuthType.Password } - const addr = bindAddrFromAllSources(configArgs || { _: [] }, cliArgs) + const addr = bindAddrFromAllSources(configArgs || {}, cliArgs) args.host = addr.host args.port = addr.port @@ -513,8 +535,29 @@ export async function setDefaults(cliArgs: Args, configArgs?: ConfigArgs): Promi const proxyDomains = new Set((args["proxy-domain"] || []).map((d) => d.replace(/^\*\./, ""))) args["proxy-domain"] = Array.from(proxyDomains) + if (typeof args._ === "undefined") { + args._ = [] + } + + let workspace = "" + let folder = "" + if (args._.length) { + const lastEntry = path.resolve(process.cwd(), args._[args._.length - 1]) + const entryIsFile = await isFile(lastEntry) + + if (entryIsFile && path.extname(lastEntry) === ".code-workspace") { + workspace = lastEntry + args._.pop() + } else if (!entryIsFile) { + folder = lastEntry + args._.pop() + } + } + return { ...args, + workspace, + folder, usingEnvPassword, usingEnvHashedPassword, } as DefaultedArgs // TODO: Technically no guarantee this is fulfilled. @@ -539,7 +582,7 @@ cert: false ` } -interface ConfigArgs extends Args { +interface ConfigArgs extends UserProvidedArgs { config: string } @@ -581,7 +624,7 @@ export async function readConfigFile(configPath?: string): Promise { */ export function parseConfigFile(configFile: string, configPath: string): ConfigArgs { if (!configFile) { - return { _: [], config: configPath } + return { config: configPath } } const config = yaml.load(configFile, { @@ -628,7 +671,7 @@ interface Addr { * This function creates the bind address * using the CLI args. */ -export function bindAddrFromArgs(addr: Addr, args: Args): Addr { +export function bindAddrFromArgs(addr: Addr, args: UserProvidedArgs): Addr { addr = { ...addr } if (args["bind-addr"]) { addr = parseBindAddr(args["bind-addr"]) @@ -646,7 +689,7 @@ export function bindAddrFromArgs(addr: Addr, args: Args): Addr { return addr } -function bindAddrFromAllSources(...argsConfig: Args[]): Addr { +function bindAddrFromAllSources(...argsConfig: UserProvidedArgs[]): Addr { let addr: Addr = { host: "localhost", port: 8080, @@ -683,30 +726,34 @@ export async function readSocketPath(path: string): Promise /** * Determine if it looks like the user is trying to open a file or folder in an * existing instance. The arguments here should be the arguments the user - * explicitly passed on the command line, not defaults or the configuration. + * explicitly passed on the command line, *NOT DEFAULTS* or the configuration. */ -export const shouldOpenInExistingInstance = async (args: Args): Promise => { +export const shouldOpenInExistingInstance = async (args: UserProvidedArgs): Promise => { // Always use the existing instance if we're running from VS Code's terminal. if (process.env.VSCODE_IPC_HOOK_CLI) { + logger.debug("Found VSCODE_IPC_HOOK_CLI") return process.env.VSCODE_IPC_HOOK_CLI } // If these flags are set then assume the user is trying to open in an // existing instance since these flags have no effect otherwise. const openInFlagCount = ["reuse-window", "new-window"].reduce((prev, cur) => { - return args[cur as keyof Args] ? prev + 1 : prev + return args[cur as keyof UserProvidedArgs] ? prev + 1 : prev }, 0) if (openInFlagCount > 0) { + logger.debug("Found --reuse-window or --new-window") return readSocketPath(DEFAULT_SOCKET_PATH) } // It's possible the user is trying to spawn another instance of code-server. - // Check if any unrelated flags are set (check against one because `_` always - // exists), that a file or directory was passed, and that the socket is - // active. - if (Object.keys(args).length === 1 && args._.length > 0) { + // 1. Check if any unrelated flags are set (this should only run when + // code-server is invoked exactly like this: `code-server my-file`). + // 2. That a file or directory was passed. + // 3. That the socket is active. + if (Object.keys(args).length === 1 && typeof args._ !== "undefined" && args._.length > 0) { const socketPath = await readSocketPath(DEFAULT_SOCKET_PATH) if (socketPath && (await canConnect(socketPath))) { + logger.debug("Found existing code-server socket") return socketPath } } diff --git a/src/node/constants.ts b/src/node/constants.ts index 343457a54..b2e53dc90 100644 --- a/src/node/constants.ts +++ b/src/node/constants.ts @@ -18,6 +18,7 @@ export function getPackageJson(relativePath: string): JSONSchemaForNPMPackageJso const pkg = getPackageJson("../../package.json") +export const pkgName = pkg.name || "code-server" export const version = pkg.version || "development" export const commit = pkg.commit || "development" export const rootPath = path.resolve(__dirname, "../..") diff --git a/src/node/entry.ts b/src/node/entry.ts index 06ce4cccf..685f62f36 100644 --- a/src/node/entry.ts +++ b/src/node/entry.ts @@ -1,13 +1,32 @@ import { logger } from "@coder/logger" import { optionDescriptions, parse, readConfigFile, setDefaults, shouldOpenInExistingInstance } from "./cli" -import { commit, version } from "./constants" +import { commit, pkgName, version } from "./constants" import { openInExistingInstance, runCodeServer, runVsCodeCli, shouldSpawnCliProcess } from "./main" import { monkeyPatchProxyProtocols } from "./proxy_agent" +import { loadAMDModule } from "./util" import { isChild, wrapper } from "./wrapper" +const cliPipe = process.env["VSCODE_IPC_HOOK_CLI"] as string +const cliCommand = process.env["VSCODE_CLIENT_COMMAND"] as string + async function entry(): Promise { monkeyPatchProxyProtocols() + if (cliPipe || cliCommand) { + const remoteAgentMain = await loadAMDModule("vs/server/remoteCli", "main") + + remoteAgentMain( + { + productName: pkgName, + version, + commit, + executableName: pkgName, + }, + process.argv.slice(2), + ) + return + } + // There's no need to check flags like --help or to spawn in an existing // instance for the child process because these would have already happened in // the parent and the child wouldn't have been spawned. We also get the @@ -30,6 +49,8 @@ async function entry(): Promise { console.log("code-server", version, commit) console.log("") console.log(`Usage: code-server [options] [path]`) + console.log(` - Opening a directory: code-server ./path/to/your/project`) + console.log(` - Opening a saved workspace: code-server ./path/to/your/project.code-workspace`) console.log("") console.log("Options") optionDescriptions().forEach((description) => { @@ -53,12 +74,14 @@ async function entry(): Promise { return } - if (await shouldSpawnCliProcess(args)) { - return runVsCodeCli() + if (shouldSpawnCliProcess(args)) { + logger.debug("Found VS Code arguments; spawning VS Code CLI") + return runVsCodeCli(args) } const socketPath = await shouldOpenInExistingInstance(cliArgs) if (socketPath) { + logger.debug("Trying to open in existing instance") return openInExistingInstance(args, socketPath) } diff --git a/src/node/main.ts b/src/node/main.ts index 698f3e1a6..a4dfcee69 100644 --- a/src/node/main.ts +++ b/src/node/main.ts @@ -4,19 +4,22 @@ import path from "path" import { Disposable } from "../common/emitter" import { plural } from "../common/util" import { createApp, ensureAddress } from "./app" -import { AuthType, DefaultedArgs, Feature } from "./cli" +import { AuthType, DefaultedArgs, Feature, UserProvidedArgs } from "./cli" import { coderCloudBind } from "./coder_cloud" -import { commit, version, vsRootPath } from "./constants" +import { commit, version } from "./constants" import { register } from "./routes" import { humanPath, isFile, loadAMDModule, open } from "./util" -export const shouldSpawnCliProcess = async (args: CodeServerLib.NativeParsedArgs): Promise => { - const shouldSpawn = await loadAMDModule<(argv: CodeServerLib.NativeParsedArgs) => boolean>( - "vs/code/node/cli", - "shouldSpawnCliProcess", +/** + * Return true if the user passed an extension-related VS Code flag. + */ +export const shouldSpawnCliProcess = (args: UserProvidedArgs): boolean => { + return ( + !!args["list-extensions"] || + !!args["install-extension"] || + !!args["uninstall-extension"] || + !!args["locate-extension"] ) - - return shouldSpawn(args) } /** @@ -24,37 +27,18 @@ export const shouldSpawnCliProcess = async (args: CodeServerLib.NativeParsedArgs * such as when managing extensions. * @deprecated This should be removed when code-server merges with lib/vscode. */ -export const runVsCodeCli = async (): Promise => { +export const runVsCodeCli = async (args: DefaultedArgs): Promise => { logger.debug("Running VS Code CLI") - // Delete `VSCODE_CWD` very early even before - // importing bootstrap files. We have seen - // reports where `code .` would use the wrong - // current working directory due to our variable - // somehow escaping to the parent shell - // (https://github.com/microsoft/vscode/issues/126399) - delete process.env["VSCODE_CWD"] - - const bootstrap = require(path.join(vsRootPath, "out", "bootstrap")) - const bootstrapNode = require(path.join(vsRootPath, "out", "bootstrap-node")) - const product = require(path.join(vsRootPath, "product.json")) - - // Avoid Monkey Patches from Application Insights - bootstrap.avoidMonkeyPatchFromAppInsights() - - // Enable portable support - bootstrapNode.configurePortable(product) - - // Enable ASAR support - bootstrap.enableASARSupport() - - // Signal processes that we got launched as CLI - process.env["VSCODE_CLI"] = "1" - - const cliProcessMain = await loadAMDModule("vs/code/node/cli", "initialize") + // See ../../vendor/modules/code-oss-dev/src/vs/server/main.js. + const spawnCli = await loadAMDModule("vs/server/remoteExtensionHostAgent", "spawnCli") try { - await cliProcessMain(process.argv) + await spawnCli({ + ...args, + // For some reason VS Code takes the port as a string. + port: typeof args.port !== "undefined" ? args.port.toString() : undefined, + }) } catch (error: any) { logger.error("Got error from VS Code", error) } @@ -70,8 +54,9 @@ export const openInExistingInstance = async (args: DefaultedArgs, socketPath: st forceReuseWindow: args["reuse-window"], forceNewWindow: args["new-window"], } - for (let i = 0; i < args._.length; i++) { - const fp = path.resolve(args._[i]) + const paths = args._ || [] + for (let i = 0; i < paths.length; i++) { + const fp = path.resolve(paths[i]) if (await isFile(fp)) { pipeArgs.fileURIs.push(fp) } else { @@ -120,11 +105,17 @@ export const runCodeServer = async ( } const app = await createApp(args) - const serverAddress = ensureAddress(app.server, args.cert ? "https" : "http") + const protocol = args.cert ? "https" : "http" + const serverAddress = ensureAddress(app.server, protocol) const disposeRoutes = await register(app, args) logger.info(`Using config file ${humanPath(args.config)}`) - logger.info(`HTTP server listening on ${serverAddress.toString()} ${args.link ? "(randomized by --link)" : ""}`) + logger.info( + `${protocol.toUpperCase()} server listening on ${serverAddress.toString()} ${ + args.link ? "(randomized by --link)" : "" + }`, + ) + if (args.auth === AuthType.Password) { logger.info(" - Authentication is enabled") if (args.usingEnvPassword) { diff --git a/src/node/routes/errors.ts b/src/node/routes/errors.ts index 66f424ac2..32e8ab532 100644 --- a/src/node/routes/errors.ts +++ b/src/node/routes/errors.ts @@ -8,7 +8,7 @@ import { rootPath } from "../constants" import { replaceTemplates } from "../http" import { escapeHtml, getMediaMime } from "../util" -const notFoundCodes = ["ENOENT", "EISDIR", "FileNotFound"] +const notFoundCodes = ["ENOENT", "EISDIR"] export const errorHandler: express.ErrorRequestHandler = async (err, req, res, next) => { if (notFoundCodes.includes(err.code)) { err.status = HttpCode.NotFound diff --git a/src/node/routes/index.ts b/src/node/routes/index.ts index b02661841..7ca79fa7c 100644 --- a/src/node/routes/index.ts +++ b/src/node/routes/index.ts @@ -121,6 +121,7 @@ export const register = async (app: App, args: DefaultedArgs): Promise => { - // Delete `VSCODE_CWD` very early even before - // importing bootstrap files. We have seen - // reports where `code .` would use the wrong - // current working directory due to our variable - // somehow escaping to the parent shell - // (https://github.com/microsoft/vscode/issues/126399) - delete process.env["VSCODE_CWD"] + // See ../../../vendor/modules/code-oss-dev/src/vs/server/main.js. + const createVSServer = await loadAMDModule( + "vs/server/remoteExtensionHostAgent", + "createServer", + ) - const bootstrap = require(path.join(vsRootPath, "out", "bootstrap")) - const bootstrapNode = require(path.join(vsRootPath, "out", "bootstrap-node")) - const product = require(path.join(vsRootPath, "product.json")) - - // Avoid Monkey Patches from Application Insights - bootstrap.avoidMonkeyPatchFromAppInsights() - - // Enable portable support - bootstrapNode.configurePortable(product) - - // Enable ASAR support - bootstrap.enableASARSupport() - - // Signal processes that we got launched as CLI - process.env["VSCODE_CLI"] = "1" - - const createVSServer = await loadAMDModule("vs/server/entry", "createVSServer") - - const serverUrl = new URL(`${args.cert ? "https" : "http"}://${args.host}:${args.port}`) - const codeServerMain = await createVSServer({ - codeServerVersion, - serverUrl, - args, - authed: args.auth !== AuthType.None, - disableUpdateCheck: !!args["disable-update-check"], + const codeServerMain = await createVSServer(null, { + connectionToken: "0000", + ...args, + // For some reason VS Code takes the port as a string. + port: typeof args.port !== "undefined" ? args.port.toString() : undefined, }) - const netServer = await codeServerMain.startup({ listenWhenReady: false }) - const router = express.Router() const wsRouter = WsRouter() @@ -66,13 +40,19 @@ export const createVSServerRouter = async (args: DefaultedArgs): Promise { - req.on("error", (error) => errorHandler(error, req, res, next)) + req.on("error", (error: any) => { + if (error instanceof Error && ["EntryNotFound", "FileNotFound", "HttpError"].includes(error.message)) { + next() + } - netServer.emit("request", req, res) + errorHandler(error, req, res, next) + }) + + codeServerMain.handleRequest(req, res) }) wsRouter.ws("/", ensureAuthenticated, (req) => { - netServer.emit("upgrade", req, req.socket, req.head) + codeServerMain.handleUpgrade(req, req.socket) req.socket.resume() }) diff --git a/src/node/util.ts b/src/node/util.ts index 37369d91c..a86f2d0fc 100644 --- a/src/node/util.ts +++ b/src/node/util.ts @@ -506,6 +506,14 @@ type AMDModule = { [exportName: string]: T } * @param exportName Given name of export in the file */ export const loadAMDModule = async (amdPath: string, exportName: string): Promise => { + // Set default remote native node modules path, if unset + process.env["VSCODE_INJECT_NODE_MODULE_LOOKUP_PATH"] = + process.env["VSCODE_INJECT_NODE_MODULE_LOOKUP_PATH"] || path.join(vsRootPath, "remote", "node_modules") + + require(path.join(vsRootPath, "out/bootstrap-node")).injectNodeModuleLookupPath( + process.env["VSCODE_INJECT_NODE_MODULE_LOOKUP_PATH"], + ) + const module = await new Promise>((resolve, reject) => { require(path.join(vsRootPath, "out/bootstrap-amd")).load(amdPath, resolve, reject) }) diff --git a/test/e2e/models/CodeServer.ts b/test/e2e/models/CodeServer.ts index 7f51e84bf..12d6ef461 100644 --- a/test/e2e/models/CodeServer.ts +++ b/test/e2e/models/CodeServer.ts @@ -3,6 +3,7 @@ import * as cp from "child_process" import { promises as fs } from "fs" import * as path from "path" import { Page } from "playwright" +import { logError } from "../../../src/common/util" import { onLine } from "../../../src/node/util" import { PASSWORD, workspaceDir } from "../../utils/constants" import { idleTimer, tmpdir } from "../../utils/helpers" @@ -51,9 +52,9 @@ export class CodeServer { */ private async createWorkspace(): Promise { const dir = await tmpdir(workspaceDir) - await fs.mkdir(path.join(dir, ".vscode")) + await fs.mkdir(path.join(dir, "User")) await fs.writeFile( - path.join(dir, ".vscode/settings.json"), + path.join(dir, "User/settings.json"), JSON.stringify({ "workbench.startupEditor": "none", }), @@ -129,7 +130,7 @@ export class CodeServer { if (resolved) { return } - const match = line.trim().match(/HTTP server listening on (https?:\/\/[.:\d]+)\/?$/) + const match = line.trim().match(/HTTPS? server listening on (https?:\/\/[.:\d]+)\/?$/) if (match) { // Cookies don't seem to work on IP address so swap to localhost. // TODO: Investigate whether this is a bug with code-server. diff --git a/test/unit/node/app.test.ts b/test/unit/node/app.test.ts index 5f8e04a5a..99c67cd2a 100644 --- a/test/unit/node/app.test.ts +++ b/test/unit/node/app.test.ts @@ -45,7 +45,6 @@ describe("createApp", () => { it("should return an Express app, a WebSockets Express app and an http server", async () => { const defaultArgs = await setDefaults({ port, - _: [], }) const app = await createApp(defaultArgs) @@ -62,7 +61,6 @@ describe("createApp", () => { it("should handle error events on the server", async () => { const defaultArgs = await setDefaults({ port, - _: [], }) const app = await createApp(defaultArgs) @@ -85,7 +83,6 @@ describe("createApp", () => { const port = 2 const defaultArgs = await setDefaults({ port, - _: [], }) async function masterBall() { @@ -105,7 +102,6 @@ describe("createApp", () => { it("should unlink a socket before listening on the socket", async () => { await promises.writeFile(tmpFilePath, "") const defaultArgs = await setDefaults({ - _: [], socket: tmpFilePath, }) @@ -121,7 +117,6 @@ describe("createApp", () => { const defaultArgs = await setDefaults({ port, cert, - _: [], ["cert-key"]: testCertificate.certKey, }) const app = await createApp(defaultArgs) diff --git a/test/unit/node/cli.test.ts b/test/unit/node/cli.test.ts index 44987eed9..0dcf24f4b 100644 --- a/test/unit/node/cli.test.ts +++ b/test/unit/node/cli.test.ts @@ -4,23 +4,19 @@ import * as net from "net" import * as os from "os" import * as path from "path" import { - Args, + UserProvidedArgs, bindAddrFromArgs, defaultConfigFile, parse, + readSocketPath, setDefaults, shouldOpenInExistingInstance, splitOnFirstEquals, - readSocketPath, } from "../../../src/node/cli" import { shouldSpawnCliProcess } from "../../../src/node/main" import { generatePassword, paths } from "../../../src/node/util" import { useEnv, tmpdir } from "../../utils/helpers" -type Mutable = { - -readonly [P in keyof T]: T[P] -} - describe("parser", () => { beforeEach(() => { delete process.env.LOG_LEVEL @@ -40,66 +36,71 @@ describe("parser", () => { usingEnvHashedPassword: false, "extensions-dir": path.join(paths.data, "extensions"), "user-data-dir": paths.data, + _: [], + workspace: "", + folder: "", } - it("should parse nothing", () => { - expect(parse([])).toStrictEqual({ _: [] }) + it("should parse nothing", async () => { + expect(parse([])).toStrictEqual({}) }) - it("should parse all available options", () => { + it("should parse all available options", async () => { expect( - parse([ - "--enable", - "feature1", - "--enable", - "feature2", - "--bind-addr=192.169.0.1:8080", - "--auth", - "none", - "--extensions-dir", - "foo", - "--builtin-extensions-dir", - "foobar", - "--extra-extensions-dir", - "nozzle", - "1", - "--extra-builtin-extensions-dir", - "bazzle", - "--verbose", - "2", - "--log", - "error", - "--help", - "--open", - "--socket=mumble", - "3", - "--user-data-dir", - "bar", - "--cert=baz", - "--cert-key", - "qux", - "--version", - "--json", - "--port=8081", - "--host", - "0.0.0.0", - "4", - "--", - "-5", - "--6", - ]), + parse( + [ + ["--enable", "feature1"], + ["--enable", "feature2"], + + "--bind-addr=192.169.0.1:8080", + + ["--auth", "none"], + + ["--extensions-dir", "path/to/ext/dir"], + + ["--builtin-extensions-dir", "path/to/builtin/ext/dir"], + + "1", + "--verbose", + "2", + + ["--log", "error"], + + "--help", + + "--open", + + "--socket=mumble", + + "3", + + ["--user-data-dir", "path/to/user/dir"], + + ["--cert=path/to/cert", "--cert-key", "path/to/cert/key"], + + "--version", + + "--json", + + "--port=8081", + + ["--host", "0.0.0.0"], + "4", + "--", + "--5", + ].flat(), + ), ).toEqual({ - _: ["1", "2", "3", "4", "-5", "--6"], + _: ["1", "2", "3", "4", "--5"], auth: "none", - "builtin-extensions-dir": path.resolve("foobar"), - "cert-key": path.resolve("qux"), + "builtin-extensions-dir": path.resolve("path/to/builtin/ext/dir"), + "extensions-dir": path.resolve("path/to/ext/dir"), + "user-data-dir": path.resolve("path/to/user/dir"), + "cert-key": path.resolve("path/to/cert/key"), cert: { - value: path.resolve("baz"), + value: path.resolve("path/to/cert"), }, enable: ["feature1", "feature2"], - "extensions-dir": path.resolve("foo"), - "extra-builtin-extensions-dir": [path.resolve("bazzle")], - "extra-extensions-dir": [path.resolve("nozzle")], help: true, host: "0.0.0.0", json: true, @@ -107,16 +108,14 @@ describe("parser", () => { open: true, port: 8081, socket: path.resolve("mumble"), - "user-data-dir": path.resolve("bar"), verbose: true, version: true, "bind-addr": "192.169.0.1:8080", }) }) - it("should work with short options", () => { + it("should work with short options", async () => { expect(parse(["-vvv", "-v"])).toEqual({ - _: [], verbose: true, version: true, }) @@ -124,13 +123,12 @@ describe("parser", () => { it("should use log level env var", async () => { const args = parse([]) - expect(args).toEqual({ _: [] }) + expect(args).toEqual({}) process.env.LOG_LEVEL = "debug" const defaults = await setDefaults(args) expect(defaults).toStrictEqual({ ...defaults, - _: [], log: "debug", verbose: false, }) @@ -141,7 +139,6 @@ describe("parser", () => { const updated = await setDefaults(args) expect(updated).toStrictEqual({ ...updated, - _: [], log: "trace", verbose: true, }) @@ -152,7 +149,6 @@ describe("parser", () => { it("should prefer --log to env var and --verbose to --log", async () => { let args = parse(["--log", "info"]) expect(args).toEqual({ - _: [], log: "info", }) @@ -160,7 +156,6 @@ describe("parser", () => { const defaults = await setDefaults(args) expect(defaults).toEqual({ ...defaults, - _: [], log: "info", verbose: false, }) @@ -171,7 +166,6 @@ describe("parser", () => { const updated = await setDefaults(args) expect(updated).toEqual({ ...defaults, - _: [], log: "info", verbose: false, }) @@ -180,7 +174,6 @@ describe("parser", () => { args = parse(["--log", "info", "--verbose"]) expect(args).toEqual({ - _: [], log: "info", verbose: true, }) @@ -189,7 +182,6 @@ describe("parser", () => { const updatedAgain = await setDefaults(args) expect(updatedAgain).toEqual({ ...defaults, - _: [], log: "trace", verbose: true, }) @@ -202,7 +194,6 @@ describe("parser", () => { const defaults = await setDefaults(parse([])) expect(defaults).toEqual({ ...defaults, - _: [], }) }) @@ -224,9 +215,8 @@ describe("parser", () => { expect(() => parse(["--foo"])).toThrowError(/Unknown option --foo/) }) - it("should not error if the value is optional", () => { + it("should not error if the value is optional", async () => { expect(parse(["--cert"])).toEqual({ - _: [], cert: { value: undefined, }, @@ -237,26 +227,23 @@ describe("parser", () => { expect(() => parse(["--socket", "--socket-path-value"])).toThrowError(/--socket requires a value/) // If you actually had a path like this you would do this instead: expect(parse(["--socket", "./--socket-path-value"])).toEqual({ - _: [], socket: path.resolve("--socket-path-value"), }) expect(() => parse(["--cert", "--socket-path-value"])).toThrowError(/Unknown option --socket-path-value/) }) - it("should allow positional arguments before options", () => { - expect(parse(["foo", "test", "--auth", "none"])).toEqual({ - _: ["foo", "test"], + it("should allow positional arguments before options", async () => { + expect(parse(["test", "--auth", "none"])).toEqual({ + _: ["test"], auth: "none", }) }) - it("should support repeatable flags", () => { + it("should support repeatable flags", async () => { expect(parse(["--proxy-domain", "*.coder.com"])).toEqual({ - _: [], "proxy-domain": ["*.coder.com"], }) expect(parse(["--proxy-domain", "*.coder.com", "--proxy-domain", "test.com"])).toEqual({ - _: [], "proxy-domain": ["*.coder.com", "test.com"], }) }) @@ -264,7 +251,6 @@ describe("parser", () => { it("should enforce cert-key with cert value or otherwise generate one", async () => { const args = parse(["--cert"]) expect(args).toEqual({ - _: [], cert: { value: undefined, }, @@ -272,7 +258,6 @@ describe("parser", () => { expect(() => parse(["--cert", "test"])).toThrowError(/--cert-key is missing/) const defaultArgs = await setDefaults(args) expect(defaultArgs).toEqual({ - _: [], ...defaults, cert: { value: path.join(paths.data, "localhost.crt"), @@ -285,7 +270,6 @@ describe("parser", () => { const args = parse("--cert test --cert-key test --socket test --host 0.0.0.0 --port 8888 --link test".split(" ")) const defaultArgs = await setDefaults(args) expect(defaultArgs).toEqual({ - _: [], ...defaults, auth: "none", host: "localhost", @@ -302,14 +286,11 @@ describe("parser", () => { it("should use env var password", async () => { process.env.PASSWORD = "test" const args = parse([]) - expect(args).toEqual({ - _: [], - }) + expect(args).toEqual({}) const defaultArgs = await setDefaults(args) expect(defaultArgs).toEqual({ ...defaults, - _: [], password: "test", usingEnvPassword: true, }) @@ -319,14 +300,11 @@ describe("parser", () => { process.env.HASHED_PASSWORD = "$argon2i$v=19$m=4096,t=3,p=1$0qR/o+0t00hsbJFQCKSfdQ$oFcM4rL6o+B7oxpuA4qlXubypbBPsf+8L531U7P9HYY" // test const args = parse([]) - expect(args).toEqual({ - _: [], - }) + expect(args).toEqual({}) const defaultArgs = await setDefaults(args) expect(defaultArgs).toEqual({ ...defaults, - _: [], "hashed-password": "$argon2i$v=19$m=4096,t=3,p=1$0qR/o+0t00hsbJFQCKSfdQ$oFcM4rL6o+B7oxpuA4qlXubypbBPsf+8L531U7P9HYY", usingEnvHashedPassword: true, @@ -348,27 +326,23 @@ describe("parser", () => { it("should filter proxy domains", async () => { const args = parse(["--proxy-domain", "*.coder.com", "--proxy-domain", "coder.com", "--proxy-domain", "coder.org"]) expect(args).toEqual({ - _: [], "proxy-domain": ["*.coder.com", "coder.com", "coder.org"], }) const defaultArgs = await setDefaults(args) expect(defaultArgs).toEqual({ ...defaults, - _: [], "proxy-domain": ["coder.com", "coder.org"], }) }) it("should allow '=,$/' in strings", async () => { const args = parse([ - "--enable-proposed-api", + "--disable-update-check", "$argon2i$v=19$m=4096,t=3,p=1$0qr/o+0t00hsbjfqcksfdq$ofcm4rl6o+b7oxpua4qlxubypbbpsf+8l531u7p9hyy", ]) expect(args).toEqual({ - _: [], - "enable-proposed-api": [ - "$argon2i$v=19$m=4096,t=3,p=1$0qr/o+0t00hsbjfqcksfdq$ofcm4rl6o+b7oxpua4qlxubypbbpsf+8l531u7p9hyy", - ], + "disable-update-check": true, + _: ["$argon2i$v=19$m=4096,t=3,p=1$0qr/o+0t00hsbjfqcksfdq$ofcm4rl6o+b7oxpua4qlxubypbbpsf+8l531u7p9hyy"], }) }) it("should parse options with double-dash and multiple equal signs ", async () => { @@ -381,7 +355,6 @@ describe("parser", () => { }, ) expect(args).toEqual({ - _: [], "hashed-password": "$argon2i$v=19$m=4096,t=3,p=1$0qr/o+0t00hsbjfqcksfdq$ofcm4rl6o+b7oxpua4qlxubypbbpsf+8l531u7p9hyy", }) @@ -389,7 +362,6 @@ describe("parser", () => { }) describe("cli", () => { - let args: Mutable = { _: [] } let testDir: string const vscodeIpcPath = path.join(os.tmpdir(), "vscode-ipc") @@ -401,20 +373,21 @@ describe("cli", () => { beforeEach(async () => { delete process.env.VSCODE_IPC_HOOK_CLI - args = { _: [] } await fs.rmdir(vscodeIpcPath, { recursive: true }) }) it("should use existing if inside code-server", async () => { process.env.VSCODE_IPC_HOOK_CLI = "test" + const args: UserProvidedArgs = {} expect(await shouldOpenInExistingInstance(args)).toStrictEqual("test") args.port = 8081 - args._.push("./file") + args._ = ["./file"] expect(await shouldOpenInExistingInstance(args)).toStrictEqual("test") }) it("should use existing if --reuse-window is set", async () => { + const args: UserProvidedArgs = {} args["reuse-window"] = true await expect(shouldOpenInExistingInstance(args)).resolves.toStrictEqual(undefined) @@ -426,6 +399,7 @@ describe("cli", () => { }) it("should use existing if --new-window is set", async () => { + const args: UserProvidedArgs = {} args["new-window"] = true expect(await shouldOpenInExistingInstance(args)).toStrictEqual(undefined) @@ -437,9 +411,10 @@ describe("cli", () => { }) it("should use existing if no unrelated flags are set, has positional, and socket is active", async () => { + const args: UserProvidedArgs = {} expect(await shouldOpenInExistingInstance(args)).toStrictEqual(undefined) - args._.push("./file") + args._ = ["./file"] expect(await shouldOpenInExistingInstance(args)).toStrictEqual(undefined) const socketPath = path.join(testDir, "socket") @@ -489,9 +464,7 @@ describe("splitOnFirstEquals", () => { describe("shouldSpawnCliProcess", () => { it("should return false if no 'extension' related args passed in", async () => { - const args = { - _: [], - } + const args = {} const actual = await shouldSpawnCliProcess(args) const expected = false @@ -500,7 +473,6 @@ describe("shouldSpawnCliProcess", () => { it("should return true if 'list-extensions' passed in", async () => { const args = { - _: [], ["list-extensions"]: true, } const actual = await shouldSpawnCliProcess(args) @@ -511,7 +483,6 @@ describe("shouldSpawnCliProcess", () => { it("should return true if 'install-extension' passed in", async () => { const args = { - _: [], ["install-extension"]: ["hello.world"], } const actual = await shouldSpawnCliProcess(args) @@ -521,8 +492,7 @@ describe("shouldSpawnCliProcess", () => { }) it("should return true if 'uninstall-extension' passed in", async () => { - const args = { - _: [], + const args: UserProvidedArgs = { ["uninstall-extension"]: ["hello.world"], } const actual = await shouldSpawnCliProcess(args) @@ -534,9 +504,7 @@ describe("shouldSpawnCliProcess", () => { describe("bindAddrFromArgs", () => { it("should return the bind address", () => { - const args = { - _: [], - } + const args: UserProvidedArgs = {} const addr = { host: "localhost", @@ -550,8 +518,7 @@ describe("bindAddrFromArgs", () => { }) it("should use the bind-address if set in args", () => { - const args = { - _: [], + const args: UserProvidedArgs = { ["bind-addr"]: "localhost:3000", } @@ -570,8 +537,7 @@ describe("bindAddrFromArgs", () => { }) it("should use the host if set in args", () => { - const args = { - _: [], + const args: UserProvidedArgs = { ["host"]: "coder", } @@ -593,9 +559,7 @@ describe("bindAddrFromArgs", () => { const [setValue, resetValue] = useEnv("PORT") setValue("8000") - const args = { - _: [], - } + const args: UserProvidedArgs = {} const addr = { host: "localhost", @@ -613,8 +577,7 @@ describe("bindAddrFromArgs", () => { }) it("should set port if in args", () => { - const args = { - _: [], + const args: UserProvidedArgs = { port: 3000, } @@ -636,8 +599,7 @@ describe("bindAddrFromArgs", () => { const [setValue, resetValue] = useEnv("PORT") setValue("8000") - const args = { - _: [], + const args: UserProvidedArgs = { port: 3000, } diff --git a/test/unit/node/plugin.test.ts b/test/unit/node/plugin.test.ts index acd417316..5e23c24b6 100644 --- a/test/unit/node/plugin.test.ts +++ b/test/unit/node/plugin.test.ts @@ -42,6 +42,8 @@ describe("plugin", () => { usingEnvHashedPassword: false, "extensions-dir": "", "user-data-dir": "", + workspace: "", + folder: "", } next() } diff --git a/vendor/package.json b/vendor/package.json index d7d800f13..bdf69c897 100644 --- a/vendor/package.json +++ b/vendor/package.json @@ -7,6 +7,6 @@ "postinstall": "./postinstall.sh" }, "devDependencies": { - "code-oss-dev": "cdr/vscode#3fc885904886003d88d1f300d6158bee486f644f" + "code-oss-dev": "cdr/vscode#d62e8db202f80db7a42233cd56d04e6806109fb1" } } diff --git a/vendor/yarn.lock b/vendor/yarn.lock index 3050d752d..10acfc657 100644 --- a/vendor/yarn.lock +++ b/vendor/yarn.lock @@ -296,9 +296,9 @@ clone-response@^1.0.2: dependencies: mimic-response "^1.0.0" -code-oss-dev@cdr/vscode#3fc885904886003d88d1f300d6158bee486f644f: +code-oss-dev@cdr/vscode#d62e8db202f80db7a42233cd56d04e6806109fb1: version "1.61.1" - resolved "https://codeload.github.com/cdr/vscode/tar.gz/3fc885904886003d88d1f300d6158bee486f644f" + resolved "https://codeload.github.com/cdr/vscode/tar.gz/d62e8db202f80db7a42233cd56d04e6806109fb1" dependencies: "@microsoft/applicationinsights-web" "^2.6.4" "@vscode/sqlite3" "4.0.12"