Skip to content

fix: wrap socket in proxy before passing to vscode #4840

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Feb 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
8 changes: 6 additions & 2 deletions src/node/routes/vscode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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() {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -130,5 +133,6 @@ export class CodeServerRouteWrapper {

dispose() {
this._codeServerMain?.dispose()
this._socketProxyProvider.stop()
}
}
12 changes: 10 additions & 2 deletions test/e2e/baseFixture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/codeServer.test.ts
Original file line number Diff line number Diff line change
@@ -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
Expand Down
10 changes: 9 additions & 1 deletion test/e2e/extensions.test.ts
Original file line number Diff line number Diff line change
@@ -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()
Expand All @@ -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()
})
2 changes: 1 addition & 1 deletion test/e2e/globalSetup.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/login.test.ts
Original file line number Diff line number Diff line change
@@ -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")
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/logout.test.ts
Original file line number Diff line number Diff line change
@@ -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
Expand Down
3 changes: 2 additions & 1 deletion test/e2e/models/CodeServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/openHelpAbout.test.ts
Original file line number Diff line number Diff line change
@@ -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"])
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/terminal.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down