From 27d9741f36e05ac1ca74a891aa71975c4f68eab3 Mon Sep 17 00:00:00 2001 From: Joe Morris Date: Tue, 16 Jun 2026 11:20:24 -0400 Subject: [PATCH 1/5] fix: support table download with virtual fs uris Signed-off-by: Joe Morris --- .../LibraryNavigator/LibraryModel.ts | 24 +- .../src/components/LibraryNavigator/index.ts | 255 ++++++++- .../LibraryModel.backpressure.test.ts | 536 ++++++++++++++++++ .../LibraryNavigator/LibraryNavigator.test.ts | 531 +++++++++++++++++ 4 files changed, 1320 insertions(+), 26 deletions(-) create mode 100644 client/test/components/LibraryNavigator/LibraryModel.backpressure.test.ts create mode 100644 client/test/components/LibraryNavigator/LibraryNavigator.test.ts diff --git a/client/src/components/LibraryNavigator/LibraryModel.ts b/client/src/components/LibraryNavigator/LibraryModel.ts index 08e802885..480cbf98c 100644 --- a/client/src/components/LibraryNavigator/LibraryModel.ts +++ b/client/src/components/LibraryNavigator/LibraryModel.ts @@ -13,7 +13,6 @@ import { LibraryItemType, TableData, TableQuery, - TableRow, } from "./types"; const sortById = (a: LibraryItem, b: LibraryItem) => a.id.localeCompare(b.id); @@ -92,13 +91,28 @@ class LibraryModel { const headers = data.rows.shift(); if (!hasWrittenHeader) { - fileStream.write(stringArrayToCsvString(headers.columns)); + const canContinue = fileStream.write( + stringArrayToCsvString(headers.columns), + ); + if (!canContinue) { + await new Promise((resolve) => + fileStream.once("drain", resolve), + ); + } hasWrittenHeader = true; } - data.rows.forEach((item: TableRow) => - fileStream.write("\n" + stringArrayToCsvString(item.cells)), - ); + // handle backpressure: wait for drain event when buffer is full + for (const row of data.rows) { + const canContinue = fileStream.write( + "\n" + stringArrayToCsvString(row.cells), + ); + if (!canContinue) { + await new Promise((resolve) => + fileStream.once("drain", resolve), + ); + } + } offset += limit; } while (offset < totalItemCount); diff --git a/client/src/components/LibraryNavigator/index.ts b/client/src/components/LibraryNavigator/index.ts index 1aea94ead..79c005bc3 100644 --- a/client/src/components/LibraryNavigator/index.ts +++ b/client/src/components/LibraryNavigator/index.ts @@ -12,8 +12,9 @@ import { workspace, } from "vscode"; +import { randomBytes, timingSafeEqual } from "crypto"; import { createWriteStream } from "fs"; -import * as path from "path"; +import { createServer } from "http"; import { profileConfig } from "../../commands/profile"; import { Column } from "../../connection/rest/api/compute"; @@ -97,34 +98,54 @@ class LibraryNavigator implements SubscriptionProvider { commands.registerCommand( "SAS.downloadTable", async (item: LibraryItem) => { - let dataFilePath: string = ""; - if ( - env.remoteName !== undefined && - workspace.workspaceFolders && - workspace.workspaceFolders.length > 0 - ) { - // start from 'rootPath' workspace folder - dataFilePath = workspace.workspaceFolders[0].uri.fsPath; - } - dataFilePath = path.join( - dataFilePath, - `${item.library}.${item.name}.csv`.toLocaleLowerCase(), - ); + const defaultFileName = + `${item.library}.${item.name}.csv`.toLocaleLowerCase(); + const defaultUri = + workspace.workspaceFolders && workspace.workspaceFolders.length > 0 + ? Uri.joinPath(workspace.workspaceFolders[0].uri, defaultFileName) + : Uri.file(defaultFileName); - // display save file dialog const uri = await window.showSaveDialog({ - defaultUri: Uri.file(dataFilePath), + defaultUri, }); if (!uri) { return; } - const stream = createWriteStream(uri.fsPath); - await this.libraryDataProvider.writeTableContentsToStream( - stream, - item, - ); + if (uri.scheme === "file") { + try { + await this.libraryDataProvider.writeTableContentsToStream( + createWriteStream(uri.fsPath), + item, + ); + } catch (error) { + window.showErrorMessage( + l10n.t("Failed to download table: {error}", { + error: String( + error?.message || error || "Unknown error", + ).slice(0, 200), + }), + ); + } + return; + } + + try { + // In web/virtual file systems (e.g. vscode-local:// from code-server's "Show Local"), + // stream directly to the browser download pipeline. + const selectedName = uri.path.split("/").pop() || defaultFileName; + await this.streamTableToBrowserDownload(item, selectedName); + } catch (error) { + window.showErrorMessage( + l10n.t("Failed to download table: {error}", { + error: String(error?.message || error || "Unknown error").slice( + 0, + 200, + ), + }), + ); + } }, ), commands.registerCommand( @@ -189,6 +210,198 @@ class LibraryNavigator implements SubscriptionProvider { return new LibraryAdapterFactory().create(activeProfile.connectionType); } + + private async streamTableToBrowserDownload( + item: LibraryItem, + fileName: string, + ): Promise { + const token = randomBytes(24).toString("hex"); + // Preserve Unicode while removing only dangerous characters + const asciiFileName = + fileName + .trim() + // eslint-disable-next-line no-control-regex + .replace(/[\x00-\x1f"\r\n]/g, "") // Remove control chars + quotes + .replace(/\.\.+/g, ".") // Collapse multiple dots (path traversal) + .replace(/^\.+/, "") // Remove leading dots + .slice(0, 250) || "table.csv"; + const encodedFileName = encodeURIComponent(fileName || "table.csv"); + + await new Promise((resolve, reject) => { + let timeoutId: NodeJS.Timeout | undefined; + let streamTimeoutId: NodeJS.Timeout | undefined; + let tokenConsumed = false; + let requestLock = false; + let settled = false; + + const settleResolve = () => { + if (settled) { + return; + } + settled = true; + server.removeListener("error", errorHandler); + resolve(); + }; + + const settleReject = (error: Error) => { + if (settled) { + return; + } + settled = true; + if (timeoutId) { + clearTimeout(timeoutId); + timeoutId = undefined; + } + if (streamTimeoutId) { + clearTimeout(streamTimeoutId); + streamTimeoutId = undefined; + } + server.removeListener("error", errorHandler); + server.close(() => {}); + reject(error); + }; + + const isValidToken = (candidate: string | null): boolean => { + if (!candidate || candidate.length !== token.length) { + return false; + } + return timingSafeEqual(Buffer.from(candidate), Buffer.from(token)); + }; + + const server = createServer((request, response) => { + if (request.method !== "GET") { + response.statusCode = 405; + response.setHeader("Allow", "GET"); + response.end(); + return; + } + + if (tokenConsumed || requestLock) { + response.statusCode = 410; + response.end(); + return; + } + + const requestUrl = request.url + ? new URL(request.url, "http://127.0.0.1") + : undefined; + + if ( + !requestUrl || + requestUrl.pathname !== "/sas-table-download" || + !isValidToken(requestUrl.searchParams.get("token")) + ) { + response.statusCode = 404; + response.end(); + return; + } + + requestLock = true; + tokenConsumed = true; + if (timeoutId) { + clearTimeout(timeoutId); + timeoutId = undefined; + } + server.close(); + + response.setHeader("Content-Type", "text/csv; charset=utf-8"); + response.setHeader("Cache-Control", "no-store"); + response.setHeader("Pragma", "no-cache"); + response.setHeader("X-Content-Type-Options", "nosniff"); + response.setHeader( + "Content-Disposition", + `attachment; filename="${asciiFileName}"; filename*=UTF-8''${encodedFileName}`, + ); + + // Set streaming timeout to prevent indefinite hangs on large tables + streamTimeoutId = setTimeout(() => { + response.destroy(); + server.close(); + settleReject( + new Error(l10n.t("Download streaming timeout exceeded.")), + ); + }, 300_000); // 5 minutes + + this.libraryDataProvider + .writeTableContentsToStream(response, item) + .then(() => { + if (streamTimeoutId) { + clearTimeout(streamTimeoutId); + streamTimeoutId = undefined; + } + if (!response.writableEnded) { + response.end(); + } + settleResolve(); + }) + .catch((error) => { + if (streamTimeoutId) { + clearTimeout(streamTimeoutId); + streamTimeoutId = undefined; + } + if (!response.headersSent) { + response.statusCode = 500; + response.setHeader("Content-Type", "text/plain"); + response.end("Download failed"); + } else { + // Headers already sent - destroy connection to signal error to browser + response.destroy(); + } + settleReject(error); + }); + }); + + const errorHandler = (error: Error) => { + if (timeoutId) { + clearTimeout(timeoutId); + timeoutId = undefined; + } + settleReject(error); + }; + + server.on("error", errorHandler); + + server.listen(0, "127.0.0.1", async () => { + try { + const address = server.address(); + if (!address || typeof address === "string") { + throw new Error(l10n.t("Unable to start download server.")); + } + + // asExternalUri only transforms scheme+host+port — the proxy strips + // path and query. Resolve just the base, then append path+token. + const baseLocalUri = Uri.parse(`http://127.0.0.1:${address.port}`); + const externalBase = await env.asExternalUri(baseLocalUri); + const externalUri = Uri.parse( + `${externalBase.toString(true).replace(/\/+$/, "")}/sas-table-download?token=${token}`, + ); + const opened = await env.openExternal(externalUri); + + if (!opened) { + throw new Error(l10n.t("Failed to open browser download URL.")); + } + + // Timeout guards against the browser never making the request. + // Once the request arrives and streaming begins, settleResolve() + // is called from the request handler instead. + // Increased to 90s for slow networks and corporate proxies + timeoutId = setTimeout(() => { + server.close(); + settleReject( + new Error( + l10n.t( + "Timed out waiting for the browser to start the download.", + ), + ), + ); + }, 90_000); + } catch (error) { + server.close(); + settleReject(error); + } + }); + }); + } } export default LibraryNavigator; diff --git a/client/test/components/LibraryNavigator/LibraryModel.backpressure.test.ts b/client/test/components/LibraryNavigator/LibraryModel.backpressure.test.ts new file mode 100644 index 000000000..ceb4bcdc6 --- /dev/null +++ b/client/test/components/LibraryNavigator/LibraryModel.backpressure.test.ts @@ -0,0 +1,536 @@ +import { expect } from "chai"; +import * as sinon from "sinon"; +import { Writable } from "stream"; + +// Mock writable stream that can simulate backpressure +class MockWritableStream extends Writable { + public chunks: string[] = []; + public ended = false; + public destroyed = false; + private shouldApplyBackpressure = false; + private writeCount = 0; + private backpressureThreshold = 5; + + setBackpressureThreshold(threshold: number): void { + this.backpressureThreshold = threshold; + } + + enableBackpressure(): void { + this.shouldApplyBackpressure = true; + } + + disableBackpressure(): void { + this.shouldApplyBackpressure = false; + } + + triggerDrain(): void { + this.emit("drain"); + } + + _write( + chunk: Buffer | string, + encoding: string, + callback: (error?: Error) => void, + ): void { + this.chunks.push(chunk.toString()); + this.writeCount++; + + // Simulate backpressure after threshold + const hasBackpressure = + this.shouldApplyBackpressure && + this.writeCount > this.backpressureThreshold; + + callback(); + + if (hasBackpressure) { + // Don't signal ready immediately + setTimeout(() => this.triggerDrain(), 10); + } + } + + write( + chunk: Buffer | string, + encodingOrCallback?: + | BufferEncoding + | ((error: Error | null | undefined) => void), + callback?: (error: Error | null | undefined) => void, + ): boolean { + // Handle overload where second parameter is callback + let result: boolean; + + if (typeof encodingOrCallback === "function") { + result = super.write(chunk, encodingOrCallback); + } else if (encodingOrCallback && callback) { + result = super.write(chunk, encodingOrCallback, callback); + } else if (encodingOrCallback) { + result = super.write(chunk, encodingOrCallback); + } else if (callback) { + result = super.write(chunk, callback); + } else { + result = super.write(chunk); + } + + // Return false to signal backpressure + if ( + this.shouldApplyBackpressure && + this.writeCount > this.backpressureThreshold + ) { + return false; + } + + return result; + } + + end( + chunkOrCallback?: Buffer | string | (() => void), + encodingOrCallback?: BufferEncoding | (() => void), + callback?: () => void, + ): this { + // Handle overload variations + let actualChunk: Buffer | string | undefined; + let actualCallback: (() => void) | undefined; + + if (typeof chunkOrCallback === "function") { + actualCallback = chunkOrCallback; + } else { + actualChunk = chunkOrCallback; + if (typeof encodingOrCallback === "function") { + actualCallback = encodingOrCallback; + } else { + actualCallback = callback; + } + } + + if (actualChunk) { + this.chunks.push(actualChunk.toString()); + } + this.ended = true; + if (actualCallback) { + actualCallback(); + } + return this; + } + + destroy(): this { + this.destroyed = true; + return this; + } + + getWrittenData(): string { + return this.chunks.join(""); + } + + getChunkCount(): number { + return this.chunks.length; + } + + reset(): void { + this.chunks = []; + this.ended = false; + this.destroyed = false; + this.writeCount = 0; + } +} + +describe("LibraryModel - Backpressure Handling", function () { + let sandbox: sinon.SinonSandbox; + + beforeEach(() => { + sandbox = sinon.createSandbox(); + }); + + afterEach(() => { + sandbox.restore(); + }); + + describe("Stream Write with Backpressure", () => { + it("should wait for drain event when write returns false", async () => { + const stream = new MockWritableStream(); + stream.enableBackpressure(); + stream.setBackpressureThreshold(2); + + let drainWaited = false; + + // Simulate writing that triggers backpressure + for (let i = 0; i < 5; i++) { + const canContinue = stream.write(`chunk${i}\n`); + if (!canContinue) { + drainWaited = true; + await new Promise((resolve) => { + stream.once("drain", resolve); + }); + } + } + + expect(drainWaited).to.be.true; + expect(stream.getChunkCount()).to.equal(5); + }); + + it("should continue writing after drain event", async () => { + const stream = new MockWritableStream(); + let writesAfterDrain = 0; + + // First write succeeds + stream.write("chunk1\n"); + + // Second write triggers backpressure + stream.enableBackpressure(); + stream.setBackpressureThreshold(1); + + const canContinue = stream.write("chunk2\n"); + expect(canContinue).to.be.false; + + // Wait for drain + await new Promise((resolve) => { + stream.once("drain", () => { + stream.disableBackpressure(); + resolve(); + }); + stream.triggerDrain(); + }); + + // Continue writing after drain + stream.write("chunk3\n"); + writesAfterDrain++; + + expect(writesAfterDrain).to.equal(1); + expect(stream.getChunkCount()).to.equal(3); + }); + + it("should not wait when write returns true", async () => { + const stream = new MockWritableStream(); + stream.disableBackpressure(); + + let drainWaited = false; + + for (let i = 0; i < 10; i++) { + const canContinue = stream.write(`chunk${i}\n`); + if (!canContinue) { + drainWaited = true; + await new Promise((resolve) => { + stream.once("drain", resolve); + }); + } + } + + expect(drainWaited).to.be.false; + expect(stream.getChunkCount()).to.equal(10); + }); + }); + + describe("Header Write with Backpressure", () => { + it("should handle backpressure on header write", async () => { + const stream = new MockWritableStream(); + stream.enableBackpressure(); + stream.setBackpressureThreshold(0); // Trigger immediately + + let hasWrittenHeader = false; + const headers = ["col1", "col2", "col3"]; + const headerString = `"${headers.join('","')}"`; + + const canContinue = stream.write(headerString); + if (!canContinue) { + await new Promise((resolve) => { + stream.once("drain", resolve); + }); + stream.triggerDrain(); + } + hasWrittenHeader = true; + + expect(hasWrittenHeader).to.be.true; + expect(stream.getChunkCount()).to.equal(1); + }); + + it("should write header before rows", async () => { + const stream = new MockWritableStream(); + let hasWrittenHeader = false; + + // Write header + const canContinue = stream.write("col1,col2,col3\n"); + if (!canContinue) { + await new Promise((resolve) => { + stream.once("drain", resolve); + }); + } + hasWrittenHeader = true; + + // Write rows + for (let i = 0; i < 3; i++) { + stream.write(`row${i}\n`); + } + + const data = stream.getWrittenData(); + expect(hasWrittenHeader).to.be.true; + expect(data).to.match(/^col1,col2,col3\n/); + expect(stream.getChunkCount()).to.equal(4); + }); + }); + + describe("Row Writing with Backpressure", () => { + it("should handle backpressure for each row", async () => { + const stream = new MockWritableStream(); + stream.enableBackpressure(); + stream.setBackpressureThreshold(3); + + const rows = [ + { cells: ["a", "b", "c"] }, + { cells: ["d", "e", "f"] }, + { cells: ["g", "h", "i"] }, + { cells: ["j", "k", "l"] }, + { cells: ["m", "n", "o"] }, + ]; + + let backpressureHits = 0; + + for (const row of rows) { + const rowString = "\n" + row.cells.join(","); + const canContinue = stream.write(rowString); + if (!canContinue) { + backpressureHits++; + await new Promise((resolve) => { + stream.once("drain", resolve); + }); + stream.triggerDrain(); + } + } + + expect(backpressureHits).to.be.greaterThan(0); + expect(stream.getChunkCount()).to.equal(5); + }); + + it("should preserve data integrity during backpressure", async () => { + const stream = new MockWritableStream(); + stream.enableBackpressure(); + stream.setBackpressureThreshold(2); + + const expectedData = ["row1", "row2", "row3", "row4", "row5"]; + + for (const data of expectedData) { + const canContinue = stream.write(data + "\n"); + if (!canContinue) { + await new Promise((resolve) => { + stream.once("drain", resolve); + }); + stream.triggerDrain(); + } + } + + const written = stream.getWrittenData(); + expectedData.forEach((data) => { + expect(written).to.include(data); + }); + }); + }); + + describe("Memory Management", () => { + it("should not accumulate data in memory during backpressure", async () => { + const stream = new MockWritableStream(); + stream.enableBackpressure(); + stream.setBackpressureThreshold(5); + + const initialMemory = process.memoryUsage().heapUsed; + const largeChunkCount = 100; + + for (let i = 0; i < largeChunkCount; i++) { + const canContinue = stream.write("x".repeat(100)); + if (!canContinue) { + await new Promise((resolve) => { + stream.once("drain", resolve); + }); + stream.triggerDrain(); + } + } + + const finalMemory = process.memoryUsage().heapUsed; + const memoryIncrease = finalMemory - initialMemory; + + // Memory increase should be reasonable (less than 10MB for test data) + expect(memoryIncrease).to.be.lessThan(10 * 1024 * 1024); + }); + + it("should handle large datasets with multiple backpressure cycles", async () => { + const stream = new MockWritableStream(); + stream.enableBackpressure(); + stream.setBackpressureThreshold(10); + + let backpressureCycles = 0; + const rowCount = 100; + + for (let i = 0; i < rowCount; i++) { + const canContinue = stream.write(`row${i},data${i},value${i}\n`); + if (!canContinue) { + backpressureCycles++; + await new Promise((resolve) => { + stream.once("drain", resolve); + }); + stream.triggerDrain(); + } + } + + expect(backpressureCycles).to.be.greaterThan(0); + expect(stream.getChunkCount()).to.equal(rowCount); + }); + }); + + describe("Cancellation During Backpressure", () => { + it("should respect cancellation token during write", async () => { + const stream = new MockWritableStream(); + let cancelled = false; + + // Simulate cancellation + const cancellationCallback = () => { + stream.destroy(); + cancelled = true; + }; + + // Write some data + stream.write("chunk1\n"); + + // Simulate cancellation + cancellationCallback(); + + expect(stream.destroyed).to.be.true; + expect(cancelled).to.be.true; + }); + + it("should stop writing on cancellation", async () => { + const stream = new MockWritableStream(); + let cancelled = false; + + for (let i = 0; i < 10; i++) { + if (cancelled) { + break; + } + + stream.write(`chunk${i}\n`); + + // Simulate cancellation after 5 writes + if (i === 4) { + stream.destroy(); + cancelled = true; + } + } + + expect(stream.destroyed).to.be.true; + expect(stream.getChunkCount()).to.equal(5); + }); + }); + + describe("Error Handling During Backpressure", () => { + it("should handle write errors gracefully", async () => { + const stream = new MockWritableStream(); + let errorCaught = false; + + try { + stream.write("valid data\n"); + + // Simulate error + stream.destroy(); + + if (stream.destroyed) { + throw new Error("Stream destroyed"); + } + } catch { + errorCaught = true; + } + + expect(errorCaught).to.be.true; + }); + + it("should cleanup on error during drain wait", async () => { + const stream = new MockWritableStream(); + stream.enableBackpressure(); + stream.setBackpressureThreshold(0); + + let cleanedUp = false; + + try { + const canContinue = stream.write("data\n"); + if (!canContinue) { + const drainPromise = new Promise((resolve, reject) => { + stream.once("drain", resolve); + stream.once("error", reject); + }); + + // Simulate error before drain + setTimeout(() => { + stream.emit("error", new Error("Test error")); + }, 5); + + await drainPromise; + } + } catch { + stream.destroy(); + cleanedUp = true; + } + + expect(cleanedUp).to.be.true; + expect(stream.destroyed).to.be.true; + }); + }); + + describe("CSV Formatting with Backpressure", () => { + it("should properly escape quotes in cells", () => { + const input = 'value with "quotes"'; + const escaped = input.replace(/"/g, '""'); + const formatted = `"${escaped}"`; + + expect(formatted).to.equal('"value with ""quotes"""'); + }); + + it("should handle null values", () => { + const value: unknown = null; + const formatted = (value ?? "").toString().replace(/"/g, '""'); + + expect(formatted).to.equal(""); + }); + + it("should convert numbers to strings", () => { + const value: unknown = 12345; + const formatted = (value ?? "").toString().replace(/"/g, '""'); + + expect(formatted).to.equal("12345"); + }); + + it("should format array as CSV row", () => { + const cells = ["col1", "col2", "col3"]; + const formatted = `"${cells + .map((item) => (item ?? "").toString().replace(/"/g, '""')) + .join('","')}"`; + + expect(formatted).to.equal('"col1","col2","col3"'); + }); + }); + + describe("Stream Completion", () => { + it("should end stream after all writes complete", async () => { + const stream = new MockWritableStream(); + + for (let i = 0; i < 5; i++) { + stream.write(`row${i}\n`); + } + + stream.end(); + + expect(stream.ended).to.be.true; + expect(stream.getChunkCount()).to.equal(5); + }); + + it("should not write after stream ends", () => { + const stream = new MockWritableStream(); + + stream.write("data1\n"); + stream.end(); + + let writeAfterEnd = false; + try { + stream.write("data2\n"); + } catch { + writeAfterEnd = true; + } + + expect(stream.ended).to.be.true; + expect(writeAfterEnd).to.be.false; // Node.js Writable doesn't throw, just ignores writes after end + }); + }); +}); diff --git a/client/test/components/LibraryNavigator/LibraryNavigator.test.ts b/client/test/components/LibraryNavigator/LibraryNavigator.test.ts new file mode 100644 index 000000000..57780f489 --- /dev/null +++ b/client/test/components/LibraryNavigator/LibraryNavigator.test.ts @@ -0,0 +1,531 @@ +import { expect } from "chai"; +import { timingSafeEqual } from "crypto"; +import { EventEmitter } from "events"; +import * as sinon from "sinon"; +import { Writable } from "stream"; + +// Mock HTTP server response +class MockResponse extends Writable { + public statusCode?: number; + public headers: Record = {}; + public headersSent = false; + private writeCallback?: () => void; + + setHeader(name: string, value: string): void { + this.headers[name] = value; + } + + end(callback?: () => void): this { + this.headersSent = true; + super.end(); + if (callback) { + callback(); + } + return this; + } + + destroy(): this { + super.destroy(); + return this; + } + + _write( + chunk: Buffer | string, + encoding: string, + callback: (error?: Error) => void, + ): void { + this.headersSent = true; + if (this.writeCallback) { + this.writeCallback(); + } + callback(); + } + + simulateBackpressure(callback: () => void): void { + this.writeCallback = callback; + } +} + +// Mock HTTP server request +class MockRequest extends EventEmitter { + public method = "GET"; + public url = "/sas-table-download?token=validtoken"; +} + +// Mock HTTP server +class MockServer extends EventEmitter { + private requestHandler?: (req: MockRequest, res: MockResponse) => void; + private listening = false; + public address(): { port: number } | null { + return this.listening ? { port: 12345 } : null; + } + + listen(port: number, host: string, callback: () => void): this { + this.listening = true; + setTimeout(() => callback(), 10); + return this; + } + + close(callback?: () => void): this { + this.listening = false; + if (callback) { + setTimeout(callback, 10); + } + return this; + } + + setRequestHandler( + handler: (req: MockRequest, res: MockResponse) => void, + ): void { + this.requestHandler = handler; + } + + simulateRequest(): { request: MockRequest; response: MockResponse } { + const req = new MockRequest(); + const res = new MockResponse(); + if (this.requestHandler) { + this.requestHandler(req, res); + } + return { request: req, response: res }; + } +} + +describe("LibraryNavigator - Browser Download", function () { + let sandbox: sinon.SinonSandbox; + + beforeEach(() => { + sandbox = sinon.createSandbox(); + }); + + afterEach(() => { + sandbox.restore(); + }); + + describe("Filename Sanitization", () => { + const controlCharsPattern = new RegExp( + "[" + String.fromCharCode(0) + "-" + String.fromCharCode(31) + '"\r\n]', + "g", + ); + + it("should remove control characters from filename", () => { + const input = "test\x00\x01\x1ffile.csv"; + const result = input + .trim() + .replace(controlCharsPattern, "") + .replace(/\.\.+/g, ".") + .replace(/^\.+/, ""); + + expect(result).to.equal("testfile.csv"); + }); + + it("should remove quotes from filename", () => { + const input = 'test"file".csv'; + const result = input + .trim() + .replace(controlCharsPattern, "") + .replace(/\.\.+/g, ".") + .replace(/^\.+/, ""); + + expect(result).to.equal("testfile.csv"); + }); + + it("should collapse multiple dots to prevent path traversal", () => { + const input = "test..file...csv"; + const result = input + .trim() + .replace(controlCharsPattern, "") + .replace(/\.\.+/g, ".") + .replace(/^\.+/, ""); + + expect(result).to.equal("test.file.csv"); + }); + + it("should remove leading dots", () => { + const input = "...test.csv"; + const result = input + .trim() + .replace(controlCharsPattern, "") + .replace(/\.\.+/g, ".") + .replace(/^\.+/, ""); + + expect(result).to.equal("test.csv"); + }); + + it("should preserve Unicode characters", () => { + const input = "テスト日本語.csv"; + const result = input + .trim() + .replace(controlCharsPattern, "") + .replace(/\.\.+/g, ".") + .replace(/^\.+/, ""); + + expect(result).to.equal("テスト日本語.csv"); + }); + + it("should limit filename length to 250 characters", () => { + const input = "a".repeat(300) + ".csv"; + const result = input + .trim() + .replace(controlCharsPattern, "") + .replace(/\.\.+/g, ".") + .replace(/^\.+/, "") + .slice(0, 250); + + expect(result.length).to.equal(250); + }); + + it("should use fallback name when sanitization results in empty string", () => { + const input = "\x00\x01\x1f"; + const result = + input + .trim() + .replace(controlCharsPattern, "") + .replace(/\.\.+/g, ".") + .replace(/^\.+/, "") + .slice(0, 250) || "table.csv"; + + expect(result).to.equal("table.csv"); + }); + }); + + describe("Token Validation", () => { + it("should use timing-safe comparison for token validation", () => { + const token1 = Buffer.from("abc123"); + const token2 = Buffer.from("abc123"); + const token3 = Buffer.from("xyz789"); + + expect(timingSafeEqual(token1, token2)).to.be.true; + expect(timingSafeEqual(token1, token3)).to.be.false; + }); + + it("should reject tokens of different lengths", () => { + const validToken = "a".repeat(48); // 24 bytes as hex + const candidate = "a".repeat(40); + + const isValid = candidate && candidate.length === validToken.length; + expect(isValid).to.be.false; + }); + + it("should accept valid token", () => { + const validToken = "a".repeat(48); + const candidate = "a".repeat(48); + + const isValid = candidate && candidate.length === validToken.length; + expect(isValid).to.be.true; + }); + }); + + describe("Error Message Sanitization", () => { + it("should limit error messages to 200 characters", () => { + const longError = new Error("a".repeat(500)); + const sanitized = String(longError?.message || "Unknown error").slice( + 0, + 200, + ); + + expect(sanitized.length).to.equal(200); + }); + + it("should handle undefined errors", () => { + const error: unknown = undefined; + const errorObj = + error && typeof error === "object" && "message" in error ? error : null; + const sanitized = String( + errorObj?.message || error || "Unknown error", + ).slice(0, 200); + + expect(sanitized).to.equal("Unknown error"); + }); + + it("should handle null errors", () => { + const error: unknown = null; + const errorObj = + error && typeof error === "object" && "message" in error ? error : null; + const sanitized = String( + errorObj?.message || error || "Unknown error", + ).slice(0, 200); + + expect(sanitized).to.equal("Unknown error"); + }); + + it("should handle errors without message property", () => { + const error: unknown = { code: "ERR_UNKNOWN" }; + const errorObj = + error && typeof error === "object" && "message" in error ? error : null; + const sanitized = String( + errorObj?.message || error || "Unknown error", + ).slice(0, 200); + + expect(sanitized).to.equal("[object Object]"); + }); + }); + + describe("HTTP Response Headers", () => { + it("should set correct Content-Type header", () => { + const response = new MockResponse(); + response.setHeader("Content-Type", "text/csv; charset=utf-8"); + + expect(response.headers["Content-Type"]).to.equal( + "text/csv; charset=utf-8", + ); + }); + + it("should set security headers", () => { + const response = new MockResponse(); + response.setHeader("Cache-Control", "no-store"); + response.setHeader("Pragma", "no-cache"); + response.setHeader("X-Content-Type-Options", "nosniff"); + + expect(response.headers["Cache-Control"]).to.equal("no-store"); + expect(response.headers.Pragma).to.equal("no-cache"); + expect(response.headers["X-Content-Type-Options"]).to.equal("nosniff"); + }); + + it("should set Content-Disposition with ASCII and UTF-8 filenames", () => { + const asciiName = "test.csv"; + const utf8Name = encodeURIComponent("テスト.csv"); + const header = `attachment; filename="${asciiName}"; filename*=UTF-8''${utf8Name}`; + + const response = new MockResponse(); + response.setHeader("Content-Disposition", header); + + expect(response.headers["Content-Disposition"]).to.include("attachment"); + expect(response.headers["Content-Disposition"]).to.include(asciiName); + expect(response.headers["Content-Disposition"]).to.include("UTF-8"); + }); + }); + + describe("HTTP Status Codes", () => { + it("should return 405 for non-GET requests", () => { + const response = new MockResponse(); + const method: string = "POST"; + + if (method !== "GET") { + response.statusCode = 405; + response.setHeader("Allow", "GET"); + } + + expect(response.statusCode).to.equal(405); + expect(response.headers.Allow).to.equal("GET"); + }); + + it("should return 410 for consumed tokens", () => { + const response = new MockResponse(); + const tokenConsumed = true; + + if (tokenConsumed) { + response.statusCode = 410; + } + + expect(response.statusCode).to.equal(410); + }); + + it("should return 404 for invalid tokens", () => { + const response = new MockResponse(); + const validToken = false; + + if (!validToken) { + response.statusCode = 404; + } + + expect(response.statusCode).to.equal(404); + }); + + it("should return 500 on server errors before headers sent", () => { + const response = new MockResponse(); + + if (!response.headersSent) { + response.statusCode = 500; + response.setHeader("Content-Type", "text/plain"); + } + + expect(response.statusCode).to.equal(500); + expect(response.headers["Content-Type"]).to.equal("text/plain"); + }); + }); + + describe("Request Lock Race Condition Protection", () => { + it("should prevent concurrent access with request lock", () => { + let tokenConsumed = false; + let requestLock = false; + + // First request + const canProcess1 = !(tokenConsumed || requestLock); + if (canProcess1) { + requestLock = true; + tokenConsumed = true; + } + + expect(canProcess1).to.be.true; + + // Second concurrent request + const canProcess2 = !(tokenConsumed || requestLock); + + expect(canProcess2).to.be.false; + }); + + it("should set both flags atomically", () => { + let tokenConsumed = false; + let requestLock = false; + + // Simulate checking and setting + if (!(tokenConsumed || requestLock)) { + requestLock = true; // Set lock first + tokenConsumed = true; // Then set consumed + } + + expect(requestLock).to.be.true; + expect(tokenConsumed).to.be.true; + }); + }); + + describe("Response Stream Error Handling", () => { + it("should set error response before headers sent", () => { + const response = new MockResponse(); + + if (!response.headersSent) { + response.statusCode = 500; + response.setHeader("Content-Type", "text/plain"); + response.end(); + } + + expect(response.statusCode).to.equal(500); + expect(response.writableEnded).to.be.true; + }); + + it("should destroy response if headers already sent", () => { + const response = new MockResponse(); + response.headersSent = true; + + if (response.headersSent) { + response.destroy(); + } + + expect(response.destroyed).to.be.true; + }); + + it("should end response only if not already ended", () => { + const response = new MockResponse(); + + if (!response.writableEnded) { + response.end(); + } + + expect(response.writableEnded).to.be.true; + + // Should not throw on second attempt + if (!response.writableEnded) { + response.end(); + } + }); + }); + + describe("Timeout Handling", () => { + it("should clear timeouts on successful completion", () => { + let timeoutId: NodeJS.Timeout | undefined = setTimeout(() => {}, 1000); + let streamTimeoutId: NodeJS.Timeout | undefined = setTimeout( + () => {}, + 1000, + ); + + if (timeoutId) { + clearTimeout(timeoutId); + timeoutId = undefined; + } + if (streamTimeoutId) { + clearTimeout(streamTimeoutId); + streamTimeoutId = undefined; + } + + expect(timeoutId).to.be.undefined; + expect(streamTimeoutId).to.be.undefined; + }); + + it("should clear timeouts on error", () => { + let timeoutId: NodeJS.Timeout | undefined = setTimeout(() => {}, 1000); + let streamTimeoutId: NodeJS.Timeout | undefined = setTimeout( + () => {}, + 1000, + ); + + // Simulate error cleanup + if (timeoutId) { + clearTimeout(timeoutId); + timeoutId = undefined; + } + if (streamTimeoutId) { + clearTimeout(streamTimeoutId); + streamTimeoutId = undefined; + } + + expect(timeoutId).to.be.undefined; + expect(streamTimeoutId).to.be.undefined; + }); + }); + + describe("Server Cleanup", () => { + it("should close server in error paths", (done) => { + const server = new MockServer(); + + server.listen(0, "127.0.0.1", () => { + expect(server.address()).to.not.be.null; + + server.close(() => { + expect(server.address()).to.be.null; + done(); + }); + }); + }); + + it("should remove error listener on completion", () => { + const server = new MockServer(); + const errorHandler = () => {}; + + server.on("error", errorHandler); + expect(server.listenerCount("error")).to.equal(1); + + server.removeListener("error", errorHandler); + expect(server.listenerCount("error")).to.equal(0); + }); + }); + + describe("Settle Pattern Idempotency", () => { + it("should prevent double resolution", () => { + let settled = false; + let resolveCount = 0; + + const settleResolve = () => { + if (settled) { + return; + } + settled = true; + resolveCount++; + }; + + settleResolve(); + settleResolve(); + settleResolve(); + + expect(resolveCount).to.equal(1); + }); + + it("should prevent double rejection", () => { + let settled = false; + let rejectCount = 0; + + const settleReject = () => { + if (settled) { + return; + } + settled = true; + rejectCount++; + }; + + settleReject(); + settleReject(); + settleReject(); + + expect(rejectCount).to.equal(1); + }); + }); +}); From efc29aecabc7ccb7c471828fa1916165c691f610 Mon Sep 17 00:00:00 2001 From: Joe Morris Date: Wed, 17 Jun 2026 08:26:53 -0400 Subject: [PATCH 2/5] fix: remove arbitrary timeout Signed-off-by: Joe Morris --- .../src/components/LibraryNavigator/index.ts | 25 +++---------------- 1 file changed, 3 insertions(+), 22 deletions(-) diff --git a/client/src/components/LibraryNavigator/index.ts b/client/src/components/LibraryNavigator/index.ts index 79c005bc3..d4d5f247f 100644 --- a/client/src/components/LibraryNavigator/index.ts +++ b/client/src/components/LibraryNavigator/index.ts @@ -135,6 +135,9 @@ class LibraryNavigator implements SubscriptionProvider { // In web/virtual file systems (e.g. vscode-local:// from code-server's "Show Local"), // stream directly to the browser download pipeline. const selectedName = uri.path.split("/").pop() || defaultFileName; + window.showInformationMessage( + l10n.t("Opening browser to download table..."), + ); await this.streamTableToBrowserDownload(item, selectedName); } catch (error) { window.showErrorMessage( @@ -229,7 +232,6 @@ class LibraryNavigator implements SubscriptionProvider { await new Promise((resolve, reject) => { let timeoutId: NodeJS.Timeout | undefined; - let streamTimeoutId: NodeJS.Timeout | undefined; let tokenConsumed = false; let requestLock = false; let settled = false; @@ -252,10 +254,6 @@ class LibraryNavigator implements SubscriptionProvider { clearTimeout(timeoutId); timeoutId = undefined; } - if (streamTimeoutId) { - clearTimeout(streamTimeoutId); - streamTimeoutId = undefined; - } server.removeListener("error", errorHandler); server.close(() => {}); reject(error); @@ -313,32 +311,15 @@ class LibraryNavigator implements SubscriptionProvider { `attachment; filename="${asciiFileName}"; filename*=UTF-8''${encodedFileName}`, ); - // Set streaming timeout to prevent indefinite hangs on large tables - streamTimeoutId = setTimeout(() => { - response.destroy(); - server.close(); - settleReject( - new Error(l10n.t("Download streaming timeout exceeded.")), - ); - }, 300_000); // 5 minutes - this.libraryDataProvider .writeTableContentsToStream(response, item) .then(() => { - if (streamTimeoutId) { - clearTimeout(streamTimeoutId); - streamTimeoutId = undefined; - } if (!response.writableEnded) { response.end(); } settleResolve(); }) .catch((error) => { - if (streamTimeoutId) { - clearTimeout(streamTimeoutId); - streamTimeoutId = undefined; - } if (!response.headersSent) { response.statusCode = 500; response.setHeader("Content-Type", "text/plain"); From 0ac80373ba913a7ce4e3c00a70b458dd032f7dfa Mon Sep 17 00:00:00 2001 From: Joe Morris Date: Wed, 17 Jun 2026 08:47:50 -0400 Subject: [PATCH 3/5] chore: code cleanup Signed-off-by: Joe Morris --- .../src/components/LibraryNavigator/index.ts | 30 ++++++++++++++----- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/client/src/components/LibraryNavigator/index.ts b/client/src/components/LibraryNavigator/index.ts index d4d5f247f..a38776ca8 100644 --- a/client/src/components/LibraryNavigator/index.ts +++ b/client/src/components/LibraryNavigator/index.ts @@ -31,6 +31,13 @@ import { Messages } from "./const"; import { LibraryAdapter, LibraryItem, TableData } from "./types"; class LibraryNavigator implements SubscriptionProvider { + private static readonly DOWNLOAD_TOKEN_BYTES = 24; // 192-bit token + private static readonly MAX_DOWNLOAD_FILENAME_LENGTH = 250; + private static readonly BROWSER_CONNECTION_TIMEOUT_MS = 60_000; + private static readonly DOWNLOAD_ENDPOINT_PATH = "/sas-library-download"; + private static readonly DOWNLOAD_TOKEN_PARAM = "token"; + private static readonly DEFAULT_DOWNLOAD_FILENAME = "table.csv"; + private libraryDataProvider: LibraryDataProvider; private extensionUri: Uri; private webviewManager: WebViewManager; @@ -218,7 +225,9 @@ class LibraryNavigator implements SubscriptionProvider { item: LibraryItem, fileName: string, ): Promise { - const token = randomBytes(24).toString("hex"); + const token = randomBytes(LibraryNavigator.DOWNLOAD_TOKEN_BYTES).toString( + "hex", + ); // Preserve Unicode while removing only dangerous characters const asciiFileName = fileName @@ -227,8 +236,11 @@ class LibraryNavigator implements SubscriptionProvider { .replace(/[\x00-\x1f"\r\n]/g, "") // Remove control chars + quotes .replace(/\.\.+/g, ".") // Collapse multiple dots (path traversal) .replace(/^\.+/, "") // Remove leading dots - .slice(0, 250) || "table.csv"; - const encodedFileName = encodeURIComponent(fileName || "table.csv"); + .slice(0, LibraryNavigator.MAX_DOWNLOAD_FILENAME_LENGTH) || + LibraryNavigator.DEFAULT_DOWNLOAD_FILENAME; + const encodedFileName = encodeURIComponent( + fileName || LibraryNavigator.DEFAULT_DOWNLOAD_FILENAME, + ); await new Promise((resolve, reject) => { let timeoutId: NodeJS.Timeout | undefined; @@ -286,8 +298,10 @@ class LibraryNavigator implements SubscriptionProvider { if ( !requestUrl || - requestUrl.pathname !== "/sas-table-download" || - !isValidToken(requestUrl.searchParams.get("token")) + requestUrl.pathname !== LibraryNavigator.DOWNLOAD_ENDPOINT_PATH || + !isValidToken( + requestUrl.searchParams.get(LibraryNavigator.DOWNLOAD_TOKEN_PARAM), + ) ) { response.statusCode = 404; response.end(); @@ -354,7 +368,7 @@ class LibraryNavigator implements SubscriptionProvider { const baseLocalUri = Uri.parse(`http://127.0.0.1:${address.port}`); const externalBase = await env.asExternalUri(baseLocalUri); const externalUri = Uri.parse( - `${externalBase.toString(true).replace(/\/+$/, "")}/sas-table-download?token=${token}`, + `${externalBase.toString(true).replace(/\/+$/, "")}${LibraryNavigator.DOWNLOAD_ENDPOINT_PATH}?${LibraryNavigator.DOWNLOAD_TOKEN_PARAM}=${token}`, ); const opened = await env.openExternal(externalUri); @@ -365,7 +379,7 @@ class LibraryNavigator implements SubscriptionProvider { // Timeout guards against the browser never making the request. // Once the request arrives and streaming begins, settleResolve() // is called from the request handler instead. - // Increased to 90s for slow networks and corporate proxies + // Increased to 60s for slow networks and corporate proxies timeoutId = setTimeout(() => { server.close(); settleReject( @@ -375,7 +389,7 @@ class LibraryNavigator implements SubscriptionProvider { ), ), ); - }, 90_000); + }, LibraryNavigator.BROWSER_CONNECTION_TIMEOUT_MS); } catch (error) { server.close(); settleReject(error); From d057b613dc637356b21f6c382b0ac1acab73cc63 Mon Sep 17 00:00:00 2001 From: Joe Morris Date: Wed, 17 Jun 2026 08:56:32 -0400 Subject: [PATCH 4/5] chore: add copyright to new files Signed-off-by: Joe Morris --- .../LibraryNavigator/LibraryModel.backpressure.test.ts | 2 ++ .../test/components/LibraryNavigator/LibraryNavigator.test.ts | 2 ++ 2 files changed, 4 insertions(+) diff --git a/client/test/components/LibraryNavigator/LibraryModel.backpressure.test.ts b/client/test/components/LibraryNavigator/LibraryModel.backpressure.test.ts index ceb4bcdc6..eae9246b7 100644 --- a/client/test/components/LibraryNavigator/LibraryModel.backpressure.test.ts +++ b/client/test/components/LibraryNavigator/LibraryModel.backpressure.test.ts @@ -1,3 +1,5 @@ +// Copyright © 2026, SAS Institute Inc., Cary, NC, USA. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 import { expect } from "chai"; import * as sinon from "sinon"; import { Writable } from "stream"; diff --git a/client/test/components/LibraryNavigator/LibraryNavigator.test.ts b/client/test/components/LibraryNavigator/LibraryNavigator.test.ts index 57780f489..a4345081e 100644 --- a/client/test/components/LibraryNavigator/LibraryNavigator.test.ts +++ b/client/test/components/LibraryNavigator/LibraryNavigator.test.ts @@ -1,3 +1,5 @@ +// Copyright © 2026, SAS Institute Inc., Cary, NC, USA. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 import { expect } from "chai"; import { timingSafeEqual } from "crypto"; import { EventEmitter } from "events"; From f665637b111d53598c00f171ef0214a0c4ac0fc2 Mon Sep 17 00:00:00 2001 From: Joe Morris Date: Wed, 1 Jul 2026 12:25:57 -0400 Subject: [PATCH 5/5] chore: cleanup --- .../LibraryNavigator/LibraryModel.ts | 40 +- .../LibraryNavigator/browserDownload.ts | 225 +++++++ .../src/components/LibraryNavigator/index.ts | 236 +------ .../LibraryModel.backpressure.test.ts | 559 ++--------------- .../LibraryNavigator/LibraryNavigator.test.ts | 587 ++++-------------- 5 files changed, 449 insertions(+), 1198 deletions(-) create mode 100644 client/src/components/LibraryNavigator/browserDownload.ts diff --git a/client/src/components/LibraryNavigator/LibraryModel.ts b/client/src/components/LibraryNavigator/LibraryModel.ts index 480cbf98c..3dd94125e 100644 --- a/client/src/components/LibraryNavigator/LibraryModel.ts +++ b/client/src/components/LibraryNavigator/LibraryModel.ts @@ -17,6 +17,24 @@ import { const sortById = (a: LibraryItem, b: LibraryItem) => a.id.localeCompare(b.id); +export function stringArrayToCsvString( + strings: (string | number | null)[], +): string { + return `"${strings + .map((item) => (item ?? "").toString().replace(/"/g, '""')) + .join('","')}"`; +} + +export async function writeWithBackpressure( + stream: Writable, + data: string, +): Promise { + const canContinue = stream.write(data); + if (!canContinue) { + await new Promise((resolve) => stream.once("drain", resolve)); + } +} + class LibraryModel { public constructor(protected libraryAdapter: LibraryAdapter | undefined) {} @@ -62,12 +80,6 @@ class LibraryModel { const { rowCount: totalItemCount, maxNumberOfRowsToRead: limit } = await this.libraryAdapter.getTableRowCount(item); let hasWrittenHeader: boolean = false; - const stringArrayToCsvString = (strings: string[]): string => - `"${strings - .map((item: string | number) => - (item ?? "").toString().replace(/"/g, '""'), - ) - .join('","')}"`; await window.withProgress( { @@ -91,27 +103,19 @@ class LibraryModel { const headers = data.rows.shift(); if (!hasWrittenHeader) { - const canContinue = fileStream.write( + await writeWithBackpressure( + fileStream, stringArrayToCsvString(headers.columns), ); - if (!canContinue) { - await new Promise((resolve) => - fileStream.once("drain", resolve), - ); - } hasWrittenHeader = true; } // handle backpressure: wait for drain event when buffer is full for (const row of data.rows) { - const canContinue = fileStream.write( + await writeWithBackpressure( + fileStream, "\n" + stringArrayToCsvString(row.cells), ); - if (!canContinue) { - await new Promise((resolve) => - fileStream.once("drain", resolve), - ); - } } offset += limit; diff --git a/client/src/components/LibraryNavigator/browserDownload.ts b/client/src/components/LibraryNavigator/browserDownload.ts new file mode 100644 index 000000000..f414bbe8d --- /dev/null +++ b/client/src/components/LibraryNavigator/browserDownload.ts @@ -0,0 +1,225 @@ +// Copyright © 2023, SAS Institute Inc., Cary, NC, USA. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 +import { Uri, env, l10n } from "vscode"; + +import { randomBytes, timingSafeEqual } from "crypto"; +import { createServer } from "http"; + +import LibraryDataProvider from "./LibraryDataProvider"; +import { LibraryItem } from "./types"; + +const DOWNLOAD_TOKEN_BYTES = 24; // 192-bit token +const MAX_DOWNLOAD_FILENAME_LENGTH = 250; +const BROWSER_CONNECTION_TIMEOUT_MS = 60_000; +const DOWNLOAD_ENDPOINT_PATH = "/sas-library-download"; +const DOWNLOAD_TOKEN_PARAM = "token"; +const DEFAULT_DOWNLOAD_FILENAME = "table.csv"; + +export function isValidDownloadToken( + candidate: string | null, + token: string, +): boolean { + if (!candidate || candidate.length !== token.length) { + return false; + } + return timingSafeEqual(Buffer.from(candidate), Buffer.from(token)); +} + +export function sanitizeDownloadFilename( + fileName: string, + maxLength: number, + fallback: string, +): string { + // Preserve Unicode while removing only dangerous characters + return ( + fileName + .trim() + // eslint-disable-next-line no-control-regex + .replace(/[\x00-\x1f"\r\n]/g, "") // Remove control chars + quotes + .replace(/\.\.+/g, ".") // Collapse multiple dots (path traversal) + .replace(/^\.+/, "") // Remove leading dots + .slice(0, maxLength) || fallback + ); +} + +export type DownloadRequestOutcome = + | { readonly outcome: 405 } + | { readonly outcome: 410 } + | { readonly outcome: 404 } + | { readonly outcome: "stream" }; + +export function classifyDownloadRequest( + method: string | undefined, + url: string | undefined, + token: string, + endpointPath: string, + tokenParam: string, + requestLock: boolean, +): DownloadRequestOutcome { + if (method !== "GET") { + return { outcome: 405 }; + } + if (requestLock) { + return { outcome: 410 }; + } + const requestUrl = url ? new URL(url, "http://127.0.0.1") : undefined; + if ( + !requestUrl || + requestUrl.pathname !== endpointPath || + !isValidDownloadToken(requestUrl.searchParams.get(tokenParam), token) + ) { + return { outcome: 404 }; + } + return { outcome: "stream" }; +} + +export async function streamTableToBrowserDownload( + item: LibraryItem, + fileName: string, + libraryDataProvider: LibraryDataProvider, +): Promise { + const token = randomBytes(DOWNLOAD_TOKEN_BYTES).toString("hex"); + const asciiFileName = sanitizeDownloadFilename( + fileName, + MAX_DOWNLOAD_FILENAME_LENGTH, + DEFAULT_DOWNLOAD_FILENAME, + ); + const encodedFileName = encodeURIComponent(asciiFileName); + + await new Promise((resolve, reject) => { + let timeoutId: NodeJS.Timeout | undefined; + let requestLock = false; + let settled = false; + + const settleResolve = () => { + if (settled) { + return; + } + settled = true; + if (timeoutId) { + clearTimeout(timeoutId); + timeoutId = undefined; + } + server.removeListener("error", errorHandler); + resolve(); + }; + + const settleReject = (error: Error) => { + if (settled) { + return; + } + settled = true; + if (timeoutId) { + clearTimeout(timeoutId); + timeoutId = undefined; + } + server.removeListener("error", errorHandler); + server.close(() => {}); + reject(error); + }; + + const server = createServer((request, response) => { + const verdict = classifyDownloadRequest( + request.method, + request.url, + token, + DOWNLOAD_ENDPOINT_PATH, + DOWNLOAD_TOKEN_PARAM, + requestLock, + ); + + if (verdict.outcome !== "stream") { + response.statusCode = verdict.outcome; + if (verdict.outcome === 405) { + response.setHeader("Allow", "GET"); + } + response.end(); + return; + } + + requestLock = true; + if (timeoutId) { + clearTimeout(timeoutId); + timeoutId = undefined; + } + + response.setHeader("Content-Type", "text/csv; charset=utf-8"); + response.setHeader("Cache-Control", "no-store"); + response.setHeader("Pragma", "no-cache"); + response.setHeader("X-Content-Type-Options", "nosniff"); + response.setHeader( + "Content-Disposition", + `attachment; filename="${asciiFileName}"; filename*=UTF-8''${encodedFileName}`, + ); + + libraryDataProvider + .writeTableContentsToStream(response, item) + .then(() => { + if (!response.writableEnded) { + response.end(); + } + settleResolve(); + }) + .catch((error) => { + if (!response.headersSent) { + response.statusCode = 500; + response.setHeader("Content-Type", "text/plain"); + response.end("Download failed"); + } else { + // Headers already sent - destroy connection to signal error to browser + response.destroy(); + } + settleReject(error); + }); + }); + + const errorHandler = (error: Error) => { + if (timeoutId) { + clearTimeout(timeoutId); + timeoutId = undefined; + } + settleReject(error); + }; + + server.on("error", errorHandler); + + server.listen(0, "127.0.0.1", async () => { + try { + const address = server.address(); + if (!address || typeof address === "string") { + throw new Error(l10n.t("Unable to start download server.")); + } + + // asExternalUri only transforms scheme+host+port — the proxy strips + // path and query. Resolve just the base, then append path+token. + const baseLocalUri = Uri.parse(`http://127.0.0.1:${address.port}`); + const externalBase = await env.asExternalUri(baseLocalUri); + const externalUri = Uri.parse( + `${externalBase.toString(true).replace(/\/+$/, "")}${DOWNLOAD_ENDPOINT_PATH}?${DOWNLOAD_TOKEN_PARAM}=${token}`, + ); + // Timeout guards against the browser never making the request. + // Once the request arrives and streaming begins, settleResolve() + // is called from the request handler instead. + // Arm before openExternal so no window exists between open and guard. + timeoutId = setTimeout(() => { + settleReject( + new Error( + l10n.t( + "Timed out waiting for the browser to start the download.", + ), + ), + ); + }, BROWSER_CONNECTION_TIMEOUT_MS); + + const opened = await env.openExternal(externalUri); + + if (!opened) { + throw new Error(l10n.t("Failed to open browser download URL.")); + } + } catch (error) { + server.close(); + settleReject(error); + } + }); + }); +} diff --git a/client/src/components/LibraryNavigator/index.ts b/client/src/components/LibraryNavigator/index.ts index a38776ca8..013255b4e 100644 --- a/client/src/components/LibraryNavigator/index.ts +++ b/client/src/components/LibraryNavigator/index.ts @@ -4,6 +4,7 @@ import { ConfigurationChangeEvent, Disposable, ExtensionContext, + UIKind, Uri, commands, env, @@ -12,9 +13,7 @@ import { workspace, } from "vscode"; -import { randomBytes, timingSafeEqual } from "crypto"; import { createWriteStream } from "fs"; -import { createServer } from "http"; import { profileConfig } from "../../commands/profile"; import { Column } from "../../connection/rest/api/compute"; @@ -27,17 +26,11 @@ import LibraryAdapterFactory from "./LibraryAdapterFactory"; import LibraryDataProvider from "./LibraryDataProvider"; import LibraryModel from "./LibraryModel"; import PaginatedResultSet from "./PaginatedResultSet"; +import { streamTableToBrowserDownload } from "./browserDownload"; import { Messages } from "./const"; import { LibraryAdapter, LibraryItem, TableData } from "./types"; class LibraryNavigator implements SubscriptionProvider { - private static readonly DOWNLOAD_TOKEN_BYTES = 24; // 192-bit token - private static readonly MAX_DOWNLOAD_FILENAME_LENGTH = 250; - private static readonly BROWSER_CONNECTION_TIMEOUT_MS = 60_000; - private static readonly DOWNLOAD_ENDPOINT_PATH = "/sas-library-download"; - private static readonly DOWNLOAD_TOKEN_PARAM = "token"; - private static readonly DEFAULT_DOWNLOAD_FILENAME = "table.csv"; - private libraryDataProvider: LibraryDataProvider; private extensionUri: Uri; private webviewManager: WebViewManager; @@ -107,15 +100,36 @@ class LibraryNavigator implements SubscriptionProvider { async (item: LibraryItem) => { const defaultFileName = `${item.library}.${item.name}.csv`.toLocaleLowerCase(); + + // In web-enabled vscode distros, the native save dialog cannot write to the user's local + // file system, so skip it and stream directly to the browser. + // In this mode, the file will be downloaded to the browser's default download location. + if (env.uiKind === UIKind.Web) { + try { + await streamTableToBrowserDownload( + item, + defaultFileName, + this.libraryDataProvider, + ); + } catch (error) { + window.showErrorMessage( + l10n.t("Failed to download table: {error}", { + error: String( + error?.message || error || "Unknown error", + ).slice(0, 200), + }), + ); + } + return; + } + + // Desktop mode: let the user pick a save location. const defaultUri = workspace.workspaceFolders && workspace.workspaceFolders.length > 0 ? Uri.joinPath(workspace.workspaceFolders[0].uri, defaultFileName) : Uri.file(defaultFileName); - const uri = await window.showSaveDialog({ - defaultUri, - }); - + const uri = await window.showSaveDialog({ defaultUri }); if (!uri) { return; } @@ -137,25 +151,6 @@ class LibraryNavigator implements SubscriptionProvider { } return; } - - try { - // In web/virtual file systems (e.g. vscode-local:// from code-server's "Show Local"), - // stream directly to the browser download pipeline. - const selectedName = uri.path.split("/").pop() || defaultFileName; - window.showInformationMessage( - l10n.t("Opening browser to download table..."), - ); - await this.streamTableToBrowserDownload(item, selectedName); - } catch (error) { - window.showErrorMessage( - l10n.t("Failed to download table: {error}", { - error: String(error?.message || error || "Unknown error").slice( - 0, - 200, - ), - }), - ); - } }, ), commands.registerCommand( @@ -220,183 +215,6 @@ class LibraryNavigator implements SubscriptionProvider { return new LibraryAdapterFactory().create(activeProfile.connectionType); } - - private async streamTableToBrowserDownload( - item: LibraryItem, - fileName: string, - ): Promise { - const token = randomBytes(LibraryNavigator.DOWNLOAD_TOKEN_BYTES).toString( - "hex", - ); - // Preserve Unicode while removing only dangerous characters - const asciiFileName = - fileName - .trim() - // eslint-disable-next-line no-control-regex - .replace(/[\x00-\x1f"\r\n]/g, "") // Remove control chars + quotes - .replace(/\.\.+/g, ".") // Collapse multiple dots (path traversal) - .replace(/^\.+/, "") // Remove leading dots - .slice(0, LibraryNavigator.MAX_DOWNLOAD_FILENAME_LENGTH) || - LibraryNavigator.DEFAULT_DOWNLOAD_FILENAME; - const encodedFileName = encodeURIComponent( - fileName || LibraryNavigator.DEFAULT_DOWNLOAD_FILENAME, - ); - - await new Promise((resolve, reject) => { - let timeoutId: NodeJS.Timeout | undefined; - let tokenConsumed = false; - let requestLock = false; - let settled = false; - - const settleResolve = () => { - if (settled) { - return; - } - settled = true; - server.removeListener("error", errorHandler); - resolve(); - }; - - const settleReject = (error: Error) => { - if (settled) { - return; - } - settled = true; - if (timeoutId) { - clearTimeout(timeoutId); - timeoutId = undefined; - } - server.removeListener("error", errorHandler); - server.close(() => {}); - reject(error); - }; - - const isValidToken = (candidate: string | null): boolean => { - if (!candidate || candidate.length !== token.length) { - return false; - } - return timingSafeEqual(Buffer.from(candidate), Buffer.from(token)); - }; - - const server = createServer((request, response) => { - if (request.method !== "GET") { - response.statusCode = 405; - response.setHeader("Allow", "GET"); - response.end(); - return; - } - - if (tokenConsumed || requestLock) { - response.statusCode = 410; - response.end(); - return; - } - - const requestUrl = request.url - ? new URL(request.url, "http://127.0.0.1") - : undefined; - - if ( - !requestUrl || - requestUrl.pathname !== LibraryNavigator.DOWNLOAD_ENDPOINT_PATH || - !isValidToken( - requestUrl.searchParams.get(LibraryNavigator.DOWNLOAD_TOKEN_PARAM), - ) - ) { - response.statusCode = 404; - response.end(); - return; - } - - requestLock = true; - tokenConsumed = true; - if (timeoutId) { - clearTimeout(timeoutId); - timeoutId = undefined; - } - server.close(); - - response.setHeader("Content-Type", "text/csv; charset=utf-8"); - response.setHeader("Cache-Control", "no-store"); - response.setHeader("Pragma", "no-cache"); - response.setHeader("X-Content-Type-Options", "nosniff"); - response.setHeader( - "Content-Disposition", - `attachment; filename="${asciiFileName}"; filename*=UTF-8''${encodedFileName}`, - ); - - this.libraryDataProvider - .writeTableContentsToStream(response, item) - .then(() => { - if (!response.writableEnded) { - response.end(); - } - settleResolve(); - }) - .catch((error) => { - if (!response.headersSent) { - response.statusCode = 500; - response.setHeader("Content-Type", "text/plain"); - response.end("Download failed"); - } else { - // Headers already sent - destroy connection to signal error to browser - response.destroy(); - } - settleReject(error); - }); - }); - - const errorHandler = (error: Error) => { - if (timeoutId) { - clearTimeout(timeoutId); - timeoutId = undefined; - } - settleReject(error); - }; - - server.on("error", errorHandler); - - server.listen(0, "127.0.0.1", async () => { - try { - const address = server.address(); - if (!address || typeof address === "string") { - throw new Error(l10n.t("Unable to start download server.")); - } - - // asExternalUri only transforms scheme+host+port — the proxy strips - // path and query. Resolve just the base, then append path+token. - const baseLocalUri = Uri.parse(`http://127.0.0.1:${address.port}`); - const externalBase = await env.asExternalUri(baseLocalUri); - const externalUri = Uri.parse( - `${externalBase.toString(true).replace(/\/+$/, "")}${LibraryNavigator.DOWNLOAD_ENDPOINT_PATH}?${LibraryNavigator.DOWNLOAD_TOKEN_PARAM}=${token}`, - ); - const opened = await env.openExternal(externalUri); - - if (!opened) { - throw new Error(l10n.t("Failed to open browser download URL.")); - } - - // Timeout guards against the browser never making the request. - // Once the request arrives and streaming begins, settleResolve() - // is called from the request handler instead. - // Increased to 60s for slow networks and corporate proxies - timeoutId = setTimeout(() => { - server.close(); - settleReject( - new Error( - l10n.t( - "Timed out waiting for the browser to start the download.", - ), - ), - ); - }, LibraryNavigator.BROWSER_CONNECTION_TIMEOUT_MS); - } catch (error) { - server.close(); - settleReject(error); - } - }); - }); - } } export default LibraryNavigator; diff --git a/client/test/components/LibraryNavigator/LibraryModel.backpressure.test.ts b/client/test/components/LibraryNavigator/LibraryModel.backpressure.test.ts index eae9246b7..a2702fcf0 100644 --- a/client/test/components/LibraryNavigator/LibraryModel.backpressure.test.ts +++ b/client/test/components/LibraryNavigator/LibraryModel.backpressure.test.ts @@ -4,535 +4,68 @@ import { expect } from "chai"; import * as sinon from "sinon"; import { Writable } from "stream"; -// Mock writable stream that can simulate backpressure -class MockWritableStream extends Writable { - public chunks: string[] = []; - public ended = false; - public destroyed = false; - private shouldApplyBackpressure = false; - private writeCount = 0; - private backpressureThreshold = 5; +import { + stringArrayToCsvString, + writeWithBackpressure, +} from "../../../src/components/LibraryNavigator/LibraryModel"; - setBackpressureThreshold(threshold: number): void { - this.backpressureThreshold = threshold; - } - - enableBackpressure(): void { - this.shouldApplyBackpressure = true; - } - - disableBackpressure(): void { - this.shouldApplyBackpressure = false; - } - - triggerDrain(): void { - this.emit("drain"); - } - - _write( - chunk: Buffer | string, - encoding: string, - callback: (error?: Error) => void, - ): void { - this.chunks.push(chunk.toString()); - this.writeCount++; - - // Simulate backpressure after threshold - const hasBackpressure = - this.shouldApplyBackpressure && - this.writeCount > this.backpressureThreshold; - - callback(); - - if (hasBackpressure) { - // Don't signal ready immediately - setTimeout(() => this.triggerDrain(), 10); - } - } - - write( - chunk: Buffer | string, - encodingOrCallback?: - | BufferEncoding - | ((error: Error | null | undefined) => void), - callback?: (error: Error | null | undefined) => void, - ): boolean { - // Handle overload where second parameter is callback - let result: boolean; - - if (typeof encodingOrCallback === "function") { - result = super.write(chunk, encodingOrCallback); - } else if (encodingOrCallback && callback) { - result = super.write(chunk, encodingOrCallback, callback); - } else if (encodingOrCallback) { - result = super.write(chunk, encodingOrCallback); - } else if (callback) { - result = super.write(chunk, callback); - } else { - result = super.write(chunk); - } - - // Return false to signal backpressure - if ( - this.shouldApplyBackpressure && - this.writeCount > this.backpressureThreshold - ) { - return false; - } - - return result; - } - - end( - chunkOrCallback?: Buffer | string | (() => void), - encodingOrCallback?: BufferEncoding | (() => void), - callback?: () => void, - ): this { - // Handle overload variations - let actualChunk: Buffer | string | undefined; - let actualCallback: (() => void) | undefined; - - if (typeof chunkOrCallback === "function") { - actualCallback = chunkOrCallback; - } else { - actualChunk = chunkOrCallback; - if (typeof encodingOrCallback === "function") { - actualCallback = encodingOrCallback; - } else { - actualCallback = callback; - } - } - - if (actualChunk) { - this.chunks.push(actualChunk.toString()); - } - this.ended = true; - if (actualCallback) { - actualCallback(); - } - return this; - } - - destroy(): this { - this.destroyed = true; - return this; - } - - getWrittenData(): string { - return this.chunks.join(""); - } - - getChunkCount(): number { - return this.chunks.length; - } - - reset(): void { - this.chunks = []; - this.ended = false; - this.destroyed = false; - this.writeCount = 0; - } -} - -describe("LibraryModel - Backpressure Handling", function () { - let sandbox: sinon.SinonSandbox; - - beforeEach(() => { - sandbox = sinon.createSandbox(); - }); - - afterEach(() => { - sandbox.restore(); - }); - - describe("Stream Write with Backpressure", () => { - it("should wait for drain event when write returns false", async () => { - const stream = new MockWritableStream(); - stream.enableBackpressure(); - stream.setBackpressureThreshold(2); - - let drainWaited = false; - - // Simulate writing that triggers backpressure - for (let i = 0; i < 5; i++) { - const canContinue = stream.write(`chunk${i}\n`); - if (!canContinue) { - drainWaited = true; - await new Promise((resolve) => { - stream.once("drain", resolve); - }); - } - } - - expect(drainWaited).to.be.true; - expect(stream.getChunkCount()).to.equal(5); - }); - - it("should continue writing after drain event", async () => { - const stream = new MockWritableStream(); - let writesAfterDrain = 0; - - // First write succeeds - stream.write("chunk1\n"); - - // Second write triggers backpressure - stream.enableBackpressure(); - stream.setBackpressureThreshold(1); - - const canContinue = stream.write("chunk2\n"); - expect(canContinue).to.be.false; - - // Wait for drain - await new Promise((resolve) => { - stream.once("drain", () => { - stream.disableBackpressure(); - resolve(); - }); - stream.triggerDrain(); - }); - - // Continue writing after drain - stream.write("chunk3\n"); - writesAfterDrain++; - - expect(writesAfterDrain).to.equal(1); - expect(stream.getChunkCount()).to.equal(3); +describe("LibraryModel", function () { + describe("CSV Formatting", () => { + it("should escape double quotes in cell values", () => { + expect(stringArrayToCsvString(['value with "quotes"'])).to.equal( + '"value with ""quotes"""', + ); }); - it("should not wait when write returns true", async () => { - const stream = new MockWritableStream(); - stream.disableBackpressure(); - - let drainWaited = false; - - for (let i = 0; i < 10; i++) { - const canContinue = stream.write(`chunk${i}\n`); - if (!canContinue) { - drainWaited = true; - await new Promise((resolve) => { - stream.once("drain", resolve); - }); - } - } - - expect(drainWaited).to.be.false; - expect(stream.getChunkCount()).to.equal(10); + it("should coerce null cells to empty string", () => { + expect(stringArrayToCsvString([null])).to.equal('""'); }); - }); - - describe("Header Write with Backpressure", () => { - it("should handle backpressure on header write", async () => { - const stream = new MockWritableStream(); - stream.enableBackpressure(); - stream.setBackpressureThreshold(0); // Trigger immediately - let hasWrittenHeader = false; - const headers = ["col1", "col2", "col3"]; - const headerString = `"${headers.join('","')}"`; - - const canContinue = stream.write(headerString); - if (!canContinue) { - await new Promise((resolve) => { - stream.once("drain", resolve); - }); - stream.triggerDrain(); - } - hasWrittenHeader = true; - - expect(hasWrittenHeader).to.be.true; - expect(stream.getChunkCount()).to.equal(1); + it("should coerce numeric cells to string", () => { + expect(stringArrayToCsvString([12345])).to.equal('"12345"'); }); - it("should write header before rows", async () => { - const stream = new MockWritableStream(); - let hasWrittenHeader = false; - - // Write header - const canContinue = stream.write("col1,col2,col3\n"); - if (!canContinue) { - await new Promise((resolve) => { - stream.once("drain", resolve); - }); - } - hasWrittenHeader = true; - - // Write rows - for (let i = 0; i < 3; i++) { - stream.write(`row${i}\n`); - } - - const data = stream.getWrittenData(); - expect(hasWrittenHeader).to.be.true; - expect(data).to.match(/^col1,col2,col3\n/); - expect(stream.getChunkCount()).to.equal(4); + it("should format multiple cells as a quoted CSV row", () => { + expect(stringArrayToCsvString(["col1", "col2", "col3"])).to.equal( + '"col1","col2","col3"', + ); }); }); - describe("Row Writing with Backpressure", () => { - it("should handle backpressure for each row", async () => { - const stream = new MockWritableStream(); - stream.enableBackpressure(); - stream.setBackpressureThreshold(3); - - const rows = [ - { cells: ["a", "b", "c"] }, - { cells: ["d", "e", "f"] }, - { cells: ["g", "h", "i"] }, - { cells: ["j", "k", "l"] }, - { cells: ["m", "n", "o"] }, - ]; - - let backpressureHits = 0; - - for (const row of rows) { - const rowString = "\n" + row.cells.join(","); - const canContinue = stream.write(rowString); - if (!canContinue) { - backpressureHits++; - await new Promise((resolve) => { - stream.once("drain", resolve); - }); - stream.triggerDrain(); - } - } - - expect(backpressureHits).to.be.greaterThan(0); - expect(stream.getChunkCount()).to.equal(5); - }); - - it("should preserve data integrity during backpressure", async () => { - const stream = new MockWritableStream(); - stream.enableBackpressure(); - stream.setBackpressureThreshold(2); - - const expectedData = ["row1", "row2", "row3", "row4", "row5"]; - - for (const data of expectedData) { - const canContinue = stream.write(data + "\n"); - if (!canContinue) { - await new Promise((resolve) => { - stream.once("drain", resolve); - }); - stream.triggerDrain(); - } - } - - const written = stream.getWrittenData(); - expectedData.forEach((data) => { - expect(written).to.include(data); + describe("writeWithBackpressure", () => { + it("should write data and resolve immediately when write returns true", async () => { + const written: string[] = []; + const stream = new Writable({ + write(chunk, _enc, cb) { + written.push(chunk.toString()); + cb(); + }, }); - }); - }); - - describe("Memory Management", () => { - it("should not accumulate data in memory during backpressure", async () => { - const stream = new MockWritableStream(); - stream.enableBackpressure(); - stream.setBackpressureThreshold(5); - const initialMemory = process.memoryUsage().heapUsed; - const largeChunkCount = 100; - - for (let i = 0; i < largeChunkCount; i++) { - const canContinue = stream.write("x".repeat(100)); - if (!canContinue) { - await new Promise((resolve) => { - stream.once("drain", resolve); - }); - stream.triggerDrain(); - } - } - - const finalMemory = process.memoryUsage().heapUsed; - const memoryIncrease = finalMemory - initialMemory; - - // Memory increase should be reasonable (less than 10MB for test data) - expect(memoryIncrease).to.be.lessThan(10 * 1024 * 1024); - }); - - it("should handle large datasets with multiple backpressure cycles", async () => { - const stream = new MockWritableStream(); - stream.enableBackpressure(); - stream.setBackpressureThreshold(10); - - let backpressureCycles = 0; - const rowCount = 100; - - for (let i = 0; i < rowCount; i++) { - const canContinue = stream.write(`row${i},data${i},value${i}\n`); - if (!canContinue) { - backpressureCycles++; - await new Promise((resolve) => { - stream.once("drain", resolve); - }); - stream.triggerDrain(); - } - } - - expect(backpressureCycles).to.be.greaterThan(0); - expect(stream.getChunkCount()).to.equal(rowCount); - }); - }); - - describe("Cancellation During Backpressure", () => { - it("should respect cancellation token during write", async () => { - const stream = new MockWritableStream(); - let cancelled = false; - - // Simulate cancellation - const cancellationCallback = () => { - stream.destroy(); - cancelled = true; - }; - - // Write some data - stream.write("chunk1\n"); - - // Simulate cancellation - cancellationCallback(); - - expect(stream.destroyed).to.be.true; - expect(cancelled).to.be.true; - }); - - it("should stop writing on cancellation", async () => { - const stream = new MockWritableStream(); - let cancelled = false; - - for (let i = 0; i < 10; i++) { - if (cancelled) { - break; - } - - stream.write(`chunk${i}\n`); - - // Simulate cancellation after 5 writes - if (i === 4) { - stream.destroy(); - cancelled = true; - } - } - - expect(stream.destroyed).to.be.true; - expect(stream.getChunkCount()).to.equal(5); - }); - }); - - describe("Error Handling During Backpressure", () => { - it("should handle write errors gracefully", async () => { - const stream = new MockWritableStream(); - let errorCaught = false; - - try { - stream.write("valid data\n"); - - // Simulate error - stream.destroy(); - - if (stream.destroyed) { - throw new Error("Stream destroyed"); - } - } catch { - errorCaught = true; - } - - expect(errorCaught).to.be.true; - }); - - it("should cleanup on error during drain wait", async () => { - const stream = new MockWritableStream(); - stream.enableBackpressure(); - stream.setBackpressureThreshold(0); - - let cleanedUp = false; - - try { - const canContinue = stream.write("data\n"); - if (!canContinue) { - const drainPromise = new Promise((resolve, reject) => { - stream.once("drain", resolve); - stream.once("error", reject); - }); - - // Simulate error before drain - setTimeout(() => { - stream.emit("error", new Error("Test error")); - }, 5); - - await drainPromise; - } - } catch { - stream.destroy(); - cleanedUp = true; - } - - expect(cleanedUp).to.be.true; - expect(stream.destroyed).to.be.true; + await writeWithBackpressure(stream, "hello"); + expect(written).to.deep.equal(["hello"]); }); - }); - - describe("CSV Formatting with Backpressure", () => { - it("should properly escape quotes in cells", () => { - const input = 'value with "quotes"'; - const escaped = input.replace(/"/g, '""'); - const formatted = `"${escaped}"`; - expect(formatted).to.equal('"value with ""quotes"""'); - }); - - it("should handle null values", () => { - const value: unknown = null; - const formatted = (value ?? "").toString().replace(/"/g, '""'); - - expect(formatted).to.equal(""); - }); - - it("should convert numbers to strings", () => { - const value: unknown = 12345; - const formatted = (value ?? "").toString().replace(/"/g, '""'); - - expect(formatted).to.equal("12345"); - }); - - it("should format array as CSV row", () => { - const cells = ["col1", "col2", "col3"]; - const formatted = `"${cells - .map((item) => (item ?? "").toString().replace(/"/g, '""')) - .join('","')}"`; - - expect(formatted).to.equal('"col1","col2","col3"'); - }); - }); - - describe("Stream Completion", () => { - it("should end stream after all writes complete", async () => { - const stream = new MockWritableStream(); - - for (let i = 0; i < 5; i++) { - stream.write(`row${i}\n`); - } - - stream.end(); - - expect(stream.ended).to.be.true; - expect(stream.getChunkCount()).to.equal(5); - }); - - it("should not write after stream ends", () => { - const stream = new MockWritableStream(); - - stream.write("data1\n"); - stream.end(); + it("should wait for drain before resolving when write returns false", async () => { + const stream = new Writable({ + write(_chunk, _enc, cb) { + cb(); + }, + }); + const stub = sinon.stub(stream, "write").callsFake(() => { + setTimeout(() => stream.emit("drain"), 0); + return false; + }); - let writeAfterEnd = false; - try { - stream.write("data2\n"); - } catch { - writeAfterEnd = true; - } + let resolved = false; + const p = writeWithBackpressure(stream, "data").then(() => { + resolved = true; + }); - expect(stream.ended).to.be.true; - expect(writeAfterEnd).to.be.false; // Node.js Writable doesn't throw, just ignores writes after end + expect(resolved).to.be.false; + await p; + expect(resolved).to.be.true; + stub.restore(); }); }); }); diff --git a/client/test/components/LibraryNavigator/LibraryNavigator.test.ts b/client/test/components/LibraryNavigator/LibraryNavigator.test.ts index a4345081e..2db3cac8b 100644 --- a/client/test/components/LibraryNavigator/LibraryNavigator.test.ts +++ b/client/test/components/LibraryNavigator/LibraryNavigator.test.ts @@ -1,533 +1,204 @@ // Copyright © 2026, SAS Institute Inc., Cary, NC, USA. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 import { expect } from "chai"; -import { timingSafeEqual } from "crypto"; -import { EventEmitter } from "events"; -import * as sinon from "sinon"; -import { Writable } from "stream"; +import { randomBytes } from "crypto"; -// Mock HTTP server response -class MockResponse extends Writable { - public statusCode?: number; - public headers: Record = {}; - public headersSent = false; - private writeCallback?: () => void; +import { + classifyDownloadRequest, + isValidDownloadToken, + sanitizeDownloadFilename, +} from "../../../src/components/LibraryNavigator/browserDownload"; - setHeader(name: string, value: string): void { - this.headers[name] = value; - } - - end(callback?: () => void): this { - this.headersSent = true; - super.end(); - if (callback) { - callback(); - } - return this; - } - - destroy(): this { - super.destroy(); - return this; - } - - _write( - chunk: Buffer | string, - encoding: string, - callback: (error?: Error) => void, - ): void { - this.headersSent = true; - if (this.writeCallback) { - this.writeCallback(); - } - callback(); - } - - simulateBackpressure(callback: () => void): void { - this.writeCallback = callback; - } -} - -// Mock HTTP server request -class MockRequest extends EventEmitter { - public method = "GET"; - public url = "/sas-table-download?token=validtoken"; -} - -// Mock HTTP server -class MockServer extends EventEmitter { - private requestHandler?: (req: MockRequest, res: MockResponse) => void; - private listening = false; - public address(): { port: number } | null { - return this.listening ? { port: 12345 } : null; - } - - listen(port: number, host: string, callback: () => void): this { - this.listening = true; - setTimeout(() => callback(), 10); - return this; - } - - close(callback?: () => void): this { - this.listening = false; - if (callback) { - setTimeout(callback, 10); - } - return this; - } - - setRequestHandler( - handler: (req: MockRequest, res: MockResponse) => void, - ): void { - this.requestHandler = handler; - } - - simulateRequest(): { request: MockRequest; response: MockResponse } { - const req = new MockRequest(); - const res = new MockResponse(); - if (this.requestHandler) { - this.requestHandler(req, res); - } - return { request: req, response: res }; - } -} +const MAX_LENGTH = 250; +const FALLBACK = "table.csv"; describe("LibraryNavigator - Browser Download", function () { - let sandbox: sinon.SinonSandbox; - - beforeEach(() => { - sandbox = sinon.createSandbox(); - }); - - afterEach(() => { - sandbox.restore(); - }); - describe("Filename Sanitization", () => { - const controlCharsPattern = new RegExp( - "[" + String.fromCharCode(0) + "-" + String.fromCharCode(31) + '"\r\n]', - "g", - ); - it("should remove control characters from filename", () => { - const input = "test\x00\x01\x1ffile.csv"; - const result = input - .trim() - .replace(controlCharsPattern, "") - .replace(/\.\.+/g, ".") - .replace(/^\.+/, ""); - - expect(result).to.equal("testfile.csv"); + expect( + sanitizeDownloadFilename( + "test\x00\x01\x1ffile.csv", + MAX_LENGTH, + FALLBACK, + ), + ).to.equal("testfile.csv"); }); it("should remove quotes from filename", () => { - const input = 'test"file".csv'; - const result = input - .trim() - .replace(controlCharsPattern, "") - .replace(/\.\.+/g, ".") - .replace(/^\.+/, ""); - - expect(result).to.equal("testfile.csv"); + expect( + sanitizeDownloadFilename('test"file".csv', MAX_LENGTH, FALLBACK), + ).to.equal("testfile.csv"); }); it("should collapse multiple dots to prevent path traversal", () => { - const input = "test..file...csv"; - const result = input - .trim() - .replace(controlCharsPattern, "") - .replace(/\.\.+/g, ".") - .replace(/^\.+/, ""); - - expect(result).to.equal("test.file.csv"); + expect( + sanitizeDownloadFilename("test..file...csv", MAX_LENGTH, FALLBACK), + ).to.equal("test.file.csv"); }); it("should remove leading dots", () => { - const input = "...test.csv"; - const result = input - .trim() - .replace(controlCharsPattern, "") - .replace(/\.\.+/g, ".") - .replace(/^\.+/, ""); - - expect(result).to.equal("test.csv"); + expect( + sanitizeDownloadFilename("...test.csv", MAX_LENGTH, FALLBACK), + ).to.equal("test.csv"); }); it("should preserve Unicode characters", () => { - const input = "テスト日本語.csv"; - const result = input - .trim() - .replace(controlCharsPattern, "") - .replace(/\.\.+/g, ".") - .replace(/^\.+/, ""); - - expect(result).to.equal("テスト日本語.csv"); + expect( + sanitizeDownloadFilename("テスト日本語.csv", MAX_LENGTH, FALLBACK), + ).to.equal("テスト日本語.csv"); }); - it("should limit filename length to 250 characters", () => { - const input = "a".repeat(300) + ".csv"; - const result = input - .trim() - .replace(controlCharsPattern, "") - .replace(/\.\.+/g, ".") - .replace(/^\.+/, "") - .slice(0, 250); - - expect(result.length).to.equal(250); + it("should limit filename to max length", () => { + const result = sanitizeDownloadFilename( + "a".repeat(300) + ".csv", + MAX_LENGTH, + FALLBACK, + ); + expect(result.length).to.equal(MAX_LENGTH); }); - it("should use fallback name when sanitization results in empty string", () => { - const input = "\x00\x01\x1f"; - const result = - input - .trim() - .replace(controlCharsPattern, "") - .replace(/\.\.+/g, ".") - .replace(/^\.+/, "") - .slice(0, 250) || "table.csv"; - - expect(result).to.equal("table.csv"); + it("should return fallback when sanitization results in empty string", () => { + expect( + sanitizeDownloadFilename("\x00\x01\x1f", MAX_LENGTH, FALLBACK), + ).to.equal(FALLBACK); }); }); describe("Token Validation", () => { - it("should use timing-safe comparison for token validation", () => { - const token1 = Buffer.from("abc123"); - const token2 = Buffer.from("abc123"); - const token3 = Buffer.from("xyz789"); - - expect(timingSafeEqual(token1, token2)).to.be.true; - expect(timingSafeEqual(token1, token3)).to.be.false; + it("should accept a valid token", () => { + const token = randomBytes(24).toString("hex"); + expect(isValidDownloadToken(token, token)).to.be.true; }); - it("should reject tokens of different lengths", () => { - const validToken = "a".repeat(48); // 24 bytes as hex - const candidate = "a".repeat(40); - - const isValid = candidate && candidate.length === validToken.length; - expect(isValid).to.be.false; + it("should reject a token of wrong length", () => { + const token = randomBytes(24).toString("hex"); + expect(isValidDownloadToken(token.slice(0, -1), token)).to.be.false; }); - it("should accept valid token", () => { - const validToken = "a".repeat(48); - const candidate = "a".repeat(48); - - const isValid = candidate && candidate.length === validToken.length; - expect(isValid).to.be.true; - }); - }); - - describe("Error Message Sanitization", () => { - it("should limit error messages to 200 characters", () => { - const longError = new Error("a".repeat(500)); - const sanitized = String(longError?.message || "Unknown error").slice( - 0, - 200, - ); - - expect(sanitized.length).to.equal(200); + it("should reject a null token", () => { + const token = randomBytes(24).toString("hex"); + expect(isValidDownloadToken(null, token)).to.be.false; }); - it("should handle undefined errors", () => { - const error: unknown = undefined; - const errorObj = - error && typeof error === "object" && "message" in error ? error : null; - const sanitized = String( - errorObj?.message || error || "Unknown error", - ).slice(0, 200); - - expect(sanitized).to.equal("Unknown error"); - }); - - it("should handle null errors", () => { - const error: unknown = null; - const errorObj = - error && typeof error === "object" && "message" in error ? error : null; - const sanitized = String( - errorObj?.message || error || "Unknown error", - ).slice(0, 200); - - expect(sanitized).to.equal("Unknown error"); - }); - - it("should handle errors without message property", () => { - const error: unknown = { code: "ERR_UNKNOWN" }; - const errorObj = - error && typeof error === "object" && "message" in error ? error : null; - const sanitized = String( - errorObj?.message || error || "Unknown error", - ).slice(0, 200); - - expect(sanitized).to.equal("[object Object]"); + it("should reject a token with correct length but wrong value", () => { + const token = randomBytes(24).toString("hex"); + const wrong = "x".repeat(token.length); + expect(isValidDownloadToken(wrong, token)).to.be.false; }); }); - describe("HTTP Response Headers", () => { - it("should set correct Content-Type header", () => { - const response = new MockResponse(); - response.setHeader("Content-Type", "text/csv; charset=utf-8"); - - expect(response.headers["Content-Type"]).to.equal( - "text/csv; charset=utf-8", + describe("Download Request Classification", () => { + const TOKEN = randomBytes(24).toString("hex"); + const ENDPOINT = "/sas-library-download"; + const TOKEN_PARAM = "token"; + + it("should reject non-GET methods with 405", () => { + const result = classifyDownloadRequest( + "POST", + `${ENDPOINT}?${TOKEN_PARAM}=${TOKEN}`, + TOKEN, + ENDPOINT, + TOKEN_PARAM, + false, ); + expect(result.outcome).to.equal(405); }); - it("should set security headers", () => { - const response = new MockResponse(); - response.setHeader("Cache-Control", "no-store"); - response.setHeader("Pragma", "no-cache"); - response.setHeader("X-Content-Type-Options", "nosniff"); - - expect(response.headers["Cache-Control"]).to.equal("no-store"); - expect(response.headers.Pragma).to.equal("no-cache"); - expect(response.headers["X-Content-Type-Options"]).to.equal("nosniff"); - }); - - it("should set Content-Disposition with ASCII and UTF-8 filenames", () => { - const asciiName = "test.csv"; - const utf8Name = encodeURIComponent("テスト.csv"); - const header = `attachment; filename="${asciiName}"; filename*=UTF-8''${utf8Name}`; - - const response = new MockResponse(); - response.setHeader("Content-Disposition", header); - - expect(response.headers["Content-Disposition"]).to.include("attachment"); - expect(response.headers["Content-Disposition"]).to.include(asciiName); - expect(response.headers["Content-Disposition"]).to.include("UTF-8"); + it("should reject a locked server with 410", () => { + const result = classifyDownloadRequest( + "GET", + `${ENDPOINT}?${TOKEN_PARAM}=${TOKEN}`, + TOKEN, + ENDPOINT, + TOKEN_PARAM, + true, + ); + expect(result.outcome).to.equal(410); }); - }); - describe("HTTP Status Codes", () => { - it("should return 405 for non-GET requests", () => { - const response = new MockResponse(); - const method: string = "POST"; - - if (method !== "GET") { - response.statusCode = 405; - response.setHeader("Allow", "GET"); - } - - expect(response.statusCode).to.equal(405); - expect(response.headers.Allow).to.equal("GET"); + it("should reject an invalid token with 404", () => { + const result = classifyDownloadRequest( + "GET", + `${ENDPOINT}?${TOKEN_PARAM}=badtoken`, + TOKEN, + ENDPOINT, + TOKEN_PARAM, + false, + ); + expect(result.outcome).to.equal(404); }); - it("should return 410 for consumed tokens", () => { - const response = new MockResponse(); - const tokenConsumed = true; - - if (tokenConsumed) { - response.statusCode = 410; - } - - expect(response.statusCode).to.equal(410); + it("should reject a wrong path with 404", () => { + const result = classifyDownloadRequest( + "GET", + `/other?${TOKEN_PARAM}=${TOKEN}`, + TOKEN, + ENDPOINT, + TOKEN_PARAM, + false, + ); + expect(result.outcome).to.equal(404); }); - it("should return 404 for invalid tokens", () => { - const response = new MockResponse(); - const validToken = false; - - if (!validToken) { - response.statusCode = 404; - } - - expect(response.statusCode).to.equal(404); + it("should reject a missing URL with 404", () => { + const result = classifyDownloadRequest( + "GET", + undefined, + TOKEN, + ENDPOINT, + TOKEN_PARAM, + false, + ); + expect(result.outcome).to.equal(404); }); - it("should return 500 on server errors before headers sent", () => { - const response = new MockResponse(); - - if (!response.headersSent) { - response.statusCode = 500; - response.setHeader("Content-Type", "text/plain"); - } - - expect(response.statusCode).to.equal(500); - expect(response.headers["Content-Type"]).to.equal("text/plain"); + it("should allow a valid GET request with matching token and path", () => { + const result = classifyDownloadRequest( + "GET", + `${ENDPOINT}?${TOKEN_PARAM}=${TOKEN}`, + TOKEN, + ENDPOINT, + TOKEN_PARAM, + false, + ); + expect(result.outcome).to.equal("stream"); }); }); - describe("Request Lock Race Condition Protection", () => { - it("should prevent concurrent access with request lock", () => { - let tokenConsumed = false; + describe("Request Lock Protection", () => { + it("should block a second request once the first acquires the lock", () => { let requestLock = false; - // First request - const canProcess1 = !(tokenConsumed || requestLock); - if (canProcess1) { + const canProcess = !requestLock; + if (canProcess) { requestLock = true; - tokenConsumed = true; } - expect(canProcess1).to.be.true; - - // Second concurrent request - const canProcess2 = !(tokenConsumed || requestLock); - - expect(canProcess2).to.be.false; - }); - - it("should set both flags atomically", () => { - let tokenConsumed = false; - let requestLock = false; - - // Simulate checking and setting - if (!(tokenConsumed || requestLock)) { - requestLock = true; // Set lock first - tokenConsumed = true; // Then set consumed - } - - expect(requestLock).to.be.true; - expect(tokenConsumed).to.be.true; - }); - }); - - describe("Response Stream Error Handling", () => { - it("should set error response before headers sent", () => { - const response = new MockResponse(); - - if (!response.headersSent) { - response.statusCode = 500; - response.setHeader("Content-Type", "text/plain"); - response.end(); - } - - expect(response.statusCode).to.equal(500); - expect(response.writableEnded).to.be.true; - }); - - it("should destroy response if headers already sent", () => { - const response = new MockResponse(); - response.headersSent = true; - - if (response.headersSent) { - response.destroy(); - } - - expect(response.destroyed).to.be.true; - }); - - it("should end response only if not already ended", () => { - const response = new MockResponse(); - - if (!response.writableEnded) { - response.end(); - } - - expect(response.writableEnded).to.be.true; - - // Should not throw on second attempt - if (!response.writableEnded) { - response.end(); - } - }); - }); - - describe("Timeout Handling", () => { - it("should clear timeouts on successful completion", () => { - let timeoutId: NodeJS.Timeout | undefined = setTimeout(() => {}, 1000); - let streamTimeoutId: NodeJS.Timeout | undefined = setTimeout( - () => {}, - 1000, - ); - - if (timeoutId) { - clearTimeout(timeoutId); - timeoutId = undefined; - } - if (streamTimeoutId) { - clearTimeout(streamTimeoutId); - streamTimeoutId = undefined; - } - - expect(timeoutId).to.be.undefined; - expect(streamTimeoutId).to.be.undefined; - }); - - it("should clear timeouts on error", () => { - let timeoutId: NodeJS.Timeout | undefined = setTimeout(() => {}, 1000); - let streamTimeoutId: NodeJS.Timeout | undefined = setTimeout( - () => {}, - 1000, - ); - - // Simulate error cleanup - if (timeoutId) { - clearTimeout(timeoutId); - timeoutId = undefined; - } - if (streamTimeoutId) { - clearTimeout(streamTimeoutId); - streamTimeoutId = undefined; - } - - expect(timeoutId).to.be.undefined; - expect(streamTimeoutId).to.be.undefined; - }); - }); - - describe("Server Cleanup", () => { - it("should close server in error paths", (done) => { - const server = new MockServer(); - - server.listen(0, "127.0.0.1", () => { - expect(server.address()).to.not.be.null; - - server.close(() => { - expect(server.address()).to.be.null; - done(); - }); - }); - }); - - it("should remove error listener on completion", () => { - const server = new MockServer(); - const errorHandler = () => {}; - - server.on("error", errorHandler); - expect(server.listenerCount("error")).to.equal(1); - - server.removeListener("error", errorHandler); - expect(server.listenerCount("error")).to.equal(0); + expect(canProcess).to.be.true; + expect(!requestLock).to.be.false; }); }); describe("Settle Pattern Idempotency", () => { - it("should prevent double resolution", () => { + it("settleResolve should clear timeout to prevent spurious timeout logs", () => { + let timeoutId: NodeJS.Timeout | undefined = setTimeout(() => {}, 1000); let settled = false; - let resolveCount = 0; const settleResolve = () => { if (settled) { return; } settled = true; - resolveCount++; - }; - - settleResolve(); - settleResolve(); - settleResolve(); - - expect(resolveCount).to.equal(1); - }); - - it("should prevent double rejection", () => { - let settled = false; - let rejectCount = 0; - - const settleReject = () => { - if (settled) { - return; + if (timeoutId) { + clearTimeout(timeoutId); + timeoutId = undefined; } - settled = true; - rejectCount++; }; - settleReject(); - settleReject(); - settleReject(); + settleResolve(); - expect(rejectCount).to.equal(1); + expect(timeoutId).to.be.undefined; + expect(settled).to.be.true; }); }); });