diff --git a/extensions/ql-vscode/CHANGELOG.md b/extensions/ql-vscode/CHANGELOG.md index 61798e510..15cf3b357 100644 --- a/extensions/ql-vscode/CHANGELOG.md +++ b/extensions/ql-vscode/CHANGELOG.md @@ -3,6 +3,7 @@ ## [UNRELEASED] - Remove support for CodeQL CLI versions older than 2.13.5. [#3371](https://github.com/github/vscode-codeql/pull/3371) +- Add a timeout to downloading databases and the CodeQL CLI. These can be changed using the `codeQL.addingDatabases.downloadTimeout` and `codeQL.cli.downloadTimeout` settings respectively. [#3373](https://github.com/github/vscode-codeql/pull/3373) ## 1.12.2 - 14 February 2024 diff --git a/extensions/ql-vscode/package.json b/extensions/ql-vscode/package.json index c75b72ce2..392b28410 100644 --- a/extensions/ql-vscode/package.json +++ b/extensions/ql-vscode/package.json @@ -178,6 +178,11 @@ "type": "string", "default": "", "markdownDescription": "Path to the CodeQL executable that should be used by the CodeQL extension. The executable is named `codeql` on Linux/Mac and `codeql.exe` on Windows. If empty, the extension will look for a CodeQL executable on your shell PATH, or if CodeQL is not on your PATH, download and manage its own CodeQL executable (note: if you later introduce CodeQL on your PATH, the extension will prefer a CodeQL executable it has downloaded itself)." + }, + "codeQL.cli.downloadTimeout": { + "type": "integer", + "default": 10, + "description": "Download timeout in seconds for downloading the CLI distribution." } } }, @@ -376,6 +381,11 @@ "title": "Adding databases", "order": 6, "properties": { + "codeQL.addingDatabases.downloadTimeout": { + "type": "integer", + "default": 10, + "description": "Download timeout in seconds for adding a CodeQL database." + }, "codeQL.addingDatabases.allowHttp": { "type": "boolean", "default": false, diff --git a/extensions/ql-vscode/src/codeql-cli/distribution.ts b/extensions/ql-vscode/src/codeql-cli/distribution.ts index e69fca721..223bf5fe5 100644 --- a/extensions/ql-vscode/src/codeql-cli/distribution.ts +++ b/extensions/ql-vscode/src/codeql-cli/distribution.ts @@ -1,3 +1,4 @@ +import type { WriteStream } from "fs"; import { createWriteStream, mkdtemp, pathExists, remove } from "fs-extra"; import { tmpdir } from "os"; import { delimiter, dirname, join } from "path"; @@ -26,6 +27,8 @@ import { unzipToDirectoryConcurrently } from "../common/unzip-concurrently"; import { reportUnzipProgress } from "../common/vscode/unzip-progress"; import type { Release } from "./distribution/release"; import { ReleasesApiConsumer } from "./distribution/releases-api-consumer"; +import { createTimeoutSignal } from "../common/fetch-stream"; +import { AbortError } from "node-fetch"; /** * distribution.ts @@ -384,15 +387,25 @@ class ExtensionSpecificDistributionManager { ); } - const assetStream = - await this.createReleasesApiConsumer().streamBinaryContentOfAsset( - assets[0], - ); + const { + signal, + onData, + dispose: disposeTimeout, + } = createTimeoutSignal(this.config.downloadTimeout); + const tmpDirectory = await mkdtemp(join(tmpdir(), "vscode-codeql")); + let archiveFile: WriteStream | undefined = undefined; + try { + const assetStream = + await this.createReleasesApiConsumer().streamBinaryContentOfAsset( + assets[0], + signal, + ); + const archivePath = join(tmpDirectory, "distributionDownload.zip"); - const archiveFile = createWriteStream(archivePath); + archiveFile = createWriteStream(archivePath); const contentLength = assetStream.headers.get("content-length"); const totalNumBytes = contentLength @@ -405,12 +418,23 @@ class ExtensionSpecificDistributionManager { progressCallback, ); - await new Promise((resolve, reject) => + assetStream.body.on("data", onData); + + await new Promise((resolve, reject) => { + if (!archiveFile) { + throw new Error("Invariant violation: archiveFile not set"); + } + assetStream.body .pipe(archiveFile) .on("finish", resolve) - .on("error", reject), - ); + .on("error", reject); + + // If an error occurs on the body, we also want to reject the promise (e.g. during a timeout error). + assetStream.body.on("error", reject); + }); + + disposeTimeout(); await this.bumpDistributionFolderIndex(); @@ -427,7 +451,19 @@ class ExtensionSpecificDistributionManager { ) : undefined, ); + } catch (e) { + if (e instanceof AbortError) { + const thrownError = new AbortError("The download timed out."); + thrownError.stack = e.stack; + throw thrownError; + } + + throw e; } finally { + disposeTimeout(); + + archiveFile?.close(); + await remove(tmpDirectory); } } diff --git a/extensions/ql-vscode/src/codeql-cli/distribution/releases-api-consumer.ts b/extensions/ql-vscode/src/codeql-cli/distribution/releases-api-consumer.ts index 28bef77ad..ff482a7b2 100644 --- a/extensions/ql-vscode/src/codeql-cli/distribution/releases-api-consumer.ts +++ b/extensions/ql-vscode/src/codeql-cli/distribution/releases-api-consumer.ts @@ -90,21 +90,28 @@ export class ReleasesApiConsumer { public async streamBinaryContentOfAsset( asset: ReleaseAsset, + signal?: AbortSignal, ): Promise { const apiPath = `/repos/${this.repositoryNwo}/releases/assets/${asset.id}`; - return await this.makeApiCall(apiPath, { - accept: "application/octet-stream", - }); + return await this.makeApiCall( + apiPath, + { + accept: "application/octet-stream", + }, + signal, + ); } protected async makeApiCall( apiPath: string, additionalHeaders: { [key: string]: string } = {}, + signal?: AbortSignal, ): Promise { const response = await this.makeRawRequest( ReleasesApiConsumer.apiBase + apiPath, Object.assign({}, this.defaultHeaders, additionalHeaders), + signal, ); if (!response.ok) { @@ -129,11 +136,13 @@ export class ReleasesApiConsumer { private async makeRawRequest( requestUrl: string, headers: { [key: string]: string }, + signal?: AbortSignal, redirectCount = 0, ): Promise { const response = await fetch(requestUrl, { headers, redirect: "manual", + signal, }); const redirectUrl = response.headers.get("location"); @@ -153,7 +162,12 @@ export class ReleasesApiConsumer { // mechanism is provided. delete headers["authorization"]; } - return await this.makeRawRequest(redirectUrl, headers, redirectCount + 1); + return await this.makeRawRequest( + redirectUrl, + headers, + signal, + redirectCount + 1, + ); } return response; diff --git a/extensions/ql-vscode/src/common/fetch-stream.ts b/extensions/ql-vscode/src/common/fetch-stream.ts new file mode 100644 index 000000000..b60bda83a --- /dev/null +++ b/extensions/ql-vscode/src/common/fetch-stream.ts @@ -0,0 +1,36 @@ +import { clearTimeout } from "node:timers"; + +export function createTimeoutSignal(timeoutSeconds: number): { + signal: AbortSignal; + onData: () => void; + dispose: () => void; +} { + const timeout = timeoutSeconds * 1000; + + const abortController = new AbortController(); + + let timeoutId: NodeJS.Timeout; + + // If we don't get any data within the timeout, abort the download + timeoutId = setTimeout(() => { + abortController.abort(); + }, timeout); + + // If we receive any data within the timeout, reset the timeout + const onData = () => { + clearTimeout(timeoutId); + timeoutId = setTimeout(() => { + abortController.abort(); + }, timeout); + }; + + const dispose = () => { + clearTimeout(timeoutId); + }; + + return { + signal: abortController.signal, + onData, + dispose, + }; +} diff --git a/extensions/ql-vscode/src/config.ts b/extensions/ql-vscode/src/config.ts index 9aa54ad33..b3a22e52a 100644 --- a/extensions/ql-vscode/src/config.ts +++ b/extensions/ql-vscode/src/config.ts @@ -93,6 +93,10 @@ const PERSONAL_ACCESS_TOKEN_SETTING = new Setting( "personalAccessToken", DISTRIBUTION_SETTING, ); +const CLI_DOWNLOAD_TIMEOUT_SETTING = new Setting( + "downloadTimeout", + DISTRIBUTION_SETTING, +); const CLI_CHANNEL_SETTING = new Setting("channel", DISTRIBUTION_SETTING); // Query History configuration @@ -118,6 +122,7 @@ export interface DistributionConfig { updateCustomCodeQlPath: (newPath: string | undefined) => Promise; includePrerelease: boolean; personalAccessToken?: string; + downloadTimeout: number; channel: CLIChannel; onDidChangeConfiguration?: Event; } @@ -272,6 +277,10 @@ export class DistributionConfigListener return PERSONAL_ACCESS_TOKEN_SETTING.getValue() || undefined; } + public get downloadTimeout(): number { + return CLI_DOWNLOAD_TIMEOUT_SETTING.getValue() || 10; + } + public async updateCustomCodeQlPath(newPath: string | undefined) { await CUSTOM_CODEQL_PATH_SETTING.updateValue( newPath, @@ -644,8 +653,16 @@ const DEPRECATED_ALLOW_HTTP_SETTING = new Setting( const ADDING_DATABASES_SETTING = new Setting("addingDatabases", ROOT_SETTING); +const DOWNLOAD_TIMEOUT_SETTING = new Setting( + "downloadTimeout", + ADDING_DATABASES_SETTING, +); const ALLOW_HTTP_SETTING = new Setting("allowHttp", ADDING_DATABASES_SETTING); +export function downloadTimeout(): number { + return DOWNLOAD_TIMEOUT_SETTING.getValue() || 10; +} + export function allowHttp(): boolean { return ( ALLOW_HTTP_SETTING.getValue() || diff --git a/extensions/ql-vscode/src/databases/database-fetcher.ts b/extensions/ql-vscode/src/databases/database-fetcher.ts index 91ef4f79d..c9ccbdf09 100644 --- a/extensions/ql-vscode/src/databases/database-fetcher.ts +++ b/extensions/ql-vscode/src/databases/database-fetcher.ts @@ -1,5 +1,5 @@ import type { Response } from "node-fetch"; -import fetch from "node-fetch"; +import fetch, { AbortError } from "node-fetch"; import { zip } from "zip-a-folder"; import type { InputBoxOptions } from "vscode"; import { Uri, window } from "vscode"; @@ -28,11 +28,16 @@ import { } from "../common/github-url-identifier-helper"; import type { Credentials } from "../common/authentication"; import type { AppCommandManager } from "../common/commands"; -import { addDatabaseSourceToWorkspace, allowHttp } from "../config"; +import { + addDatabaseSourceToWorkspace, + allowHttp, + downloadTimeout, +} from "../config"; import { showAndLogInformationMessage } from "../common/logging"; import { AppOctokit } from "../common/octokit"; import { getLanguageDisplayName } from "../common/query-language"; import type { DatabaseOrigin } from "./local-databases/database-origin"; +import { createTimeoutSignal } from "../common/fetch-stream"; /** * Prompts a user to fetch a database from a remote location. Database is assumed to be an archive file. @@ -478,10 +483,33 @@ async function fetchAndUnzip( step: 1, }); - const response = await checkForFailingResponse( - await fetch(databaseUrl, { headers: requestHeaders }), - "Error downloading database", - ); + const { + signal, + onData, + dispose: disposeTimeout, + } = createTimeoutSignal(downloadTimeout()); + + let response: Response; + try { + response = await checkForFailingResponse( + await fetch(databaseUrl, { + headers: requestHeaders, + signal, + }), + "Error downloading database", + ); + } catch (e) { + disposeTimeout(); + + if (e instanceof AbortError) { + const thrownError = new AbortError("The request timed out."); + thrownError.stack = e.stack; + throw thrownError; + } + + throw e; + } + const archiveFileStream = createWriteStream(archivePath); const contentLength = response.headers.get("content-length"); @@ -493,12 +521,34 @@ async function fetchAndUnzip( progress, ); - await new Promise((resolve, reject) => - response.body - .pipe(archiveFileStream) - .on("finish", resolve) - .on("error", reject), - ); + response.body.on("data", onData); + + try { + await new Promise((resolve, reject) => { + response.body + .pipe(archiveFileStream) + .on("finish", resolve) + .on("error", reject); + + // If an error occurs on the body, we also want to reject the promise (e.g. during a timeout error). + response.body.on("error", reject); + }); + } catch (e) { + // Close and remove the file if an error occurs + archiveFileStream.close(() => { + void remove(archivePath); + }); + + if (e instanceof AbortError) { + const thrownError = new AbortError("The download timed out."); + thrownError.stack = e.stack; + throw thrownError; + } + + throw e; + } finally { + disposeTimeout(); + } await readAndUnzip( Uri.file(archivePath).toString(true),