From 2819fd51e2318a6324b8d43a1f18096435e4f916 Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 22 Jul 2020 15:28:33 -0500 Subject: [PATCH 01/10] Remove unused endpoints - dashboard - app api --- ci/build/build-code-server.sh | 1 - ci/dev/watch.ts | 6 +- src/browser/pages/app.html | 28 --- src/browser/pages/app.ts | 37 ---- src/browser/pages/error.html | 2 +- src/browser/pages/home.css | 51 ------ src/browser/pages/home.html | 59 ------- src/browser/pages/login.html | 2 +- src/browser/pages/update.html | 2 +- src/browser/register.ts | 6 +- src/common/api.ts | 60 ------- src/common/http.ts | 8 - src/node/app/api.ts | 312 ---------------------------------- src/node/app/bin.ts | 30 ---- src/node/app/dashboard.ts | 147 ---------------- src/node/entry.ts | 8 +- 16 files changed, 11 insertions(+), 748 deletions(-) delete mode 100644 src/browser/pages/app.html delete mode 100644 src/browser/pages/app.ts delete mode 100644 src/browser/pages/home.css delete mode 100644 src/browser/pages/home.html delete mode 100644 src/common/api.ts delete mode 100644 src/node/app/api.ts delete mode 100644 src/node/app/bin.ts delete mode 100644 src/node/app/dashboard.ts diff --git a/ci/build/build-code-server.sh b/ci/build/build-code-server.sh index a48ab0e0..df528874 100755 --- a/ci/build/build-code-server.sh +++ b/ci/build/build-code-server.sh @@ -21,7 +21,6 @@ main() { --public-url "/static/$(git rev-parse HEAD)/dist" \ --out-dir dist \ $([[ $MINIFY ]] || echo --no-minify) \ - src/browser/pages/app.ts \ src/browser/register.ts \ src/browser/serviceWorker.ts } diff --git a/ci/dev/watch.ts b/ci/dev/watch.ts index 03ce2e42..fd144653 100644 --- a/ci/dev/watch.ts +++ b/ci/dev/watch.ts @@ -144,11 +144,7 @@ class Watcher { private createBundler(out = "dist"): Bundler { return new Bundler( - [ - path.join(this.rootPath, "src/browser/pages/app.ts"), - path.join(this.rootPath, "src/browser/register.ts"), - path.join(this.rootPath, "src/browser/serviceWorker.ts"), - ], + [path.join(this.rootPath, "src/browser/register.ts"), path.join(this.rootPath, "src/browser/serviceWorker.ts")], { outDir: path.join(this.rootPath, out), cacheDir: path.join(this.rootPath, ".cache"), diff --git a/src/browser/pages/app.html b/src/browser/pages/app.html deleted file mode 100644 index 551471a1..00000000 --- a/src/browser/pages/app.html +++ /dev/null @@ -1,28 +0,0 @@ - - - - - - - code-server - - - - - - - - - - - diff --git a/src/browser/pages/app.ts b/src/browser/pages/app.ts deleted file mode 100644 index f7162947..00000000 --- a/src/browser/pages/app.ts +++ /dev/null @@ -1,37 +0,0 @@ -import { getOptions, normalize } from "../../common/util" -import { ApiEndpoint } from "../../common/http" - -import "./error.css" -import "./global.css" -import "./home.css" -import "./login.css" -import "./update.css" - -const options = getOptions() - -const isInput = (el: Element): el is HTMLInputElement => { - return !!(el as HTMLInputElement).name -} - -document.querySelectorAll("form").forEach((form) => { - if (!form.classList.contains("-x11")) { - return - } - form.addEventListener("submit", (event) => { - event.preventDefault() - const values: { [key: string]: string } = {} - Array.from(form.elements).forEach((element) => { - if (isInput(element)) { - values[element.name] = element.value - } - }) - fetch(normalize(`${options.base}/api/${ApiEndpoint.process}`), { - method: "POST", - body: JSON.stringify(values), - }) - }) -}) - -// TEMP: Until we can get the real ready event. -const event = new CustomEvent("ide-ready") -window.dispatchEvent(event) diff --git a/src/browser/pages/error.html b/src/browser/pages/error.html index 0ae7bb2b..12d6efe2 100644 --- a/src/browser/pages/error.html +++ b/src/browser/pages/error.html @@ -18,7 +18,7 @@ crossorigin="use-credentials" /> - + diff --git a/src/browser/pages/home.css b/src/browser/pages/home.css deleted file mode 100644 index d77d2640..00000000 --- a/src/browser/pages/home.css +++ /dev/null @@ -1,51 +0,0 @@ -.block-row { - display: flex; -} - -.block-row > .item { - flex: 1; - margin: 2px 0; -} - -.block-row > button.item { - background: none; - border: none; - cursor: pointer; - text-align: left; -} - -.block-row > .item > .sub { - font-size: 0.95em; -} - -.block-row .-link { - color: rgb(87, 114, 245); - display: block; - text-decoration: none; -} - -.block-row .-link:hover { - text-decoration: underline; -} - -.block-row > .item > .icon { - height: 1rem; - margin-right: 5px; - vertical-align: top; - width: 1rem; -} - -.block-row > .item > .icon.-missing { - background-color: rgba(87, 114, 245, 0.2); - display: inline-block; - text-align: center; -} - -.kill-form { - display: inline-block; -} - -.kill-form > .kill { - border-radius: 3px; - padding: 2px 5px; -} diff --git a/src/browser/pages/home.html b/src/browser/pages/home.html deleted file mode 100644 index 4fbe8fbd..00000000 --- a/src/browser/pages/home.html +++ /dev/null @@ -1,59 +0,0 @@ - - - - - - - code-server - - - - - - - -
-
-
-

Editors

-
Choose an editor to launch below.
-
-
- {{APP_LIST:EDITORS}} -
-
- -
-
-

Other

-
Choose an application to launch below.
-
-
- {{APP_LIST:OTHER}} -
-
- -
-
-

Version

-
Version information and updates.
-
-
- {{UPDATE:NAME}} -
-
-
- - - - diff --git a/src/browser/pages/login.html b/src/browser/pages/login.html index 37d51f20..788055d6 100644 --- a/src/browser/pages/login.html +++ b/src/browser/pages/login.html @@ -18,7 +18,7 @@ crossorigin="use-credentials" /> - + diff --git a/src/browser/pages/update.html b/src/browser/pages/update.html index e1506f0f..954d3010 100644 --- a/src/browser/pages/update.html +++ b/src/browser/pages/update.html @@ -18,7 +18,7 @@ crossorigin="use-credentials" /> - + diff --git a/src/browser/register.ts b/src/browser/register.ts index 9fb29c8e..3bc810e5 100644 --- a/src/browser/register.ts +++ b/src/browser/register.ts @@ -2,13 +2,17 @@ import { getOptions, normalize } from "../common/util" const options = getOptions() +import "./pages/error.css" +import "./pages/global.css" +import "./pages/login.css" + if ("serviceWorker" in navigator) { const path = normalize(`${options.base}/static/${options.commit}/dist/serviceWorker.js`) navigator.serviceWorker .register(path, { scope: options.base || "/", }) - .then(function () { + .then(() => { console.log("[Service Worker] registered") }) } diff --git a/src/common/api.ts b/src/common/api.ts deleted file mode 100644 index 2a2b14ea..00000000 --- a/src/common/api.ts +++ /dev/null @@ -1,60 +0,0 @@ -export interface Application { - readonly categories?: string[] - readonly comment?: string - readonly directory?: string - readonly exec?: string - readonly genericName?: string - readonly icon?: string - readonly installed?: boolean - readonly name: string - /** - * Path if this is a browser app (like VS Code). - */ - readonly path?: string - /** - * PID if this is a process. - */ - readonly pid?: number - readonly version?: string -} - -export interface ApplicationsResponse { - readonly applications: ReadonlyArray -} - -export enum SessionError { - FailedToStart = 4000, - Starting = 4001, - InvalidState = 4002, - Unknown = 4003, -} - -export interface SessionResponse { - /** - * Whether the process was spawned or an existing one was returned. - */ - created: boolean - pid: number -} - -export interface RecentResponse { - readonly paths: string[] - readonly workspaces: string[] -} - -export interface HealthRequest { - readonly event: "health" -} - -export type ClientMessage = HealthRequest - -export interface HealthResponse { - readonly event: "health" - readonly connections: number -} - -export type ServerMessage = HealthResponse - -export interface ReadyMessage { - protocol: string -} diff --git a/src/common/http.ts b/src/common/http.ts index a90cee37..8ecbaa34 100644 --- a/src/common/http.ts +++ b/src/common/http.ts @@ -14,11 +14,3 @@ export class HttpError extends Error { this.name = this.constructor.name } } - -export enum ApiEndpoint { - applications = "/applications", - process = "/process", - recent = "/recent", - run = "/run", - status = "/status", -} diff --git a/src/node/app/api.ts b/src/node/app/api.ts deleted file mode 100644 index 88519ee3..00000000 --- a/src/node/app/api.ts +++ /dev/null @@ -1,312 +0,0 @@ -import { field, logger } from "@coder/logger" -import * as cp from "child_process" -import * as fs from "fs-extra" -import * as http from "http" -import * as net from "net" -import * as path from "path" -import * as url from "url" -import * as WebSocket from "ws" -import { - Application, - ApplicationsResponse, - ClientMessage, - RecentResponse, - ServerMessage, - SessionError, - SessionResponse, -} from "../../common/api" -import { ApiEndpoint, HttpCode, HttpError } from "../../common/http" -import { HttpProvider, HttpProviderOptions, HttpResponse, HttpServer, Route } from "../http" -import { findApplications, findWhitelistedApplications, Vscode } from "./bin" -import { VscodeHttpProvider } from "./vscode" - -interface VsRecents { - [key: string]: (string | { configURIPath: string })[] -} - -type VsSettings = [string, string][] - -/** - * API HTTP provider. - */ -export class ApiHttpProvider extends HttpProvider { - private readonly ws = new WebSocket.Server({ noServer: true }) - - public constructor( - options: HttpProviderOptions, - private readonly server: HttpServer, - private readonly vscode: VscodeHttpProvider, - private readonly dataDir?: string, - ) { - super(options) - } - - public async handleRequest(route: Route, request: http.IncomingMessage): Promise { - this.ensureAuthenticated(request) - if (!this.isRoot(route)) { - throw new HttpError("Not found", HttpCode.NotFound) - } - - switch (route.base) { - case ApiEndpoint.applications: - this.ensureMethod(request) - return { - mime: "application/json", - content: { - applications: await this.applications(), - }, - } as HttpResponse - case ApiEndpoint.process: - return this.process(request) - case ApiEndpoint.recent: - this.ensureMethod(request) - return { - mime: "application/json", - content: await this.recent(), - } as HttpResponse - } - - throw new HttpError("Not found", HttpCode.NotFound) - } - - public async handleWebSocket( - route: Route, - request: http.IncomingMessage, - socket: net.Socket, - head: Buffer, - ): Promise { - if (!this.authenticated(request)) { - throw new Error("not authenticated") - } - switch (route.base) { - case ApiEndpoint.status: - return this.handleStatusSocket(request, socket, head) - case ApiEndpoint.run: - return this.handleRunSocket(route, request, socket, head) - } - - throw new HttpError("Not found", HttpCode.NotFound) - } - - private async handleStatusSocket(request: http.IncomingMessage, socket: net.Socket, head: Buffer): Promise { - const getMessageResponse = async (event: "health"): Promise => { - switch (event) { - case "health": - return { event, connections: await this.server.getConnections() } - default: - throw new Error("unexpected message") - } - } - - await new Promise((resolve) => { - this.ws.handleUpgrade(request, socket, head, (ws) => { - const send = (event: ServerMessage): void => { - ws.send(JSON.stringify(event)) - } - ws.on("message", (data) => { - logger.trace("got message", field("message", data)) - try { - const message: ClientMessage = JSON.parse(data.toString()) - getMessageResponse(message.event).then(send) - } catch (error) { - logger.error(error.message, field("message", data)) - } - }) - resolve() - }) - }) - } - - /** - * A socket that connects to the process. - */ - private async handleRunSocket( - _route: Route, - request: http.IncomingMessage, - socket: net.Socket, - head: Buffer, - ): Promise { - logger.debug("connecting to process") - const ws = await new Promise((resolve, reject) => { - this.ws.handleUpgrade(request, socket, head, (socket) => { - socket.binaryType = "arraybuffer" - - socket.on("error", (error) => { - socket.close(SessionError.FailedToStart) - logger.error("got error while connecting socket", field("error", error)) - reject(error) - }) - - resolve(socket as WebSocket) - }) - }) - - logger.debug("connected to process") - - // Send ready message. - ws.send( - Buffer.from( - JSON.stringify({ - protocol: "TODO", - }), - ), - ) - } - - /** - * Return whitelisted applications. - */ - public async applications(): Promise> { - return findWhitelistedApplications() - } - - /** - * Return installed applications. - */ - public async installedApplications(): Promise> { - return findApplications() - } - - /** - * Handle /process endpoint. - */ - private async process(request: http.IncomingMessage): Promise { - this.ensureMethod(request, ["DELETE", "POST"]) - - const data = await this.getData(request) - if (!data) { - throw new HttpError("No data was provided", HttpCode.BadRequest) - } - - const parsed: Application = JSON.parse(data) - - switch (request.method) { - case "DELETE": - if (parsed.pid) { - await this.killProcess(parsed.pid) - } else if (parsed.path) { - await this.killProcess(parsed.path) - } else { - throw new Error("No pid or path was provided") - } - return { - mime: "application/json", - code: HttpCode.Ok, - } - case "POST": { - if (!parsed.exec) { - throw new Error("No exec was provided") - } - return { - mime: "application/json", - content: { - created: true, - pid: await this.spawnProcess(parsed.exec), - }, - } as HttpResponse - } - } - - throw new HttpError("Not found", HttpCode.NotFound) - } - - /** - * Kill a process identified by pid or path if a web app. - */ - public async killProcess(pid: number | string): Promise { - if (typeof pid === "string") { - switch (pid) { - case Vscode.path: - await this.vscode.dispose() - break - default: - throw new Error(`Process "${pid}" does not exist`) - } - } else { - process.kill(pid) - } - } - - /** - * Spawn a process and return the pid. - */ - public async spawnProcess(exec: string): Promise { - const proc = cp.spawn(exec, { - shell: process.env.SHELL || true, - env: { - ...process.env, - }, - }) - - proc.on("error", (error) => logger.error("process errored", field("pid", proc.pid), field("error", error))) - proc.on("exit", () => logger.debug("process exited", field("pid", proc.pid))) - - logger.debug("started process", field("pid", proc.pid)) - - return proc.pid - } - - /** - * Return VS Code's recent paths. - */ - public async recent(): Promise { - try { - if (!this.dataDir) { - throw new Error("data directory is not set") - } - - const state: VsSettings = JSON.parse(await fs.readFile(path.join(this.dataDir, "User/state/global.json"), "utf8")) - const setting = Array.isArray(state) && state.find((item) => item[0] === "recently.opened") - if (!setting) { - return { paths: [], workspaces: [] } - } - - const pathPromises: { [key: string]: Promise } = {} - const workspacePromises: { [key: string]: Promise } = {} - Object.values(JSON.parse(setting[1]) as VsRecents).forEach((recents) => { - recents.forEach((recent) => { - try { - const target = typeof recent === "string" ? pathPromises : workspacePromises - const pathname = url.parse(typeof recent === "string" ? recent : recent.configURIPath).pathname - if (pathname && !target[pathname]) { - target[pathname] = new Promise((resolve) => { - fs.stat(pathname) - .then(() => resolve(pathname)) - .catch(() => resolve()) - }) - } - } catch (error) { - logger.debug("invalid path", field("path", recent)) - } - }) - }) - - const [paths, workspaces] = await Promise.all([ - Promise.all(Object.values(pathPromises)), - Promise.all(Object.values(workspacePromises)), - ]) - - return { - paths: paths.filter((p) => !!p), - workspaces: workspaces.filter((p) => !!p), - } - } catch (error) { - if (error.code !== "ENOENT") { - throw error - } - } - - return { paths: [], workspaces: [] } - } - - /** - * For these, just return the error message since they'll be requested as - * JSON. - */ - public async getErrorRoot(_route: Route, _title: string, _header: string, error: string): Promise { - return { - mime: "application/json", - content: JSON.stringify({ error }), - } - } -} diff --git a/src/node/app/bin.ts b/src/node/app/bin.ts deleted file mode 100644 index f12ce3a2..00000000 --- a/src/node/app/bin.ts +++ /dev/null @@ -1,30 +0,0 @@ -import * as fs from "fs" -import * as path from "path" -import { Application } from "../../common/api" - -const getVscodeVersion = (): string => { - try { - return require(path.resolve(__dirname, "../../../lib/vscode/package.json")).version - } catch (error) { - return "unknown" - } -} - -export const Vscode: Application = { - categories: ["Editor"], - icon: fs.readFileSync(path.resolve(__dirname, "../../../lib/vscode/resources/linux/code.png")).toString("base64"), - installed: true, - name: "VS Code", - path: "/", - version: getVscodeVersion(), -} - -export const findApplications = async (): Promise> => { - const apps: Application[] = [Vscode] - - return apps.sort((a, b): number => a.name.localeCompare(b.name)) -} - -export const findWhitelistedApplications = async (): Promise> => { - return [Vscode] -} diff --git a/src/node/app/dashboard.ts b/src/node/app/dashboard.ts deleted file mode 100644 index 261e93c5..00000000 --- a/src/node/app/dashboard.ts +++ /dev/null @@ -1,147 +0,0 @@ -import * as http from "http" -import * as querystring from "querystring" -import { Application } from "../../common/api" -import { HttpCode, HttpError } from "../../common/http" -import { normalize } from "../../common/util" -import { HttpProvider, HttpProviderOptions, HttpResponse, Route } from "../http" -import { ApiHttpProvider } from "./api" -import { UpdateHttpProvider } from "./update" - -/** - * Dashboard HTTP provider. - */ -export class DashboardHttpProvider extends HttpProvider { - public constructor( - options: HttpProviderOptions, - private readonly api: ApiHttpProvider, - private readonly update: UpdateHttpProvider, - ) { - super(options) - } - - public async handleRequest(route: Route, request: http.IncomingMessage): Promise { - if (!this.isRoot(route)) { - throw new HttpError("Not found", HttpCode.NotFound) - } - - switch (route.base) { - case "/spawn": { - this.ensureAuthenticated(request) - this.ensureMethod(request, "POST") - const data = await this.getData(request) - const app = data ? querystring.parse(data) : {} - if (app.path) { - return { redirect: Array.isArray(app.path) ? app.path[0] : app.path } - } - if (!app.exec) { - throw new Error("No exec was provided") - } - this.api.spawnProcess(Array.isArray(app.exec) ? app.exec[0] : app.exec) - return { redirect: this.options.base } - } - case "/app": - case "/": { - this.ensureMethod(request) - if (!this.authenticated(request)) { - return { redirect: "/login", query: { to: this.options.base } } - } - return route.base === "/" ? this.getRoot(route) : this.getAppRoot(route) - } - } - - throw new HttpError("Not found", HttpCode.NotFound) - } - - public async getRoot(route: Route): Promise { - const base = this.base(route) - const apps = await this.api.installedApplications() - const response = await this.getUtf8Resource(this.rootPath, "src/browser/pages/home.html") - response.content = response.content - .replace(/{{UPDATE:NAME}}/, await this.getUpdate(base)) - .replace( - /{{APP_LIST:EDITORS}}/, - this.getAppRows( - base, - apps.filter((app) => app.categories && app.categories.includes("Editor")), - ), - ) - .replace( - /{{APP_LIST:OTHER}}/, - this.getAppRows( - base, - apps.filter((app) => !app.categories || !app.categories.includes("Editor")), - ), - ) - return this.replaceTemplates(route, response) - } - - public async getAppRoot(route: Route): Promise { - const response = await this.getUtf8Resource(this.rootPath, "src/browser/pages/app.html") - return this.replaceTemplates(route, response) - } - - private getAppRows(base: string, apps: ReadonlyArray): string { - return apps.length > 0 - ? apps.map((app) => this.getAppRow(base, app)).join("\n") - : `
No applications found.
` - } - - private getAppRow(base: string, app: Application): string { - return `
- -
` - } - - private async getUpdate(base: string): Promise { - if (!this.update.enabled) { - return `
Updates are disabled
` - } - - const humanize = (time: number): string => { - const d = new Date(time) - const pad = (t: number): string => (t < 10 ? "0" : "") + t - return ( - `${d.getFullYear()}-${pad(d.getMonth() + 1)}-${pad(d.getDate())}` + - ` ${pad(d.getHours())}:${pad(d.getMinutes())}` - ) - } - - const update = await this.update.getUpdate() - if (this.update.isLatestVersion(update)) { - return `
-
- Latest: ${update.version} -
Up to date
-
-
- ${humanize(update.checked)} - Check now -
-
Current: ${this.update.currentVersion}
-
` - } - - return `
-
- Latest: ${update.version} -
Out of date
-
-
- ${humanize(update.checked)} - Update now -
-
Current: ${this.update.currentVersion}
-
` - } -} diff --git a/src/node/entry.ts b/src/node/entry.ts index a7d8663d..dea47d9f 100644 --- a/src/node/entry.ts +++ b/src/node/entry.ts @@ -2,8 +2,6 @@ import { field, logger } from "@coder/logger" import * as cp from "child_process" import * as path from "path" import { CliMessage } from "../../lib/vscode/src/vs/server/ipc" -import { ApiHttpProvider } from "./app/api" -import { DashboardHttpProvider } from "./app/dashboard" import { LoginHttpProvider } from "./app/login" import { ProxyHttpProvider } from "./app/proxy" import { StaticHttpProvider } from "./app/static" @@ -73,13 +71,11 @@ const main = async (args: Args, cliArgs: Args, configArgs: Args): Promise } const httpServer = new HttpServer(options) - const vscode = httpServer.registerHttpProvider("/", VscodeHttpProvider, args) - const api = httpServer.registerHttpProvider("/api", ApiHttpProvider, httpServer, vscode, args["user-data-dir"]) - const update = httpServer.registerHttpProvider("/update", UpdateHttpProvider, false) + httpServer.registerHttpProvider("/", VscodeHttpProvider, args) + httpServer.registerHttpProvider("/update", UpdateHttpProvider, false) httpServer.registerHttpProvider("/proxy", ProxyHttpProvider) httpServer.registerHttpProvider("/login", LoginHttpProvider, args.config!, envPassword) httpServer.registerHttpProvider("/static", StaticHttpProvider) - httpServer.registerHttpProvider("/dashboard", DashboardHttpProvider, api, update) ipcMain().onDispose(() => httpServer.dispose()) From e8f6d3005539761f2849d131ed5c843cdb636cd0 Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 22 Jul 2020 14:53:15 -0500 Subject: [PATCH 02/10] Make providers endpoint-agnostic A provider can now be registered on multiple endpoints (or potentially moved if needed). --- src/node/app/proxy.ts | 4 +-- src/node/app/vscode.ts | 2 +- src/node/entry.ts | 2 +- src/node/http.ts | 60 +++++++++++++++++++++++++----------------- test/update.test.ts | 1 - 5 files changed, 40 insertions(+), 29 deletions(-) diff --git a/src/node/app/proxy.ts b/src/node/app/proxy.ts index eff5059c..14647b8f 100644 --- a/src/node/app/proxy.ts +++ b/src/node/app/proxy.ts @@ -24,7 +24,7 @@ export class ProxyHttpProvider extends HttpProvider { const port = route.base.replace(/^\//, "") return { proxy: { - base: `${this.options.base}/${port}`, + base: `${route.providerBase}/${port}`, port, }, } @@ -35,7 +35,7 @@ export class ProxyHttpProvider extends HttpProvider { const port = route.base.replace(/^\//, "") return { proxy: { - base: `${this.options.base}/${port}`, + base: `${route.providerBase}/${port}`, port, }, } diff --git a/src/node/app/vscode.ts b/src/node/app/vscode.ts index 37cf1046..681f8336 100644 --- a/src/node/app/vscode.ts +++ b/src/node/app/vscode.ts @@ -131,7 +131,7 @@ export class VscodeHttpProvider extends HttpProvider { if (!this.isRoot(route)) { throw new HttpError("Not found", HttpCode.NotFound) } else if (!this.authenticated(request)) { - return { redirect: "/login", query: { to: this.options.base } } + return { redirect: "/login", query: { to: route.providerBase } } } try { return await this.getRoot(request, route) diff --git a/src/node/entry.ts b/src/node/entry.ts index dea47d9f..90e5d91a 100644 --- a/src/node/entry.ts +++ b/src/node/entry.ts @@ -71,7 +71,7 @@ const main = async (args: Args, cliArgs: Args, configArgs: Args): Promise } const httpServer = new HttpServer(options) - httpServer.registerHttpProvider("/", VscodeHttpProvider, args) + httpServer.registerHttpProvider(["/", "/vscode"], VscodeHttpProvider, args) httpServer.registerHttpProvider("/update", UpdateHttpProvider, false) httpServer.registerHttpProvider("/proxy", ProxyHttpProvider) httpServer.registerHttpProvider("/login", LoginHttpProvider, args.config!, envPassword) diff --git a/src/node/http.ts b/src/node/http.ts index 96f074a6..4628dd97 100644 --- a/src/node/http.ts +++ b/src/node/http.ts @@ -79,9 +79,8 @@ export interface HttpResponse { */ mime?: string /** - * Redirect to this path. Will rewrite against the base path but NOT the - * provider endpoint so you must include it. This allows redirecting outside - * of your endpoint. + * Redirect to this path. This is constructed against the site base (not the + * provider's base). */ redirect?: string /** @@ -133,12 +132,16 @@ export interface HttpServerOptions { export interface Route { /** - * Base path part (in /test/path it would be "/test"). + * Provider base path part (for /provider/base/path it would be /provider). + */ + providerBase: string + /** + * Base path part (for /provider/base/path it would be /base). */ base: string /** - * Remaining part of the route (in /test/path it would be "/path"). It can be - * blank. + * Remaining part of the route after factoring out the base and provider base + * (for /provider/base/path it would be /path). It can be blank. */ requestPath: string /** @@ -161,7 +164,6 @@ interface ProviderRoute extends Route { export interface HttpProviderOptions { readonly auth: AuthType - readonly base: string readonly commit: string readonly password?: string } @@ -518,41 +520,51 @@ export class HttpServer { /** * Register a provider for a top-level endpoint. */ - public registerHttpProvider(endpoint: string, provider: HttpProvider0): T - public registerHttpProvider(endpoint: string, provider: HttpProvider1, a1: A1): T + public registerHttpProvider(endpoint: string | string[], provider: HttpProvider0): T + public registerHttpProvider( + endpoint: string | string[], + provider: HttpProvider1, + a1: A1, + ): T public registerHttpProvider( - endpoint: string, + endpoint: string | string[], provider: HttpProvider2, a1: A1, a2: A2, ): T public registerHttpProvider( - endpoint: string, + endpoint: string | string[], provider: HttpProvider3, a1: A1, a2: A2, a3: A3, ): T // eslint-disable-next-line @typescript-eslint/no-explicit-any - public registerHttpProvider(endpoint: string, provider: any, ...args: any[]): any { - endpoint = endpoint.replace(/^\/+|\/+$/g, "") - if (this.providers.has(`/${endpoint}`)) { - throw new Error(`${endpoint} is already registered`) - } - if (/\//.test(endpoint)) { - throw new Error(`Only top-level endpoints are supported (got ${endpoint})`) - } + public registerHttpProvider(endpoint: string | string[], provider: any, ...args: any[]): void { const p = new provider( { auth: this.options.auth || AuthType.None, - base: `/${endpoint}`, commit: this.options.commit, password: this.options.password, }, ...args, ) - this.providers.set(`/${endpoint}`, p) - return p + const endpoints = (typeof endpoint === "string" ? [endpoint] : endpoint).map((e) => e.replace(/^\/+|\/+$/g, "")) + endpoints.forEach((endpoint) => { + if (/\//.test(endpoint)) { + throw new Error(`Only top-level endpoints are supported (got ${endpoint})`) + } + const existingProvider = this.providers.get(`/${endpoint}`) + this.providers.set(`/${endpoint}`, p) + if (existingProvider) { + logger.debug(`Overridding existing /${endpoint} provider`) + // If the existing provider isn't registered elsewhere we can dispose. + if (!Array.from(this.providers.values()).find((p) => p === existingProvider)) { + logger.debug(`Disposing existing /${endpoint} provider`) + existingProvider.dispose() + } + } + }) } /** @@ -759,7 +771,7 @@ export class HttpServer { // that by shifting the next base out of the request path. let provider = this.providers.get(base) if (base !== "/" && provider) { - return { ...parse(requestPath), fullPath, query: parsedUrl.query, provider, originalPath } + return { ...parse(requestPath), providerBase: base, fullPath, query: parsedUrl.query, provider, originalPath } } // Fall back to the top-level provider. @@ -767,7 +779,7 @@ export class HttpServer { if (!provider) { throw new Error(`No provider for ${base}`) } - return { base, fullPath, requestPath, query: parsedUrl.query, provider, originalPath } + return { base, providerBase: "/", fullPath, requestPath, query: parsedUrl.query, provider, originalPath } } /** diff --git a/test/update.test.ts b/test/update.test.ts index 8725eb5f..0a83b063 100644 --- a/test/update.test.ts +++ b/test/update.test.ts @@ -51,7 +51,6 @@ describe("update", () => { _provider = new UpdateHttpProvider( { auth: AuthType.None, - base: "/update", commit: "test", }, true, From 554b6d6fcf219afdbb9c6cef5393c492d401b73f Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 22 Jul 2020 15:55:14 -0500 Subject: [PATCH 03/10] Remove apply portion of update endpoint It can still be used to check for updates but will not apply them. For now also remove the update check loop in VS Code since it's currently unused (update check is hardcoded off right now) and won't work anyway since it also applies the update which now won't work. In the future we should integrate the check into the browser update service. --- ci/dev/vscode.patch | 60 +---------- src/browser/pages/update.html | 43 -------- src/node/app/update.ts | 195 ++-------------------------------- test/update.test.ts | 90 ++-------------- 4 files changed, 23 insertions(+), 365 deletions(-) delete mode 100644 src/browser/pages/update.html diff --git a/ci/dev/vscode.patch b/ci/dev/vscode.patch index ec10c3fa..9690a27a 100644 --- a/ci/dev/vscode.patch +++ b/ci/dev/vscode.patch @@ -679,10 +679,10 @@ index eab8591492..26668701f7 100644 options.logService.error(`${logPrefix} socketFactory.connect() failed. Error:`); diff --git a/src/vs/server/browser/client.ts b/src/vs/server/browser/client.ts new file mode 100644 -index 0000000000..649cf32f0a +index 0000000000..8fb2a87303 --- /dev/null +++ b/src/vs/server/browser/client.ts -@@ -0,0 +1,264 @@ +@@ -0,0 +1,208 @@ +import { Emitter } from 'vs/base/common/event'; +import { URI } from 'vs/base/common/uri'; +import { localize } from 'vs/nls'; @@ -690,7 +690,6 @@ index 0000000000..649cf32f0a +import { registerSingleton } from 'vs/platform/instantiation/common/extensions'; +import { ServiceCollection } from 'vs/platform/instantiation/common/serviceCollection'; +import { ILocalizationsService } from 'vs/platform/localizations/common/localizations'; -+import { ILogService } from 'vs/platform/log/common/log'; +import { INotificationService, Severity } from 'vs/platform/notification/common/notification'; +import { Registry } from 'vs/platform/registry/common/platform'; +import { PersistentConnectionEventType } from 'vs/platform/remote/common/remoteAgentConnection'; @@ -852,61 +851,6 @@ index 0000000000..649cf32f0a + }); + } + -+ const applyUpdate = async (): Promise => { -+ (services.get(ILogService) as ILogService).debug("Applying update..."); -+ -+ const response = await fetch(normalize(`${options.base}/update/apply`), { -+ headers: { "content-type": "application/json" }, -+ }); -+ const json = await response.json(); -+ if (response.status !== 200 || json.error) { -+ throw new Error(json.error || response.statusText); -+ } -+ (services.get(INotificationService) as INotificationService).info(`Updated to ${json.version}`); -+ }; -+ -+ const getUpdate = async (): Promise => { -+ (services.get(ILogService) as ILogService).debug("Checking for update..."); -+ -+ const response = await fetch(normalize(`${options.base}/update`), { -+ headers: { "content-type": "application/json" }, -+ }); -+ const json = await response.json(); -+ if (response.status !== 200 || json.error) { -+ throw new Error(json.error || response.statusText); -+ } -+ if (json.isLatest) { -+ return; -+ } -+ -+ (services.get(INotificationService) as INotificationService).notify({ -+ severity: Severity.Info, -+ message: `code-server has an update: ${json.version}`, -+ actions: { -+ primary: [{ -+ id: 'update', -+ label: 'Apply Update', -+ tooltip: '', -+ class: undefined, -+ enabled: true, -+ checked: true, -+ dispose: () => undefined, -+ run: applyUpdate, -+ }], -+ } -+ }); -+ }; -+ -+ const updateLoop = (): void => { -+ getUpdate().catch((error) => { -+ (services.get(ILogService) as ILogService).warn(error); -+ }).finally(() => { -+ setTimeout(updateLoop, 300000); -+ }); -+ }; -+ -+ updateLoop(); -+ + // This will be used to set the background color while VS Code loads. + const theme = (services.get(IStorageService) as IStorageService).get("colorThemeData", StorageScope.GLOBAL); + if (theme) { diff --git a/src/browser/pages/update.html b/src/browser/pages/update.html deleted file mode 100644 index 954d3010..00000000 --- a/src/browser/pages/update.html +++ /dev/null @@ -1,43 +0,0 @@ - - - - - - - code-server - - - - - - - -
-
-
-

Update

-
Update code-server.
-
-
-
- {{UPDATE_STATUS}} {{ERROR}} -
- go home -
-
-
-
-
- - - diff --git a/src/node/app/update.ts b/src/node/app/update.ts index 23cfd88b..a83f578e 100644 --- a/src/node/app/update.ts +++ b/src/node/app/update.ts @@ -1,21 +1,12 @@ import { field, logger } from "@coder/logger" -import * as cp from "child_process" -import * as fs from "fs-extra" import * as http from "http" import * as https from "https" -import * as os from "os" import * as path from "path" import * as semver from "semver" -import { Readable, Writable } from "stream" -import * as tar from "tar-fs" import * as url from "url" -import * as util from "util" -import * as zlib from "zlib" import { HttpCode, HttpError } from "../../common/http" import { HttpProvider, HttpProviderOptions, HttpResponse, Route } from "../http" import { settings as globalSettings, SettingsProvider, UpdateSettings } from "../settings" -import { tmpdir } from "../util" -import { ipcMain } from "../wrapper" export interface Update { checked: number @@ -27,7 +18,7 @@ export interface LatestResponse { } /** - * Update HTTP provider. + * HTTP provider for checking updates (does not download/install them). */ export class UpdateHttpProvider extends HttpProvider { private update?: Promise @@ -41,12 +32,6 @@ export class UpdateHttpProvider extends HttpProvider { * that fulfills `LatestResponse`. */ private readonly latestUrl = "https://api.github.com/repos/cdr/code-server/releases/latest", - /** - * The URL for downloading a version of code-server. {{VERSION}} and - * {{RELEASE_NAME}} will be replaced (for example 2.1.0 and - * code-server-2.1.0-linux-x86_64.tar.gz). - */ - private readonly downloadUrl = "https://github.com/cdr/code-server/releases/download/{{VERSION}}/{{RELEASE_NAME}}", /** * Update information will be stored here. If not provided, the global * settings will be used. @@ -64,66 +49,30 @@ export class UpdateHttpProvider extends HttpProvider { throw new HttpError("Not found", HttpCode.NotFound) } - switch (route.base) { - case "/check": - this.getUpdate(true) - if (route.query && route.query.to) { - return { - redirect: Array.isArray(route.query.to) ? route.query.to[0] : route.query.to, - query: { to: undefined }, - } - } - return this.getRoot(route, request) - case "/apply": - return this.tryUpdate(route, request) - case "/": - return this.getRoot(route, request) + if (!this.enabled) { + throw new Error("update checks are disabled") } - throw new HttpError("Not found", HttpCode.NotFound) - } - - public async getRoot( - route: Route, - request: http.IncomingMessage, - errorOrUpdate?: Update | Error, - ): Promise { - if (request.headers["content-type"] === "application/json") { - if (!this.enabled) { + switch (route.base) { + case "/check": + case "/": { + const update = await this.getUpdate(route.base === "/check") return { content: { - isLatest: true, + ...update, + isLatest: this.isLatestVersion(update), }, } } - const update = await this.getUpdate() - return { - content: { - ...update, - isLatest: this.isLatestVersion(update), - }, - } } - const response = await this.getUtf8Resource(this.rootPath, "src/browser/pages/update.html") - response.content = response.content - .replace( - /{{UPDATE_STATUS}}/, - errorOrUpdate && !(errorOrUpdate instanceof Error) - ? `Updated to ${errorOrUpdate.version}` - : await this.getUpdateHtml(), - ) - .replace(/{{ERROR}}/, errorOrUpdate instanceof Error ? `
${errorOrUpdate.message}
` : "") - return this.replaceTemplates(route, response) + + throw new HttpError("Not found", HttpCode.NotFound) } /** * Query for and return the latest update. */ public async getUpdate(force?: boolean): Promise { - if (!this.enabled) { - throw new Error("updates are not enabled") - } - // Don't run multiple requests at a time. if (!this.update) { this.update = this._getUpdate(force) @@ -171,128 +120,6 @@ export class UpdateHttpProvider extends HttpProvider { } } - private async getUpdateHtml(): Promise { - if (!this.enabled) { - return "Updates are disabled" - } - - const update = await this.getUpdate() - if (this.isLatestVersion(update)) { - return "No update available" - } - - return `` - } - - public async tryUpdate(route: Route, request: http.IncomingMessage): Promise { - try { - const update = await this.getUpdate() - if (!this.isLatestVersion(update)) { - await this.downloadAndApplyUpdate(update) - return this.getRoot(route, request, update) - } - return this.getRoot(route, request) - } catch (error) { - // For JSON requests propagate the error. Otherwise catch it so we can - // show the error inline with the update button instead of an error page. - if (request.headers["content-type"] === "application/json") { - throw error - } - return this.getRoot(route, error) - } - } - - public async downloadAndApplyUpdate(update: Update, targetPath?: string): Promise { - const releaseName = await this.getReleaseName(update) - const url = this.downloadUrl.replace("{{VERSION}}", update.version).replace("{{RELEASE_NAME}}", releaseName) - - let downloadPath = path.join(tmpdir, "updates", releaseName) - fs.mkdirp(path.dirname(downloadPath)) - - const response = await this.requestResponse(url) - - try { - downloadPath = await this.extractTar(response, downloadPath) - logger.debug("Downloaded update", field("path", downloadPath)) - - // The archive should have a directory inside at the top level with the - // same name as the archive. - const directoryPath = path.join(downloadPath, path.basename(downloadPath)) - await fs.stat(directoryPath) - - if (!targetPath) { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - targetPath = path.resolve(__dirname, "../../../") - } - - // Move the old directory to prevent potential data loss. - const backupPath = path.resolve(targetPath, `../${path.basename(targetPath)}.${Date.now().toString()}`) - logger.debug("Replacing files", field("target", targetPath), field("backup", backupPath)) - await fs.move(targetPath, backupPath) - - // Move the new directory. - await fs.move(directoryPath, targetPath) - - await fs.remove(downloadPath) - - if (process.send) { - ipcMain().relaunch(update.version) - } - } catch (error) { - response.destroy(error) - throw error - } - } - - private async extractTar(response: Readable, downloadPath: string): Promise { - downloadPath = downloadPath.replace(/\.tar\.gz$/, "") - logger.debug("Extracting tar", field("path", downloadPath)) - - response.pause() - await fs.remove(downloadPath) - - const decompress = zlib.createGunzip() - response.pipe(decompress as Writable) - response.on("error", (error) => decompress.destroy(error)) - response.on("close", () => decompress.end()) - - const destination = tar.extract(downloadPath) - decompress.pipe(destination) - decompress.on("error", (error) => destination.destroy(error)) - decompress.on("close", () => destination.end()) - - await new Promise((resolve, reject) => { - destination.on("finish", resolve) - destination.on("error", reject) - response.resume() - }) - - return downloadPath - } - - /** - * Given an update return the name for the packaged archived. - */ - public async getReleaseName(update: Update): Promise { - let target: string = os.platform() - if (target === "linux") { - const result = await util - .promisify(cp.exec)("ldd --version") - .catch((error) => ({ - stderr: error.message, - stdout: "", - })) - if (/musl/.test(result.stderr) || /musl/.test(result.stdout)) { - target = "alpine" - } - } - let arch = os.arch() - if (arch === "x64") { - arch = "x86_64" - } - return `code-server-${update.version}-${target}-${arch}.tar.gz` - } - private async request(uri: string): Promise { const response = await this.requestResponse(uri) return new Promise((resolve, reject) => { diff --git a/test/update.test.ts b/test/update.test.ts index 0a83b063..15719bfa 100644 --- a/test/update.test.ts +++ b/test/update.test.ts @@ -2,40 +2,33 @@ import * as assert from "assert" import * as fs from "fs-extra" import * as http from "http" import * as path from "path" -import * as tar from "tar-fs" -import * as zlib from "zlib" import { LatestResponse, UpdateHttpProvider } from "../src/node/app/update" import { AuthType } from "../src/node/http" import { SettingsProvider, UpdateSettings } from "../src/node/settings" import { tmpdir } from "../src/node/util" describe("update", () => { - const archivePath = path.join(tmpdir, "tests/updates/code-server-loose-source") let version = "1.0.0" let spy: string[] = [] const server = http.createServer((request: http.IncomingMessage, response: http.ServerResponse) => { if (!request.url) { throw new Error("no url") } + spy.push(request.url) - response.writeHead(200) + + // Return the latest version. if (request.url === "/latest") { const latest: LatestResponse = { name: version, } + response.writeHead(200) return response.end(JSON.stringify(latest)) } - const path = archivePath + (request.url.endsWith(".tar.gz") ? ".tar.gz" : ".zip") - - const stream = fs.createReadStream(path) - stream.on("error", (error: NodeJS.ErrnoException) => { - response.writeHead(500) - response.end(error.message) - }) - response.writeHead(200) - stream.on("close", () => response.end()) - stream.pipe(response) + // Anything else is a 404. + response.writeHead(404) + response.end("not found") }) const jsonPath = path.join(tmpdir, "tests/updates/update.json") @@ -55,7 +48,6 @@ describe("update", () => { }, true, `http://${address.address}:${address.port}/latest`, - `http://${address.address}:${address.port}/download/{{VERSION}}/{{RELEASE_NAME}}`, settings, ) } @@ -71,32 +63,8 @@ describe("update", () => { host: "localhost", }) }) - - const p = provider() - const archiveName = (await p.getReleaseName({ version: "9999999.99999.9999", checked: 0 })).replace( - /.tar.gz$|.zip$/, - "", - ) await fs.remove(path.join(tmpdir, "tests/updates")) - await fs.mkdirp(path.join(archivePath, archiveName)) - - await Promise.all([ - fs.writeFile(path.join(archivePath, archiveName, "code-server"), `console.log("UPDATED")`), - fs.writeFile(path.join(archivePath, archiveName, "node"), `NODE BINARY`), - ]) - - await new Promise((resolve, reject) => { - const write = fs.createWriteStream(archivePath + ".tar.gz") - const compress = zlib.createGzip() - compress.pipe(write) - compress.on("error", (error) => compress.destroy(error)) - compress.on("close", () => write.end()) - tar.pack(archivePath).pipe(compress) - write.on("close", reject) - write.on("finish", () => { - resolve() - }) - }) + await fs.mkdirp(path.join(tmpdir, "tests/updates")) }) after(() => { @@ -184,53 +152,15 @@ describe("update", () => { assert.equal(p.isLatestVersion(update), true) }) - it("should download and apply an update", async () => { - version = "9999999.99999.9999" - - const p = provider() - const update = await p.getUpdate(true) - - // Create an existing version. - const destination = path.join(tmpdir, "tests/updates/code-server") - await fs.mkdirp(destination) - const entry = path.join(destination, "code-server") - await fs.writeFile(entry, `console.log("OLD")`) - assert.equal(`console.log("OLD")`, await fs.readFile(entry, "utf8")) - - // Updating should replace the existing version. - await p.downloadAndApplyUpdate(update, destination) - assert.equal(`console.log("UPDATED")`, await fs.readFile(entry, "utf8")) - - // There should be a backup. - const dir = (await fs.readdir(path.join(tmpdir, "tests/updates"))).filter((dir) => { - return dir.startsWith("code-server.") - }) - assert.equal(dir.length, 1) - assert.equal( - `console.log("OLD")`, - await fs.readFile(path.join(tmpdir, "tests/updates", dir[0], "code-server"), "utf8"), - ) - - const archiveName = await p.getReleaseName(update) - assert.deepEqual(spy, ["/latest", `/download/${version}/${archiveName}`]) - }) - it("should not reject if unable to fetch", async () => { const options = { auth: AuthType.None, - base: "/update", commit: "test", } - let provider = new UpdateHttpProvider(options, true, "invalid", "invalid", settings) + let provider = new UpdateHttpProvider(options, true, "invalid", settings) await assert.doesNotReject(() => provider.getUpdate(true)) - provider = new UpdateHttpProvider( - options, - true, - "http://probably.invalid.dev.localhost/latest", - "http://probably.invalid.dev.localhost/download", - settings, - ) + provider = new UpdateHttpProvider(options, true, "http://probably.invalid.dev.localhost/latest", settings) await assert.doesNotReject(() => provider.getUpdate(true)) }) }) From 4b6c0a6fc38be1396b646775eb02bcf665597931 Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 22 Jul 2020 17:10:52 -0500 Subject: [PATCH 04/10] Update logger --- package.json | 2 +- yarn.lock | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/package.json b/package.json index ff4f1a76..789db6f3 100644 --- a/package.json +++ b/package.json @@ -64,7 +64,7 @@ "vfile-message": "^2.0.2" }, "dependencies": { - "@coder/logger": "1.1.11", + "@coder/logger": "1.1.16", "env-paths": "^2.2.0", "fs-extra": "^8.1.0", "http-proxy": "^1.18.0", diff --git a/yarn.lock b/yarn.lock index a808a9a5..cf61ced9 100644 --- a/yarn.lock +++ b/yarn.lock @@ -792,10 +792,10 @@ lodash "^4.17.13" to-fast-properties "^2.0.0" -"@coder/logger@1.1.11": - version "1.1.11" - resolved "https://registry.yarnpkg.com/@coder/logger/-/logger-1.1.11.tgz#e6f36dba9436ae61e66e3f66787d75c768617605" - integrity sha512-EEh1dqSU0AaqjjjMsVqumgZGbrZimKFKIb4t5E6o3FLfVUxJCReSME78Yj2N1xWUVAHMnqafDCxLostpuIotzw== +"@coder/logger@1.1.16": + version "1.1.16" + resolved "https://registry.yarnpkg.com/@coder/logger/-/logger-1.1.16.tgz#ee5b1b188f680733f35c11b065bbd139d618c1e1" + integrity sha512-X6VB1++IkosYY6amRAiMvuvCf12NA4+ooX+gOuu5bJIkdjmh4Lz7QpJcWRdgxesvo1msriDDr9E/sDbIWf6vsQ== "@iarna/toml@^2.2.0": version "2.2.5" From 58bd7008b47b537d94236c8f5b821f42063060da Mon Sep 17 00:00:00 2001 From: Asher Date: Thu, 23 Jul 2020 12:22:38 -0500 Subject: [PATCH 05/10] Make dispose async --- src/node/entry.ts | 6 +++++- src/node/http.ts | 12 +++++++++--- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/node/entry.ts b/src/node/entry.ts index 90e5d91a..a030cb49 100644 --- a/src/node/entry.ts +++ b/src/node/entry.ts @@ -77,7 +77,11 @@ const main = async (args: Args, cliArgs: Args, configArgs: Args): Promise httpServer.registerHttpProvider("/login", LoginHttpProvider, args.config!, envPassword) httpServer.registerHttpProvider("/static", StaticHttpProvider) - ipcMain().onDispose(() => httpServer.dispose()) + ipcMain().onDispose(() => { + httpServer.dispose().then((errors) => { + errors.forEach((error) => logger.error(error.message)) + }) + }) logger.info(`code-server ${version} ${commit}`) const serverAddress = await httpServer.listen() diff --git a/src/node/http.ts b/src/node/http.ts index 4628dd97..a4a83aea 100644 --- a/src/node/http.ts +++ b/src/node/http.ts @@ -177,7 +177,7 @@ export abstract class HttpProvider { public constructor(protected readonly options: HttpProviderOptions) {} - public dispose(): void { + public async dispose(): Promise { // No default behavior. } @@ -504,9 +504,15 @@ export class HttpServer { }) } - public dispose(): void { + /** + * Stop and dispose everything. Return an array of disposal errors. + */ + public async dispose(): Promise { this.socketProvider.stop() - this.providers.forEach((p) => p.dispose()) + const providers = Array.from(this.providers.values()) + // Catch so all the errors can be seen rather than just the first one. + const responses = await Promise.all(providers.map((p) => p.dispose().catch((e) => e))) + return responses.filter((r): r is Error => typeof r !== "undefined") } public async getConnections(): Promise { From c67d31580fdbc0fb2c58a7ce2c20d05e097d519d Mon Sep 17 00:00:00 2001 From: Asher Date: Thu, 23 Jul 2020 12:23:33 -0500 Subject: [PATCH 06/10] Include details if any in JSON requests --- src/common/http.ts | 2 +- src/node/http.ts | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/common/http.ts b/src/common/http.ts index 8ecbaa34..4749247d 100644 --- a/src/common/http.ts +++ b/src/common/http.ts @@ -9,7 +9,7 @@ export enum HttpCode { } export class HttpError extends Error { - public constructor(message: string, public readonly code: number) { + public constructor(message: string, public readonly code: number, public readonly details?: object) { super(message) this.name = this.constructor.name } diff --git a/src/node/http.ts b/src/node/http.ts index a4a83aea..62abeb07 100644 --- a/src/node/http.ts +++ b/src/node/http.ts @@ -667,8 +667,10 @@ export class HttpServer { if (request.headers["content-type"] === "application/json") { write({ code, + mime: "application/json", content: { error: e.message, + ...(e.details || {}), }, }) } else { From 7c2ca7d03e30448f095e340f9d478db96c070f03 Mon Sep 17 00:00:00 2001 From: Asher Date: Thu, 23 Jul 2020 12:37:45 -0500 Subject: [PATCH 07/10] Add the ability to prepend to the proxy path This is for applications like Jupyter that aren't base path agnostic. --- src/node/app/proxy.ts | 4 ++-- src/node/http.ts | 13 +++++++++---- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/node/app/proxy.ts b/src/node/app/proxy.ts index 14647b8f..a332cc05 100644 --- a/src/node/app/proxy.ts +++ b/src/node/app/proxy.ts @@ -24,7 +24,7 @@ export class ProxyHttpProvider extends HttpProvider { const port = route.base.replace(/^\//, "") return { proxy: { - base: `${route.providerBase}/${port}`, + strip: `${route.providerBase}/${port}`, port, }, } @@ -35,7 +35,7 @@ export class ProxyHttpProvider extends HttpProvider { const port = route.base.replace(/^\//, "") return { proxy: { - base: `${route.providerBase}/${port}`, + strip: `${route.providerBase}/${port}`, port, }, } diff --git a/src/node/http.ts b/src/node/http.ts index 62abeb07..313ecbeb 100644 --- a/src/node/http.ts +++ b/src/node/http.ts @@ -36,9 +36,13 @@ export type Query = { [key: string]: string | string[] | undefined } export interface ProxyOptions { /** - * A base path to strip from from the request before proxying if necessary. + * A path to strip from from the beginning of the request before proxying */ - base?: string + strip?: string + /** + * A path to add to the beginning of the request before proxying. + */ + prepend?: string /** * The port to proxy. */ @@ -826,10 +830,11 @@ export class HttpServer { // sure how best to get this information to the `proxyRes` event handler. // For now I'm sticking it on the request object which is passed through to // the event. - ;(request as ProxyRequest).base = options.base + ;(request as ProxyRequest).base = options.strip const isHttp = response instanceof http.ServerResponse - const path = options.base ? route.fullPath.replace(options.base, "") : route.fullPath + const base = options.strip ? route.fullPath.replace(options.strip, "") : route.fullPath + const path = normalize("/" + (options.prepend || "") + "/" + base, true) const proxyOptions: proxy.ServerOptions = { changeOrigin: true, ignorePath: true, From 2fa5037859d3490d97d6b0f21a357fcd66652308 Mon Sep 17 00:00:00 2001 From: Asher Date: Thu, 23 Jul 2020 12:51:08 -0500 Subject: [PATCH 08/10] Log output to disk --- package.json | 1 + src/node/wrapper.ts | 25 +++++++++++++++++++++++-- yarn.lock | 5 +++++ 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 789db6f3..3f64559f 100644 --- a/package.json +++ b/package.json @@ -72,6 +72,7 @@ "js-yaml": "^3.13.1", "limiter": "^1.1.5", "pem": "^1.14.2", + "rotating-file-stream": "^2.1.1", "safe-compare": "^1.1.4", "semver": "^7.1.3", "tar": "^6.0.1", diff --git a/src/node/wrapper.ts b/src/node/wrapper.ts index cb0aefab..6e2e7e74 100644 --- a/src/node/wrapper.ts +++ b/src/node/wrapper.ts @@ -1,6 +1,9 @@ -import { logger, field } from "@coder/logger" +import { field, logger } from "@coder/logger" import * as cp from "child_process" +import * as path from "path" +import * as rfs from "rotating-file-stream" import { Emitter } from "../common/emitter" +import { paths } from "./util" interface HandshakeMessage { type: "handshake" @@ -140,8 +143,17 @@ export interface WrapperOptions { export class WrapperProcess { private process?: cp.ChildProcess private started?: Promise + private readonly logStdoutStream: rfs.RotatingFileStream + private readonly logStderrStream: rfs.RotatingFileStream public constructor(private currentVersion: string, private readonly options?: WrapperOptions) { + const opts = { + size: "10M", + maxFiles: 10, + } + this.logStdoutStream = rfs.createStream(path.join(paths.data, "coder-logs", "code-server-stdout.log"), opts) + this.logStderrStream = rfs.createStream(path.join(paths.data, "coder-logs", "code-server-stderr.log"), opts) + ipcMain().onDispose(() => { if (this.process) { this.process.removeAllListeners() @@ -176,6 +188,15 @@ export class WrapperProcess { public start(): Promise { if (!this.started) { this.started = this.spawn().then((child) => { + // Log both to stdout and to the log directory. + if (child.stdout) { + child.stdout.pipe(this.logStdoutStream) + child.stdout.pipe(process.stdout) + } + if (child.stderr) { + child.stderr.pipe(this.logStderrStream) + child.stderr.pipe(process.stderr) + } logger.debug(`spawned inner process ${child.pid}`) ipcMain() .handshake(child) @@ -205,7 +226,7 @@ export class WrapperProcess { CODE_SERVER_PARENT_PID: process.pid.toString(), NODE_OPTIONS: nodeOptions, }, - stdio: ["inherit", "inherit", "inherit", "ipc"], + stdio: ["ipc"], }) } } diff --git a/yarn.lock b/yarn.lock index cf61ced9..85ec456d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6144,6 +6144,11 @@ ripemd160@^2.0.0, ripemd160@^2.0.1: hash-base "^3.0.0" inherits "^2.0.1" +rotating-file-stream@^2.1.1: + version "2.1.3" + resolved "https://registry.yarnpkg.com/rotating-file-stream/-/rotating-file-stream-2.1.3.tgz#4b3cc8f56ae70b3e30ccdb4ee6b14d95e66b02bb" + integrity sha512-zZ4Tkngxispo7DgiTqX0s4ChLtM3qET6iYsDA9tmgDEqJ3BFgRq/ZotsKEDAYQt9pAn9JwwqT27CSwQt3CTxNg== + run-async@^2.4.0: version "2.4.1" resolved "https://registry.yarnpkg.com/run-async/-/run-async-2.4.1.tgz#8440eccf99ea3e70bd409d49aab88e10c189a455" From c581bca29dbd91913f712d4c5ff9e1094df1cb97 Mon Sep 17 00:00:00 2001 From: Asher Date: Thu, 23 Jul 2020 13:00:38 -0500 Subject: [PATCH 09/10] Force minimist update --- ci/dev/vscode.patch | 43 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 40 insertions(+), 3 deletions(-) diff --git a/ci/dev/vscode.patch b/ci/dev/vscode.patch index 9690a27a..e4fb3cad 100644 --- a/ci/dev/vscode.patch +++ b/ci/dev/vscode.patch @@ -218,7 +218,7 @@ index 0000000000..88b720ceee + common.minifyTask("out-vscode") +)); diff --git a/package.json b/package.json -index 86e3d5140d..2e52256e49 100644 +index 86e3d5140d..962050280c 100644 --- a/package.json +++ b/package.json @@ -42,6 +42,9 @@ @@ -231,6 +231,15 @@ index 86e3d5140d..2e52256e49 100644 "applicationinsights": "1.0.8", "chokidar": "3.2.3", "graceful-fs": "4.2.3", +@@ -185,5 +188,8 @@ + "windows-foreground-love": "0.2.0", + "windows-mutex": "0.3.0", + "windows-process-tree": "0.2.4" ++ }, ++ "resolutions": { ++ "minimist": "^1.2.5" + } + } diff --git a/product.json b/product.json index 5378b017c8..afdadda974 100644 --- a/product.json @@ -3375,7 +3384,7 @@ index 153ac595d0..a6eb49c5dd 100644 import 'vs/workbench/services/credentials/browser/credentialsService'; import 'vs/workbench/services/url/browser/urlService'; diff --git a/yarn.lock b/yarn.lock -index 6bc96e8377..585401f144 100644 +index 6bc96e8377..a2baf909d6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -140,6 +140,23 @@ @@ -3416,7 +3425,35 @@ index 6bc96e8377..585401f144 100644 just-debounce@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/just-debounce/-/just-debounce-1.0.0.tgz#87fccfaeffc0b68cd19d55f6722943f929ea35ea" -@@ -6798,6 +6822,11 @@ p-try@^2.0.0: +@@ -6009,26 +6033,11 @@ minimatch@0.3: + dependencies: + brace-expansion "^1.1.7" + +-minimist@0.0.8: +- version "0.0.8" +- resolved "https://registry.yarnpkg.com/minimist/-/minimist-0.0.8.tgz#857fcabfc3397d2625b8228262e86aa7a011b05d" +- integrity sha1-hX/Kv8M5fSYluCKCYuhqp6ARsF0= +- +-minimist@^1.2.0: +- version "1.2.0" +- resolved "https://registry.yarnpkg.com/minimist/-/minimist-1.2.0.tgz#a35008b20f41383eec1fb914f4cd5df79a264284" +- integrity sha1-o1AIsg9BOD7sH7kU9M1d95omQoQ= +- +-minimist@^1.2.5: ++minimist@0.0.8, minimist@^1.2.0, minimist@^1.2.5, minimist@~0.0.1: + version "1.2.5" + resolved "https://registry.yarnpkg.com/minimist/-/minimist-1.2.5.tgz#67d66014b66a6a8aaa0c083c5fd58df4e4e97602" + integrity sha512-FM9nNUYrRBAELZQT3xeZQ7fmMOBg6nWNmJKTcgsJeaLstP/UODVpGsr5OhXhhXg6f+qtJ8uiZ+PUxkDWcgIXLw== + +-minimist@~0.0.1: +- version "0.0.10" +- resolved "https://registry.yarnpkg.com/minimist/-/minimist-0.0.10.tgz#de3f98543dbf96082be48ad1a0c7cda836301dcf" +- integrity sha1-3j+YVD2/lggr5IrRoMfNqDYwHc8= +- + minipass@^2.2.1, minipass@^2.3.3: + version "2.3.3" + resolved "https://registry.yarnpkg.com/minipass/-/minipass-2.3.3.tgz#a7dcc8b7b833f5d368759cce544dccb55f50f233" +@@ -6798,6 +6807,11 @@ p-try@^2.0.0: resolved "https://registry.yarnpkg.com/p-try/-/p-try-2.0.0.tgz#85080bb87c64688fa47996fe8f7dfbe8211760b1" integrity sha512-hMp0onDKIajHfIkdRk3P4CdCmErkYAxxDtP3Wx/4nZ3aGlau2VKh3mZpcuFkH27WQkL/3WBCPOktzA9ZOAnMQQ== From e86c0664386cf05dcd739a2d36daff5b3af176c4 Mon Sep 17 00:00:00 2001 From: Asher Date: Thu, 30 Jul 2020 12:14:31 -0500 Subject: [PATCH 10/10] Add helper functions to make some code clearer --- src/common/util.ts | 21 +++++++++++++++++++++ src/node/app/vscode.ts | 5 ++--- src/node/http.ts | 6 +++--- 3 files changed, 26 insertions(+), 6 deletions(-) diff --git a/src/common/util.ts b/src/common/util.ts index d9195b62..4c9366b4 100644 --- a/src/common/util.ts +++ b/src/common/util.ts @@ -33,6 +33,13 @@ export const normalize = (url: string, keepTrailing = false): string => { return url.replace(/\/\/+/g, "/").replace(/\/+$/, keepTrailing ? "/" : "") } +/** + * Remove leading and trailing slashes. + */ +export const trimSlashes = (url: string): string => { + return url.replace(/^\/+|\/+$/g, "") +} + /** * Get options embedded in the HTML or query params. */ @@ -75,3 +82,17 @@ export const getOptions = (): T => { return options } + +/** + * Wrap the value in an array if it's not already an array. If the value is + * undefined return an empty array. + */ +export const arrayify = (value?: T | T[]): T[] => { + if (Array.isArray(value)) { + return value + } + if (typeof value === "undefined") { + return [] + } + return [value] +} diff --git a/src/node/app/vscode.ts b/src/node/app/vscode.ts index 681f8336..fb5ed308 100644 --- a/src/node/app/vscode.ts +++ b/src/node/app/vscode.ts @@ -14,7 +14,7 @@ import { WorkbenchOptions, } from "../../../lib/vscode/src/vs/server/ipc" import { HttpCode, HttpError } from "../../common/http" -import { generateUuid } from "../../common/util" +import { arrayify, generateUuid } from "../../common/util" import { Args } from "../cli" import { HttpProvider, HttpProviderOptions, HttpResponse, Route } from "../http" import { settings } from "../settings" @@ -224,8 +224,7 @@ export class VscodeHttpProvider extends HttpProvider { } for (let i = 0; i < startPaths.length; ++i) { const startPath = startPaths[i] - const url = - startPath && (typeof startPath.url === "string" ? [startPath.url] : startPath.url || []).find((p) => !!p) + const url = arrayify(startPath && startPath.url).find((p) => !!p) if (startPath && url) { return { url, diff --git a/src/node/http.ts b/src/node/http.ts index 313ecbeb..216ff5a2 100644 --- a/src/node/http.ts +++ b/src/node/http.ts @@ -12,7 +12,7 @@ import { Readable } from "stream" import * as tls from "tls" import * as url from "url" import { HttpCode, HttpError } from "../common/http" -import { normalize, Options, plural, split } from "../common/util" +import { arrayify, normalize, Options, plural, split, trimSlashes } from "../common/util" import { SocketProxyProvider } from "./socket" import { getMediaMime, paths } from "./util" @@ -287,7 +287,7 @@ export abstract class HttpProvider { * Helper to error on invalid methods (default GET). */ protected ensureMethod(request: http.IncomingMessage, method?: string | string[]): void { - const check = Array.isArray(method) ? method : [method || "GET"] + const check = arrayify(method || "GET") if (!request.method || !check.includes(request.method)) { throw new HttpError(`Unsupported method ${request.method}`, HttpCode.BadRequest) } @@ -559,7 +559,7 @@ export class HttpServer { }, ...args, ) - const endpoints = (typeof endpoint === "string" ? [endpoint] : endpoint).map((e) => e.replace(/^\/+|\/+$/g, "")) + const endpoints = arrayify(endpoint).map(trimSlashes) endpoints.forEach((endpoint) => { if (/\//.test(endpoint)) { throw new Error(`Only top-level endpoints are supported (got ${endpoint})`)