From 62ebaa822b0770558ffc3b8fba291996e2c4a5b0 Mon Sep 17 00:00:00 2001 From: Veetaha Date: Wed, 24 Jun 2020 13:19:14 +0300 Subject: Don't mess with messy temp dir and just download into extension dir Temp dirs are messy. Dealing with them requires handling quite a bunch of edge cases. As proposed by lnicola this seems better to just put the temp files in the extension dir and not care much about suddenly leaving garbage. Instead we get shorter and less platform-caveat-y code. We will also assume users don't try to issue a download in different vscode windows simultaneously --- editors/code/src/net.ts | 80 ++++++++++++++----------------------------------- 1 file changed, 23 insertions(+), 57 deletions(-) (limited to 'editors') diff --git a/editors/code/src/net.ts b/editors/code/src/net.ts index e02fd6d4f..7c77530b8 100644 --- a/editors/code/src/net.ts +++ b/editors/code/src/net.ts @@ -2,8 +2,6 @@ import fetch from "node-fetch"; import * as vscode from "vscode"; import * as stream from "stream"; import * as fs from "fs"; -import * as os from "os"; -import * as path from "path"; import * as util from "util"; import { log, assert } from "./util"; @@ -68,32 +66,31 @@ interface DownloadOpts { } export async function download(opts: DownloadOpts) { - // Put the artifact into a temporary folder to prevent partially downloaded files when user kills vscode - await withTempDir(async tempDir => { - const tempFile = path.join(tempDir, path.basename(opts.dest)); - - await vscode.window.withProgress( - { - location: vscode.ProgressLocation.Notification, - cancellable: false, - title: opts.progressTitle - }, - async (progress, _cancellationToken) => { - let lastPercentage = 0; - await downloadFile(opts.url, tempFile, opts.mode, (readBytes, totalBytes) => { - const newPercentage = (readBytes / totalBytes) * 100; - progress.report({ - message: newPercentage.toFixed(0) + "%", - increment: newPercentage - lastPercentage - }); - - lastPercentage = newPercentage; + // Put artifact into a temporary file (in the same dir for simplicity) + // to prevent partially downloaded files when user kills vscode + const tempFile = `${opts.dest}.tmp`; + + await vscode.window.withProgress( + { + location: vscode.ProgressLocation.Notification, + cancellable: false, + title: opts.progressTitle + }, + async (progress, _cancellationToken) => { + let lastPercentage = 0; + await downloadFile(opts.url, tempFile, opts.mode, (readBytes, totalBytes) => { + const newPercentage = (readBytes / totalBytes) * 100; + progress.report({ + message: newPercentage.toFixed(0) + "%", + increment: newPercentage - lastPercentage }); - } - ); - await moveFile(tempFile, opts.dest); - }); + lastPercentage = newPercentage; + }); + } + ); + + await fs.promises.rename(tempFile, opts.dest); } /** @@ -137,34 +134,3 @@ async function downloadFile( // https://github.com/rust-analyzer/rust-analyzer/issues/3167 }); } - -async function withTempDir(scope: (tempDirPath: string) => Promise) { - // Based on the great article: https://advancedweb.hu/secure-tempfiles-in-nodejs-without-dependencies/ - - // `.realpath()` should handle the cases where os.tmpdir() contains symlinks - const osTempDir = await fs.promises.realpath(os.tmpdir()); - - const tempDir = await fs.promises.mkdtemp(path.join(osTempDir, "rust-analyzer")); - - try { - return await scope(tempDir); - } finally { - // We are good citizens :D - void fs.promises.rmdir(tempDir, { recursive: true }).catch(log.error); - } -}; - -async function moveFile(src: fs.PathLike, dest: fs.PathLike) { - try { - await fs.promises.rename(src, dest); - } catch (err) { - if (err.code === 'EXDEV') { - // We are probably moving the file across partitions/devices - await fs.promises.copyFile(src, dest); - await fs.promises.unlink(src); - } else { - log.error(`Failed to rename the file ${src} -> ${dest}`, err); - throw err; - } - } -} -- cgit v1.2.3 From c1d39571c9ac27c80ca2c1feb5dd53fc6f325b34 Mon Sep 17 00:00:00 2001 From: Veetaha Date: Thu, 25 Jun 2020 01:00:30 +0300 Subject: Append 10 random hex chars to temp artifact files --- editors/code/src/net.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'editors') diff --git a/editors/code/src/net.ts b/editors/code/src/net.ts index 7c77530b8..866092882 100644 --- a/editors/code/src/net.ts +++ b/editors/code/src/net.ts @@ -1,8 +1,10 @@ import fetch from "node-fetch"; import * as vscode from "vscode"; import * as stream from "stream"; +import * as crypto from "crypto"; import * as fs from "fs"; import * as util from "util"; +import * as path from "path"; import { log, assert } from "./util"; const pipeline = util.promisify(stream.pipeline); @@ -68,7 +70,9 @@ interface DownloadOpts { export async function download(opts: DownloadOpts) { // Put artifact into a temporary file (in the same dir for simplicity) // to prevent partially downloaded files when user kills vscode - const tempFile = `${opts.dest}.tmp`; + const dest = path.parse(opts.dest); + const randomHex = crypto.randomBytes(5).toString("hex"); + const tempFile = path.join(dest.dir, `${dest.name}${randomHex}`); await vscode.window.withProgress( { -- cgit v1.2.3