diff --git a/extensions/theme-seti/build/update-icon-theme.js b/extensions/theme-seti/build/update-icon-theme.js index d4da9661108..5c26ac82534 100644 --- a/extensions/theme-seti/build/update-icon-theme.js +++ b/extensions/theme-seti/build/update-icon-theme.js @@ -8,7 +8,6 @@ const path = require('path'); const fs = require('fs'); const https = require('https'); -const url = require('url'); const minimatch = require('minimatch'); // list of languagesId not shipped with VSCode. The information is used to associate an icon with a language association @@ -97,10 +96,8 @@ function download(source) { return readFile(source); } return new Promise((c, e) => { - const _url = url.parse(source); - const options = { host: _url.host, port: _url.port, path: _url.path, headers: { 'User-Agent': 'NodeJS' } }; let content = ''; - https.get(options, function (response) { + https.get(new URL(source), { headers: { 'User-Agent': 'NodeJS' } }, function (response) { response.on('data', function (data) { content += data.toString(); }).on('end', function () { @@ -474,4 +471,3 @@ if (path.basename(process.argv[1]) === 'update-icon-theme.js') { } - diff --git a/src/vs/server/node/remoteExtensionHostAgentServer.ts b/src/vs/server/node/remoteExtensionHostAgentServer.ts index e7ae07d1dca..119a8bc6d6a 100644 --- a/src/vs/server/node/remoteExtensionHostAgentServer.ts +++ b/src/vs/server/node/remoteExtensionHostAgentServer.ts @@ -8,7 +8,6 @@ import type * as http from 'http'; import * as net from 'net'; import { createRequire } from 'node:module'; import { performance } from 'perf_hooks'; -import * as url from 'url'; import { VSBuffer } from '../../base/common/buffer.js'; import { CharCode } from '../../base/common/charCode.js'; import { isSigPipeError, onUnexpectedError, setUnexpectedErrorHandler } from '../../base/common/errors.js'; @@ -42,6 +41,16 @@ import { setupServerServices, SocketServer } from './serverServices.js'; import { CacheControl, serveError, serveFile, WebClientServer } from './webClientServer.js'; const require = createRequire(import.meta.url); +function parseRequestUrl(requestUrl: string): URL | undefined { + try { + return requestUrl.startsWith('/') + ? new URL(`http://localhost${requestUrl}`) + : new URL(requestUrl); + } catch { + return undefined; + } +} + declare namespace vsda { // the signer is a native module that for historical reasons uses a lower case class name // eslint-disable-next-line @typescript-eslint/naming-convention @@ -112,7 +121,10 @@ class RemoteExtensionHostAgentServer extends Disposable implements IServerAPI { return serveError(req, res, 400, `Bad request.`); } - const parsedUrl = url.parse(req.url, true); + const parsedUrl = parseRequestUrl(req.url); + if (!parsedUrl) { + return serveError(req, res, 400, `Bad request.`); + } let pathname = parsedUrl.pathname; if (!pathname) { @@ -141,7 +153,7 @@ class RemoteExtensionHostAgentServer extends Disposable implements IServerAPI { return void res.end('OK'); } - if (!httpRequestHasValidConnectionToken(this._connectionToken, req, parsedUrl)) { + if (!httpRequestHasValidConnectionToken(this._connectionToken, req, parsedUrl.searchParams)) { // invalid connection token return serveError(req, res, 403, `Forbidden.`); } @@ -149,10 +161,11 @@ class RemoteExtensionHostAgentServer extends Disposable implements IServerAPI { if (pathname === '/vscode-remote-resource') { // Handle HTTP requests for resources rendered in the rich client (images, fonts, etc.) // These resources could be files shipped with extensions or even workspace files. - const desiredPath = parsedUrl.query['path']; - if (typeof desiredPath !== 'string') { + const desiredPaths = parsedUrl.searchParams.getAll('path'); + if (desiredPaths.length !== 1) { return serveError(req, res, 400, `Bad request.`); } + const desiredPath = desiredPaths[0]; let filePath: string; try { @@ -195,14 +208,21 @@ class RemoteExtensionHostAgentServer extends Disposable implements IServerAPI { let skipWebSocketFrames = false; if (req.url) { - const query = url.parse(req.url, true).query; - if (typeof query.reconnectionToken === 'string') { - reconnectionToken = query.reconnectionToken; + const parsedUrl = parseRequestUrl(req.url); + if (!parsedUrl) { + this._logService.warn('WebSocket connection rejected: invalid request URL'); + socket.end('HTTP/1.1 400 Bad Request\r\nConnection: close\r\n\r\n'); + return; } - if (query.reconnection === 'true') { + const query = parsedUrl.searchParams; + const reconnectionTokens = query.getAll('reconnectionToken'); + if (reconnectionTokens.length === 1) { + reconnectionToken = reconnectionTokens[0]; + } + if (query.getAll('reconnection').length === 1 && query.get('reconnection') === 'true') { isReconnection = true; } - if (query.skipWebSocketFrames === 'true') { + if (query.getAll('skipWebSocketFrames').length === 1 && query.get('skipWebSocketFrames') === 'true') { skipWebSocketFrames = true; } } diff --git a/src/vs/server/node/server.cli.ts b/src/vs/server/node/server.cli.ts index 58ff362b98a..4c4ca67c7e0 100644 --- a/src/vs/server/node/server.cli.ts +++ b/src/vs/server/node/server.cli.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import * as fs from 'fs'; -import * as url from 'url'; +import { pathToFileURL } from 'url'; import * as cp from 'child_process'; import type * as http from 'http'; import { cwd } from '../../base/common/process.js'; @@ -394,7 +394,7 @@ async function openInBrowser(args: string[], verbose: boolean) { for (const location of args) { try { if (/^[a-z-]+:\/\/.+/.test(location)) { - uris.push(url.parse(location).href); + uris.push(new URL(location).href); } else { uris.push(pathToURI(location).href); } @@ -480,11 +480,11 @@ function fatal(message: string, err: unknown): void { const preferredCwd = process.env.PWD || cwd(); // prefer process.env.PWD as it does not follow symlinks -function pathToURI(input: string): url.URL { +function pathToURI(input: string): URL { input = input.trim(); input = resolve(preferredCwd, input); - return url.pathToFileURL(input); + return pathToFileURL(input); } function translatePath(input: string, mapFileUri: (input: string) => string, folderURIS: string[], fileURIS: string[]) { diff --git a/src/vs/server/node/serverConnectionToken.ts b/src/vs/server/node/serverConnectionToken.ts index 978d1b5eb41..32dbf4c4edb 100644 --- a/src/vs/server/node/serverConnectionToken.ts +++ b/src/vs/server/node/serverConnectionToken.ts @@ -6,7 +6,6 @@ import * as cookie from 'cookie'; import * as fs from 'fs'; import type * as http from 'http'; -import * as url from 'url'; import * as path from '../../base/common/path.js'; import { generateUuid } from '../../base/common/uuid.js'; import { connectionTokenCookieName, connectionTokenQueryName } from '../../base/common/network.js'; @@ -120,9 +119,10 @@ export async function determineServerConnectionToken(args: ServerParsedArgs): Pr return parseServerConnectionToken(args, readOrGenerateConnectionToken); } -export function requestHasValidConnectionToken(connectionToken: ServerConnectionToken, req: http.IncomingMessage, parsedUrl: url.UrlWithParsedQuery) { +export function requestHasValidConnectionToken(connectionToken: ServerConnectionToken, req: Pick, searchParams: URLSearchParams) { // First check if there is a valid query parameter - if (connectionToken.validate(parsedUrl.query[connectionTokenQueryName])) { + const queryTokens = searchParams.getAll(connectionTokenQueryName); + if (connectionToken.validate(queryTokens.length > 1 ? queryTokens : queryTokens[0])) { return true; } diff --git a/src/vs/server/node/webClientServer.ts b/src/vs/server/node/webClientServer.ts index 7bda5a01a1d..24b6c3dba9f 100644 --- a/src/vs/server/node/webClientServer.ts +++ b/src/vs/server/node/webClientServer.ts @@ -5,7 +5,6 @@ import { createReadStream, promises } from 'fs'; import type * as http from 'http'; -import * as url from 'url'; import * as cookie from 'cookie'; import * as crypto from 'crypto'; import { isEqualOrParent } from '../../base/common/extpath.js'; @@ -139,7 +138,7 @@ export class WebClientServer { * @param parsedUrl The URL to handle, including base and product path * @param pathname The pathname of the URL, without base and product path */ - async handle(req: http.IncomingMessage, res: http.ServerResponse, parsedUrl: url.UrlWithParsedQuery, pathname: string): Promise { + async handle(req: http.IncomingMessage, res: http.ServerResponse, parsedUrl: URL, pathname: string): Promise { try { if (pathname.startsWith(STATIC_PATH) && pathname.charCodeAt(STATIC_PATH.length) === CharCode.Slash) { return this._handleStatic(req, res, pathname.substring(STATIC_PATH.length)); @@ -257,7 +256,7 @@ export class WebClientServer { /** * Handle HTTP requests for / */ - private async _handleRoot(req: http.IncomingMessage, res: http.ServerResponse, parsedUrl: url.UrlWithParsedQuery): Promise { + private async _handleRoot(req: http.IncomingMessage, res: http.ServerResponse, parsedUrl: URL): Promise { const getFirstHeader = (headerName: string) => { const val = req.headers[headerName]; @@ -267,8 +266,9 @@ export class WebClientServer { // Prefix routes with basePath for clients const basePath = getFirstHeader('x-forwarded-prefix') || this._basePath; - const queryConnectionToken = parsedUrl.query[connectionTokenQueryName]; - if (typeof queryConnectionToken === 'string') { + const queryConnectionTokens = parsedUrl.searchParams.getAll(connectionTokenQueryName); + if (queryConnectionTokens.length === 1) { + const queryConnectionToken = queryConnectionTokens[0]; // We got a connection token as a query parameter. // We want to have a clean URL, so we strip it const responseHeaders: Record = Object.create(null); @@ -281,13 +281,10 @@ export class WebClientServer { } ); - const newQuery = Object.create(null); - for (const key in parsedUrl.query) { - if (key !== connectionTokenQueryName) { - newQuery[key] = parsedUrl.query[key]; - } - } - const newLocation = url.format({ pathname: basePath, query: newQuery }); + const newQuery = new URLSearchParams(parsedUrl.searchParams); + newQuery.delete(connectionTokenQueryName); + const queryString = newQuery.toString(); + const newLocation = queryString ? `${basePath}?${queryString}` : basePath; responseHeaders['Location'] = newLocation; res.writeHead(302, responseHeaders); diff --git a/src/vs/server/test/node/serverConnectionToken.test.ts b/src/vs/server/test/node/serverConnectionToken.test.ts index 27e0ac0a407..e9affb736ea 100644 --- a/src/vs/server/test/node/serverConnectionToken.test.ts +++ b/src/vs/server/test/node/serverConnectionToken.test.ts @@ -7,9 +7,10 @@ import assert from 'assert'; import * as fs from 'fs'; import * as os from 'os'; import { join } from '../../../base/common/path.js'; +import { connectionTokenCookieName, connectionTokenQueryName } from '../../../base/common/network.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../base/test/common/utils.js'; import { getRandomTestPath } from '../../../base/test/node/testUtils.js'; -import { parseServerConnectionToken, ServerConnectionToken, ServerConnectionTokenParseError, ServerConnectionTokenType } from '../../node/serverConnectionToken.js'; +import { MandatoryServerConnectionToken, parseServerConnectionToken, requestHasValidConnectionToken, ServerConnectionToken, ServerConnectionTokenParseError, ServerConnectionTokenType } from '../../node/serverConnectionToken.js'; import { ServerParsedArgs } from '../../node/serverEnvironmentService.js'; suite('parseServerConnectionToken', () => { @@ -70,3 +71,27 @@ suite('parseServerConnectionToken', () => { }); }); + +suite('requestHasValidConnectionToken', () => { + ensureNoDisposablesAreLeakedInTestSuite(); + + const connectionToken = new MandatoryServerConnectionToken('valid token'); + + test('validates a decoded query parameter', () => { + const searchParams = new URLSearchParams(`${connectionTokenQueryName}=valid+token`); + + assert.strictEqual(requestHasValidConnectionToken(connectionToken, { headers: {} }, searchParams), true); + }); + + test('rejects repeated query parameters', () => { + const searchParams = new URLSearchParams(`${connectionTokenQueryName}=valid+token&${connectionTokenQueryName}=valid+token`); + + assert.strictEqual(requestHasValidConnectionToken(connectionToken, { headers: {} }, searchParams), false); + }); + + test('falls back to a cookie', () => { + const headers = { cookie: `${connectionTokenCookieName}=valid%20token` }; + + assert.strictEqual(requestHasValidConnectionToken(connectionToken, { headers }, new URLSearchParams()), true); + }); +}); diff --git a/test/integration/browser/src/index.ts b/test/integration/browser/src/index.ts index 7b9e6378541..25e41545fec 100644 --- a/test/integration/browser/src/index.ts +++ b/test/integration/browser/src/index.ts @@ -63,7 +63,7 @@ const height = 900; type BrowserType = 'chromium' | 'firefox' | 'webkit'; type BrowserChannel = 'msedge' | 'chrome'; -async function runTestsInBrowser(browserType: BrowserType, browserChannel: BrowserChannel, endpoint: url.UrlWithStringQuery, server: cp.ChildProcess): Promise { +async function runTestsInBrowser(browserType: BrowserType, browserChannel: BrowserChannel, endpoint: URL, server: cp.ChildProcess): Promise { const browser = await playwright[browserType].launch({ headless: !Boolean(args.debug), channel: browserChannel }); const context = await browser.newContext(); @@ -177,7 +177,7 @@ function consoleLogFn(msg: playwright.ConsoleMessage) { return console.log; } -async function launchServer(browserType: BrowserType, browserChannel: BrowserChannel): Promise<{ endpoint: url.UrlWithStringQuery; server: cp.ChildProcess }> { +async function launchServer(browserType: BrowserType, browserChannel: BrowserChannel): Promise<{ endpoint: URL; server: cp.ChildProcess }> { // Ensure a tmp user-data-dir is used for the tests const tmpDir = tmp.dirSync({ prefix: 't' }); @@ -241,7 +241,7 @@ async function launchServer(browserType: BrowserType, browserChannel: BrowserCha serverProcess.stdout!.on('data', data => { const matches = data.toString('ascii').match(/Web UI available at (.+)/); if (matches !== null) { - c({ endpoint: url.parse(matches[1]), server: serverProcess }); + c({ endpoint: new URL(matches[1]), server: serverProcess }); } }); });