diff --git a/src/Cache.ts b/src/Cache.ts index 011bd25d..d8de8f4d 100644 --- a/src/Cache.ts +++ b/src/Cache.ts @@ -5,7 +5,6 @@ import fs from 'graceful-fs'; import crypto from 'node:crypto'; import path from 'node:path'; import { pipeline } from 'node:stream/promises'; -import url from 'node:url'; const d = debug('@electron/get:cache'); @@ -17,12 +16,12 @@ export class Cache { constructor(private cacheRoot = defaultCacheRoot) {} public static getCacheDirectory(downloadUrl: string): string { - const parsedDownloadUrl = url.parse(downloadUrl); - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const { search, hash, pathname, ...rest } = parsedDownloadUrl; - const strippedUrl = url.format({ ...rest, pathname: path.dirname(pathname || 'electron') }); + const parsed = new URL(downloadUrl); + parsed.hash = ''; + parsed.search = ''; + parsed.pathname = path.posix.dirname(parsed.pathname); - return crypto.createHash('sha256').update(strippedUrl).digest('hex'); + return crypto.createHash('sha256').update(parsed.toString()).digest('hex'); } public getCachePath(downloadUrl: string, fileName: string): string { diff --git a/src/GotDownloader.ts b/src/GotDownloader.ts index e5438fe5..544733be 100644 --- a/src/GotDownloader.ts +++ b/src/GotDownloader.ts @@ -49,7 +49,7 @@ export class GotDownloader implements Downloader { await fs.promises.mkdir(path.dirname(targetFilePath), { recursive: true }); const writeStream = fs.createWriteStream(targetFilePath); - if (!quiet || !process.env.ELECTRON_GET_NO_PROGRESS) { + if (!quiet && !process.env.ELECTRON_GET_NO_PROGRESS) { const start = new Date(); timeout = setTimeout(() => { if (!downloadCompleted) { @@ -83,11 +83,11 @@ export class GotDownloader implements Downloader { error.message += ` for ${(error as HTTPError).response.url}`; } throw error; - } - - downloadCompleted = true; - if (timeout) { - clearTimeout(timeout); + } finally { + downloadCompleted = true; + if (timeout) { + clearTimeout(timeout); + } } } } diff --git a/src/index.ts b/src/index.ts index 1ce46baf..93c77407 100644 --- a/src/index.ts +++ b/src/index.ts @@ -82,6 +82,7 @@ async function validateArtifact( cacheRoot: artifactDetails.cacheRoot, downloader: artifactDetails.downloader, mirrorOptions: artifactDetails.mirrorOptions, + tempDirectory: artifactDetails.tempDirectory, // Never use the cache for loading checksums, load // them fresh every time cacheMode: ElectronDownloadCacheMode.Bypass, @@ -114,9 +115,7 @@ async function validateArtifact( } } }, - doesCallerOwnTemporaryOutput(effectiveCacheMode(artifactDetails)) - ? TempDirCleanUpMode.ORPHAN - : TempDirCleanUpMode.CLEAN, + TempDirCleanUpMode.CLEAN, ); } diff --git a/src/proxy.ts b/src/proxy.ts index 7f5bfc67..732e3953 100644 --- a/src/proxy.ts +++ b/src/proxy.ts @@ -1,7 +1,10 @@ +import { createRequire } from 'node:module'; + import debug from 'debug'; import { getEnv, setEnv } from './utils.js'; const d = debug('@electron/get:proxy'); +const require = createRequire(import.meta.url); /** * Initializes a third-party proxy module for HTTP(S) requests. Call this function before diff --git a/test/Cache.spec.ts b/test/Cache.spec.ts index ccf5445d..8bfd3b48 100644 --- a/test/Cache.spec.ts +++ b/test/Cache.spec.ts @@ -12,7 +12,7 @@ describe('Cache', () => { let cache: Cache; const dummyUrl = 'dummy://dummypath'; - const sanitizedDummyUrl = '0c57d948bd4829db99d75c3b4a5d6836c37bc335f38012981baf5d1193b5a612'; + const sanitizedDummyUrl = 'a1f9d38d51b311ef8e4a0accc1c4d98b8b17b8b7469c5dc987b6f7ba9b6ff350'; beforeEach(async () => { cacheDir = await fs.promises.mkdtemp(path.resolve(os.tmpdir(), 'electron-download-spec-')); @@ -21,6 +21,19 @@ describe('Cache', () => { afterEach(() => fs.promises.rm(cacheDir, { recursive: true, force: true })); + describe('getCacheDirectory()', () => { + it('should produce a stable cache key for real-world download URLs', () => { + // This hash is part of the on-disk cache layout. Changing it invalidates + // every user's existing Electron cache, so any diff here must be + // deliberate and called out. + expect( + Cache.getCacheDirectory( + 'https://github.com/electron/electron/releases/download/v2.0.9/electron-v2.0.9-darwin-x64.zip', + ), + ).toEqual('7658513ccebc15fd4c4dec9ff8cdef8eb586f7fe5bad56f08b218650df37b1b6'); + }); + }); + describe('getCachePath()', () => { it('should strip the hash and query params off the url', async () => { const firstUrl = 'https://example.com?foo=1'; diff --git a/test/GotDownloader.spec.ts b/test/GotDownloader.spec.ts new file mode 100644 index 00000000..5948212f --- /dev/null +++ b/test/GotDownloader.spec.ts @@ -0,0 +1,100 @@ +import fs from 'graceful-fs'; +import os from 'node:os'; +import path from 'node:path'; + +import { PathLike } from 'node:fs'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { GotDownloader } from '../src/GotDownloader'; + +async function flushMicrotasks(): Promise { + for (let i = 0; i < 10; i++) { + await Promise.resolve(); + } +} + +describe('GotDownloader', () => { + let tmpDir: string; + + beforeEach(async () => { + tmpDir = await fs.promises.mkdtemp(path.resolve(os.tmpdir(), 'got-spec-')); + }); + + afterEach(async () => { + await fs.promises.rm(tmpDir, { recursive: true, force: true }); + delete process.env.ELECTRON_GET_NO_PROGRESS; + vi.useRealTimers(); + vi.restoreAllMocks(); + }); + + describe('progress bar suppression', () => { + it('should not schedule a progress bar timer when quiet: true', async () => { + vi.spyOn(fs.promises, 'mkdir').mockResolvedValue(undefined); + vi.useFakeTimers(); + + const downloader = new GotDownloader(); + const target = path.resolve(tmpDir, 'out.txt'); + + const p = downloader.download('http://127.0.0.1:1/nope', target, { quiet: true }); + p.catch(() => { + /* ignore */ + }); + + await flushMicrotasks(); + + expect(vi.getTimerCount()).toBe(0); + + vi.useRealTimers(); + await p.catch(() => { + /* ignore */ + }); + }); + + it('should not schedule a progress bar timer when ELECTRON_GET_NO_PROGRESS is set', async () => { + process.env.ELECTRON_GET_NO_PROGRESS = '1'; + vi.spyOn(fs.promises, 'mkdir').mockResolvedValue(undefined); + vi.useFakeTimers(); + + const downloader = new GotDownloader(); + const target = path.resolve(tmpDir, 'out.txt'); + + const p = downloader.download('http://127.0.0.1:1/nope', target); + p.catch(() => { + /* ignore */ + }); + + await flushMicrotasks(); + + expect(vi.getTimerCount()).toBe(0); + + vi.useRealTimers(); + await p.catch(() => { + /* ignore */ + }); + }); + }); + + describe('timer cleanup', () => { + it('should clear the progress timer even when the download fails', async () => { + vi.spyOn(fs.promises, 'mkdir').mockResolvedValue(undefined); + + const realCreateWriteStream = fs.createWriteStream; + vi.spyOn(fs, 'createWriteStream').mockImplementationOnce((p: PathLike) => { + const stream = realCreateWriteStream(p); + setImmediate(() => stream.emit('error', new Error('boom'))); + return stream; + }); + + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }); + + const downloader = new GotDownloader(); + const target = path.resolve(tmpDir, 'out.txt'); + + await expect(downloader.download('http://127.0.0.1:1/nope', target)).rejects.toThrow(); + + expect(vi.getTimerCount()).toBe(0); + + vi.clearAllTimers(); + }); + }); +}); diff --git a/test/index.spec.ts b/test/index.spec.ts index ef2fbe25..cc215ce4 100644 --- a/test/index.spec.ts +++ b/test/index.spec.ts @@ -271,6 +271,70 @@ describe('Public API', () => { expect(await util.promisify(fs.readFile)(driverPath2, 'utf8')).toEqual('cached content'); }); + describe('tempDirectory', () => { + let customTemp: string; + + beforeEach(async () => { + customTemp = await fs.promises.mkdtemp( + path.resolve(os.tmpdir(), 'electron-download-spec-temp-'), + ); + }); + + afterEach(async () => { + await fs.promises.rm(customTemp, { recursive: true, force: true }); + }); + + it('should use the custom tempDirectory for the SHASUMS256.txt download', async () => { + const shasumsTargetDirs: string[] = []; + + const trackingDownloader = { + async download(url: string, targetFilePath: string): Promise { + if (url.endsWith('SHASUMS256.txt')) { + shasumsTargetDirs.push(path.dirname(targetFilePath)); + } + return downloader.download(url, targetFilePath); + }, + }; + + await downloadArtifact({ + artifactName: 'electron', + version: '2.0.9', + platform: 'darwin', + arch: 'x64', + cacheRoot, + tempDirectory: customTemp, + downloader: trackingDownloader, + cacheMode: ElectronDownloadCacheMode.WriteOnly, + }); + + expect(shasumsTargetDirs.length).toBeGreaterThan(0); + for (const dir of shasumsTargetDirs) { + expect(dir.startsWith(customTemp)).toBe(true); + } + }); + + it('should not leak temp directories when cacheMode=Bypass', async () => { + const before = await fs.promises.readdir(customTemp); + expect(before).toEqual([]); + + const artifactPath = await downloadArtifact({ + artifactName: 'electron', + version: '2.0.9', + platform: 'darwin', + arch: 'x64', + cacheRoot, + tempDirectory: customTemp, + downloader, + cacheMode: ElectronDownloadCacheMode.Bypass, + }); + + await fs.promises.rm(path.dirname(artifactPath), { recursive: true, force: true }); + + const after = await fs.promises.readdir(customTemp); + expect(after).toEqual([]); + }); + }); + describe('sumchecker', () => { beforeEach(() => { vi.clearAllMocks(); diff --git a/test/proxy.spec.ts b/test/proxy.spec.ts new file mode 100644 index 00000000..8c96b861 --- /dev/null +++ b/test/proxy.spec.ts @@ -0,0 +1,15 @@ +import fs from 'graceful-fs'; +import path from 'node:path'; + +import { describe, expect, it } from 'vitest'; + +describe('initializeProxy', () => { + // `require` is not defined in ESM modules. vitest injects a shim, so the + // runtime failure only manifests when running the built package under + // plain Node.js. Guard against regressions by checking the source uses + // createRequire(import.meta.url) to define require before calling it. + it('defines require via createRequire before using it', () => { + const source = fs.readFileSync(path.resolve(__dirname, '../src/proxy.ts'), 'utf8'); + expect(source).toMatch(/createRequire\s*\(\s*import\.meta\.url\s*\)/); + }); +});