From e3e9f052c48c85a2b562ae0d5de57d09ae8f1d35 Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Tue, 15 Feb 2022 14:51:42 -0700 Subject: [PATCH] fix: wrap socket in proxy before passing to vscode (#4840) * chore: add ipc hook to e2e script * refactor: allow codeServerArgs in e2e tests * feat: add --cert e2e extension test * fix: wrap websocket in proxy * fixup: remvoe ignoreHTTPSErrors * fixup: make codeServerArgs readonly * fixup! add back ignoreHTTPSErrors --- package.json | 2 +- src/node/routes/vscode.ts | 8 ++++++-- test/e2e/baseFixture.ts | 12 ++++++++++-- test/e2e/codeServer.test.ts | 2 +- test/e2e/extensions.test.ts | 10 +++++++++- test/e2e/globalSetup.test.ts | 2 +- test/e2e/login.test.ts | 2 +- test/e2e/logout.test.ts | 2 +- test/e2e/models/CodeServer.ts | 3 ++- test/e2e/openHelpAbout.test.ts | 2 +- test/e2e/terminal.test.ts | 2 +- 11 files changed, 34 insertions(+), 13 deletions(-) diff --git a/package.json b/package.json index 9f7558bcb..cd01d123a 100644 --- a/package.json +++ b/package.json @@ -17,7 +17,7 @@ "release:github-draft": "./ci/build/release-github-draft.sh", "release:github-assets": "./ci/build/release-github-assets.sh", "release:prep": "./ci/build/release-prep.sh", - "test:e2e": "./ci/dev/test-e2e.sh", + "test:e2e": "VSCODE_IPC_HOOK_CLI= ./ci/dev/test-e2e.sh", "test:standalone-release": "./ci/build/test-standalone-release.sh", "test:unit": "./ci/dev/test-unit.sh --forceExit --detectOpenHandles", "test:scripts": "./ci/dev/test-scripts.sh", diff --git a/src/node/routes/vscode.ts b/src/node/routes/vscode.ts index 4b1d16a4e..fe9c44181 100644 --- a/src/node/routes/vscode.ts +++ b/src/node/routes/vscode.ts @@ -5,6 +5,7 @@ import { logError } from "../../common/util" import { toVsCodeArgs } from "../cli" import { isDevMode } from "../constants" import { authenticated, ensureAuthenticated, redirect, self } from "../http" +import { SocketProxyProvider } from "../socket" import { loadAMDModule } from "../util" import { Router as WsRouter } from "../wsRouter" import { errorHandler } from "./errors" @@ -13,6 +14,7 @@ export class CodeServerRouteWrapper { /** Assigned in `ensureCodeServerLoaded` */ private _codeServerMain!: CodeServerLib.IServerAPI private _wsRouterWrapper = WsRouter() + private _socketProxyProvider = new SocketProxyProvider() public router = express.Router() public get wsRouter() { @@ -77,9 +79,10 @@ export class CodeServerRouteWrapper { } private $proxyWebsocket = async (req: WebsocketRequest) => { - this._codeServerMain.handleUpgrade(req, req.socket) + const wrappedSocket = await this._socketProxyProvider.createProxy(req.ws) + this._codeServerMain.handleUpgrade(req, wrappedSocket) - req.socket.resume() + req.ws.resume() } //#endregion @@ -130,5 +133,6 @@ export class CodeServerRouteWrapper { dispose() { this._codeServerMain?.dispose() + this._socketProxyProvider.stop() } } diff --git a/test/e2e/baseFixture.ts b/test/e2e/baseFixture.ts index a18722ed4..5cf0dd6a4 100644 --- a/test/e2e/baseFixture.ts +++ b/test/e2e/baseFixture.ts @@ -9,10 +9,15 @@ import { CodeServer, CodeServerPage } from "./models/CodeServer" * * If `includeCredentials` is `true` page requests will be authenticated. */ -export const describe = (name: string, includeCredentials: boolean, fn: (codeServer: CodeServer) => void) => { +export const describe = ( + name: string, + includeCredentials: boolean, + codeServerArgs: string[], + fn: (codeServer: CodeServer) => void, +) => { test.describe(name, () => { // This will spawn on demand so nothing is necessary on before. - const codeServer = new CodeServer(name) + const codeServer = new CodeServer(name, codeServerArgs) // Kill code-server after the suite has ended. This may happen even without // doing it explicitly but it seems prudent to be sure. @@ -36,6 +41,9 @@ export const describe = (name: string, includeCredentials: boolean, fn: (codeSer authenticated: includeCredentials, // This provides a cookie that authenticates with code-server. storageState: includeCredentials ? storageState : {}, + // NOTE@jsjoeio some tests use --cert which uses a self-signed certificate + // without this option, those tests will fail. + ignoreHTTPSErrors: true, }) fn(codeServer) diff --git a/test/e2e/codeServer.test.ts b/test/e2e/codeServer.test.ts index cfac6f74c..06eaaea0d 100644 --- a/test/e2e/codeServer.test.ts +++ b/test/e2e/codeServer.test.ts @@ -1,6 +1,6 @@ import { describe, test, expect } from "./baseFixture" -describe("CodeServer", true, () => { +describe("CodeServer", true, [], () => { test("should navigate to home page", async ({ codeServerPage }) => { // We navigate codeServer before each test // and we start the test with a storage state diff --git a/test/e2e/extensions.test.ts b/test/e2e/extensions.test.ts index f458ac29d..3fbf3eaea 100644 --- a/test/e2e/extensions.test.ts +++ b/test/e2e/extensions.test.ts @@ -1,6 +1,6 @@ import { describe, test } from "./baseFixture" -describe("Extensions", true, () => { +function runTestExtensionTests() { // This will only work if the test extension is loaded into code-server. test("should have access to VSCODE_PROXY_URI", async ({ codeServerPage }) => { const address = await codeServerPage.address() @@ -9,4 +9,12 @@ describe("Extensions", true, () => { await codeServerPage.page.waitForSelector(`text=${address}/proxy/{{port}}`) }) +} + +describe("Extensions", true, [], () => { + runTestExtensionTests() +}) + +describe("Extensions with --cert", true, ["--cert"], () => { + runTestExtensionTests() }) diff --git a/test/e2e/globalSetup.test.ts b/test/e2e/globalSetup.test.ts index 8b4589b15..554eb945f 100644 --- a/test/e2e/globalSetup.test.ts +++ b/test/e2e/globalSetup.test.ts @@ -2,7 +2,7 @@ import { describe, test, expect } from "./baseFixture" // This test is to make sure the globalSetup works as expected // meaning globalSetup ran and stored the storageState -describe("globalSetup", true, () => { +describe("globalSetup", true, [], () => { test("should keep us logged in using the storageState", async ({ codeServerPage }) => { // Make sure the editor actually loaded expect(await codeServerPage.isEditorVisible()).toBe(true) diff --git a/test/e2e/login.test.ts b/test/e2e/login.test.ts index bc9d5e8e9..3462286f4 100644 --- a/test/e2e/login.test.ts +++ b/test/e2e/login.test.ts @@ -1,7 +1,7 @@ import { PASSWORD } from "../utils/constants" import { describe, test, expect } from "./baseFixture" -describe("login", false, () => { +describe("login", false, [], () => { test("should see the login page", async ({ codeServerPage }) => { // It should send us to the login page expect(await codeServerPage.page.title()).toBe("code-server login") diff --git a/test/e2e/logout.test.ts b/test/e2e/logout.test.ts index 70c26160c..b9c71a446 100644 --- a/test/e2e/logout.test.ts +++ b/test/e2e/logout.test.ts @@ -1,7 +1,7 @@ // NOTE@jsjoeio commenting out until we can figure out what's wrong // import { describe, test, expect } from "./baseFixture" -// describe("logout", true, () => { +// describe("logout", true, [], () => { // test("should be able logout", async ({ codeServerPage }) => { // // Recommended by Playwright for async navigation // // https://github.com/microsoft/playwright/issues/1987#issuecomment-620182151 diff --git a/test/e2e/models/CodeServer.ts b/test/e2e/models/CodeServer.ts index 7ae8f5b76..62d0218e3 100644 --- a/test/e2e/models/CodeServer.ts +++ b/test/e2e/models/CodeServer.ts @@ -31,7 +31,7 @@ export class CodeServer { public readonly logger: Logger private closed = false - constructor(name: string) { + constructor(name: string, private readonly codeServerArgs: string[]) { this.logger = logger.named(name) } @@ -78,6 +78,7 @@ export class CodeServer { "node", [ process.env.CODE_SERVER_TEST_ENTRY || ".", + ...this.codeServerArgs, // Using port zero will spawn on a random port. "--bind-addr", "127.0.0.1:0", diff --git a/test/e2e/openHelpAbout.test.ts b/test/e2e/openHelpAbout.test.ts index 3caa10c0c..0b7ed9fea 100644 --- a/test/e2e/openHelpAbout.test.ts +++ b/test/e2e/openHelpAbout.test.ts @@ -1,6 +1,6 @@ import { describe, test, expect } from "./baseFixture" -describe("Open Help > About", true, () => { +describe("Open Help > About", true, [], () => { test("should see code-server version in about dialog", async ({ codeServerPage }) => { // Open using the menu. await codeServerPage.navigateMenus(["Help", "About"]) diff --git a/test/e2e/terminal.test.ts b/test/e2e/terminal.test.ts index b5b2d90c9..90ea880a4 100644 --- a/test/e2e/terminal.test.ts +++ b/test/e2e/terminal.test.ts @@ -4,7 +4,7 @@ import util from "util" import { clean, tmpdir } from "../utils/helpers" import { describe, expect, test } from "./baseFixture" -describe("Integrated Terminal", true, () => { +describe("Integrated Terminal", true, [], () => { const testName = "integrated-terminal" test.beforeAll(async () => { await clean(testName)