Skip to content

Commit cc9d8fc

Browse files
seishunbpasero
seishun
authored and
bpasero
committed
Use r+ with truncation when saving existing files (microsoft#31733)
* Use r+ with truncation when saving existing files 'w' doesn't work with hidden files. Fixes: microsoft#931 * use r+ only if EPERM is caught * add check for Windows * move writing to a separate method * nits * remove string overload for options * remove encoding property from writeFileAndFlush
1 parent a8d63d3 commit cc9d8fc

3 files changed

Lines changed: 45 additions & 26 deletions

File tree

‎src/vs/base/node/extfs.ts‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -332,15 +332,13 @@ export function mv(source: string, target: string, callback: (error: Error) => v
332332
//
333333
// See https://github.com/nodejs/node/blob/v5.10.0/lib/fs.js#L1194
334334
let canFlush = true;
335-
export function writeFileAndFlush(path: string, data: string | NodeBuffer, options: { encoding?: string; mode?: number; flag?: string; }, callback: (error: Error) => void): void {
335+
export function writeFileAndFlush(path: string, data: string | NodeBuffer, options: { mode?: number; flag?: string; }, callback: (error: Error) => void): void {
336336
if (!canFlush) {
337337
return fs.writeFile(path, data, options, callback);
338338
}
339339

340340
if (!options) {
341-
options = { encoding: 'utf8', mode: 0o666, flag: 'w' };
342-
} else if (typeof options === 'string') {
343-
options = { encoding: <string>options, mode: 0o666, flag: 'w' };
341+
options = { mode: 0o666, flag: 'w' };
344342
}
345343

346344
// Open the file with same flags and mode as fs.writeFile()
@@ -350,7 +348,7 @@ export function writeFileAndFlush(path: string, data: string | NodeBuffer, optio
350348
}
351349

352350
// It is valid to pass a fd handle to fs.writeFile() and this will keep the handle open!
353-
fs.writeFile(fd, data, options.encoding, (writeError) => {
351+
fs.writeFile(fd, data, (writeError) => {
354352
if (writeError) {
355353
return fs.close(fd, () => callback(writeError)); // still need to close the handle on error!
356354
}

‎src/vs/base/node/pfs.ts‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,10 @@ export function touch(path: string): TPromise<void> {
109109
return nfcall(fs.utimes, path, now, now);
110110
}
111111

112+
export function truncate(path: string, len: number): TPromise<void> {
113+
return nfcall(fs.truncate, path, len);
114+
}
115+
112116
export function readFile(path: string): TPromise<Buffer>;
113117
export function readFile(path: string, encoding: string): TPromise<string>;
114118
export function readFile(path: string, encoding?: string): TPromise<Buffer | string> {
@@ -120,12 +124,12 @@ export function readFile(path: string, encoding?: string): TPromise<Buffer | str
120124
// Therefor we use a Queue on the path that is given to us to sequentialize calls to the same path properly.
121125
const writeFilePathQueue: { [path: string]: Queue<void> } = Object.create(null);
122126

123-
export function writeFile(path: string, data: string, encoding?: string): TPromise<void>;
124-
export function writeFile(path: string, data: NodeBuffer, encoding?: string): TPromise<void>;
125-
export function writeFile(path: string, data: any, encoding: string = 'utf8'): TPromise<void> {
127+
export function writeFile(path: string, data: string, options?: { mode?: number; flag?: string; }): TPromise<void>;
128+
export function writeFile(path: string, data: NodeBuffer, options?: { mode?: number; flag?: string; }): TPromise<void>;
129+
export function writeFile(path: string, data: any, options?: { mode?: number; flag?: string; }): TPromise<void> {
126130
let queueKey = toQueueKey(path);
127131

128-
return ensureWriteFileQueue(queueKey).queue(() => nfcall(extfs.writeFileAndFlush, path, data, encoding));
132+
return ensureWriteFileQueue(queueKey).queue(() => nfcall(extfs.writeFileAndFlush, path, data, options));
129133
}
130134

131135
function toQueueKey(path: string): string {

‎src/vs/workbench/services/files/node/fileService.ts‎

Lines changed: 34 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -348,30 +348,47 @@ export class FileService implements IFileService {
348348

349349
// 3.) check to add UTF BOM
350350
return addBomPromise.then(addBom => {
351-
let writeFilePromise: TPromise<void>;
352-
353-
// Write fast if we do UTF 8 without BOM
354-
if (!addBom && encodingToWrite === encoding.UTF8) {
355-
writeFilePromise = pfs.writeFile(absolutePath, value, encoding.UTF8);
356-
}
357-
358-
// Otherwise use encoding lib
359-
else {
360-
const encoded = encoding.encode(value, encodingToWrite, { addBOM: addBom });
361-
writeFilePromise = pfs.writeFile(absolutePath, encoded);
362-
}
363-
364-
// 4.) set contents
365-
return writeFilePromise.then(() => {
351+
// 4.) set contents and resolve
352+
return this.doSetContentsAndResolve(resource, absolutePath, value, addBom, encodingToWrite, { mode: 0o666, flag: 'w' }).then(undefined, error => {
353+
// Can't use 'w' for hidden files on Windows (see https://github.com/Microsoft/vscode/issues/931)
354+
if (!exists || error.code !== 'EPERM' || !isWindows) {
355+
return TPromise.wrapError(error);
356+
}
366357

367-
// 5.) resolve
368-
return this.resolve(resource);
358+
// 'r+' always works for existing files, but we need to truncate manually
359+
// 5.) truncate
360+
return pfs.truncate(absolutePath, 0).then(() => {
361+
// 6.) set contents and resolve again
362+
return this.doSetContentsAndResolve(resource, absolutePath, value, addBom, encodingToWrite, { mode: 0o666, flag: 'r+' });
363+
});
369364
});
370365
});
371366
});
372367
});
373368
}
374369

370+
private doSetContentsAndResolve(resource: uri, absolutePath: string, value: string, addBOM: boolean, encodingToWrite: string, options: { mode?: number; flag?: string; }): TPromise<IFileStat> {
371+
let writeFilePromise: TPromise<void>;
372+
373+
// Write fast if we do UTF 8 without BOM
374+
if (!addBOM && encodingToWrite === encoding.UTF8) {
375+
writeFilePromise = pfs.writeFile(absolutePath, value, options);
376+
}
377+
378+
// Otherwise use encoding lib
379+
else {
380+
const encoded = encoding.encode(value, encodingToWrite, { addBOM });
381+
writeFilePromise = pfs.writeFile(absolutePath, encoded, options);
382+
}
383+
384+
// set contents
385+
return writeFilePromise.then(() => {
386+
387+
// resolve
388+
return this.resolve(resource);
389+
});
390+
}
391+
375392
public createFile(resource: uri, content: string = ''): TPromise<IFileStat> {
376393

377394
// Create file

0 commit comments

Comments
 (0)