Skip to content

Commit e3e9f05

Browse files
authored
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
1 parent b26cce5 commit e3e9f05

11 files changed

+34
-13
lines changed

package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
"release:github-draft": "./ci/build/release-github-draft.sh",
1818
"release:github-assets": "./ci/build/release-github-assets.sh",
1919
"release:prep": "./ci/build/release-prep.sh",
20-
"test:e2e": "./ci/dev/test-e2e.sh",
20+
"test:e2e": "VSCODE_IPC_HOOK_CLI= ./ci/dev/test-e2e.sh",
2121
"test:standalone-release": "./ci/build/test-standalone-release.sh",
2222
"test:unit": "./ci/dev/test-unit.sh --forceExit --detectOpenHandles",
2323
"test:scripts": "./ci/dev/test-scripts.sh",

src/node/routes/vscode.ts

+6-2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { logError } from "../../common/util"
55
import { toVsCodeArgs } from "../cli"
66
import { isDevMode } from "../constants"
77
import { authenticated, ensureAuthenticated, redirect, self } from "../http"
8+
import { SocketProxyProvider } from "../socket"
89
import { loadAMDModule } from "../util"
910
import { Router as WsRouter } from "../wsRouter"
1011
import { errorHandler } from "./errors"
@@ -13,6 +14,7 @@ export class CodeServerRouteWrapper {
1314
/** Assigned in `ensureCodeServerLoaded` */
1415
private _codeServerMain!: CodeServerLib.IServerAPI
1516
private _wsRouterWrapper = WsRouter()
17+
private _socketProxyProvider = new SocketProxyProvider()
1618
public router = express.Router()
1719

1820
public get wsRouter() {
@@ -77,9 +79,10 @@ export class CodeServerRouteWrapper {
7779
}
7880

7981
private $proxyWebsocket = async (req: WebsocketRequest) => {
80-
this._codeServerMain.handleUpgrade(req, req.socket)
82+
const wrappedSocket = await this._socketProxyProvider.createProxy(req.ws)
83+
this._codeServerMain.handleUpgrade(req, wrappedSocket)
8184

82-
req.socket.resume()
85+
req.ws.resume()
8386
}
8487

8588
//#endregion
@@ -130,5 +133,6 @@ export class CodeServerRouteWrapper {
130133

131134
dispose() {
132135
this._codeServerMain?.dispose()
136+
this._socketProxyProvider.stop()
133137
}
134138
}

test/e2e/baseFixture.ts

+10-2
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,15 @@ import { CodeServer, CodeServerPage } from "./models/CodeServer"
99
*
1010
* If `includeCredentials` is `true` page requests will be authenticated.
1111
*/
12-
export const describe = (name: string, includeCredentials: boolean, fn: (codeServer: CodeServer) => void) => {
12+
export const describe = (
13+
name: string,
14+
includeCredentials: boolean,
15+
codeServerArgs: string[],
16+
fn: (codeServer: CodeServer) => void,
17+
) => {
1318
test.describe(name, () => {
1419
// This will spawn on demand so nothing is necessary on before.
15-
const codeServer = new CodeServer(name)
20+
const codeServer = new CodeServer(name, codeServerArgs)
1621

1722
// Kill code-server after the suite has ended. This may happen even without
1823
// doing it explicitly but it seems prudent to be sure.
@@ -36,6 +41,9 @@ export const describe = (name: string, includeCredentials: boolean, fn: (codeSer
3641
authenticated: includeCredentials,
3742
// This provides a cookie that authenticates with code-server.
3843
storageState: includeCredentials ? storageState : {},
44+
// NOTE@jsjoeio some tests use --cert which uses a self-signed certificate
45+
// without this option, those tests will fail.
46+
ignoreHTTPSErrors: true,
3947
})
4048

4149
fn(codeServer)

test/e2e/codeServer.test.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { describe, test, expect } from "./baseFixture"
22

3-
describe("CodeServer", true, () => {
3+
describe("CodeServer", true, [], () => {
44
test("should navigate to home page", async ({ codeServerPage }) => {
55
// We navigate codeServer before each test
66
// and we start the test with a storage state

test/e2e/extensions.test.ts

+9-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { describe, test } from "./baseFixture"
22

3-
describe("Extensions", true, () => {
3+
function runTestExtensionTests() {
44
// This will only work if the test extension is loaded into code-server.
55
test("should have access to VSCODE_PROXY_URI", async ({ codeServerPage }) => {
66
const address = await codeServerPage.address()
@@ -9,4 +9,12 @@ describe("Extensions", true, () => {
99

1010
await codeServerPage.page.waitForSelector(`text=${address}/proxy/{{port}}`)
1111
})
12+
}
13+
14+
describe("Extensions", true, [], () => {
15+
runTestExtensionTests()
16+
})
17+
18+
describe("Extensions with --cert", true, ["--cert"], () => {
19+
runTestExtensionTests()
1220
})

test/e2e/globalSetup.test.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { describe, test, expect } from "./baseFixture"
22

33
// This test is to make sure the globalSetup works as expected
44
// meaning globalSetup ran and stored the storageState
5-
describe("globalSetup", true, () => {
5+
describe("globalSetup", true, [], () => {
66
test("should keep us logged in using the storageState", async ({ codeServerPage }) => {
77
// Make sure the editor actually loaded
88
expect(await codeServerPage.isEditorVisible()).toBe(true)

test/e2e/login.test.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { PASSWORD } from "../utils/constants"
22
import { describe, test, expect } from "./baseFixture"
33

4-
describe("login", false, () => {
4+
describe("login", false, [], () => {
55
test("should see the login page", async ({ codeServerPage }) => {
66
// It should send us to the login page
77
expect(await codeServerPage.page.title()).toBe("code-server login")

test/e2e/logout.test.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// NOTE@jsjoeio commenting out until we can figure out what's wrong
22
// import { describe, test, expect } from "./baseFixture"
33

4-
// describe("logout", true, () => {
4+
// describe("logout", true, [], () => {
55
// test("should be able logout", async ({ codeServerPage }) => {
66
// // Recommended by Playwright for async navigation
77
// // https://github.com/microsoft/playwright/issues/1987#issuecomment-620182151

test/e2e/models/CodeServer.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ export class CodeServer {
3131
public readonly logger: Logger
3232
private closed = false
3333

34-
constructor(name: string) {
34+
constructor(name: string, private readonly codeServerArgs: string[]) {
3535
this.logger = logger.named(name)
3636
}
3737

@@ -78,6 +78,7 @@ export class CodeServer {
7878
"node",
7979
[
8080
process.env.CODE_SERVER_TEST_ENTRY || ".",
81+
...this.codeServerArgs,
8182
// Using port zero will spawn on a random port.
8283
"--bind-addr",
8384
"127.0.0.1:0",

test/e2e/openHelpAbout.test.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { describe, test, expect } from "./baseFixture"
22

3-
describe("Open Help > About", true, () => {
3+
describe("Open Help > About", true, [], () => {
44
test("should see code-server version in about dialog", async ({ codeServerPage }) => {
55
// Open using the menu.
66
await codeServerPage.navigateMenus(["Help", "About"])

test/e2e/terminal.test.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import util from "util"
44
import { clean, tmpdir } from "../utils/helpers"
55
import { describe, expect, test } from "./baseFixture"
66

7-
describe("Integrated Terminal", true, () => {
7+
describe("Integrated Terminal", true, [], () => {
88
const testName = "integrated-terminal"
99
test.beforeAll(async () => {
1010
await clean(testName)

0 commit comments

Comments
 (0)