diff --git a/src/shadowbox/infrastructure/file.spec.ts b/src/shadowbox/infrastructure/file.spec.ts new file mode 100644 index 00000000..738c7362 --- /dev/null +++ b/src/shadowbox/infrastructure/file.spec.ts @@ -0,0 +1,63 @@ +import * as fs from 'fs'; +import * as tmp from 'tmp'; +import * as file from './file'; + +describe('file', () => { + tmp.setGracefulCleanup(); + + describe('readFileIfExists', () => { + let tmpFile: tmp.FileResult; + + beforeEach(() => tmpFile = tmp.fileSync()); + + it('reads the file if it exists', () => { + const contents = 'test'; + + fs.writeFileSync(tmpFile.name, contents); + + expect(file.readFileIfExists(tmpFile.name)).toBe(contents); + }); + + it('reads the file if it exists and is empty', () => { + fs.writeFileSync(tmpFile.name, ''); + + expect(file.readFileIfExists(tmpFile.name)).toBe(''); + }); + + it('returns null if file doesn\'t exist', + () => expect(file.readFileIfExists(tmp.tmpNameSync())).toBe(null)); + }); + + describe('atomicWriteFileSync', () => { + let tmpFile: tmp.FileResult; + + beforeEach(() => tmpFile = tmp.fileSync()); + + it('writes to the file', () => { + const contents = 'test'; + + file.atomicWriteFileSync(tmpFile.name, contents); + + expect(fs.readFileSync(tmpFile.name, {encoding: 'utf8'})).toEqual(contents); + }); + + it('supports multiple simultaneous writes to the same file', async () => { + const writeCount = 100; + + const writer = (_, id) => new Promise((resolve, reject) => { + try { + file.atomicWriteFileSync( + tmpFile.name, `${fs.readFileSync(tmpFile.name, {encoding: 'utf-8'})}${id}\n`); + resolve(); + } catch (e) { + reject(e); + } + }); + + await Promise.all(Array.from({length: writeCount}, writer)); + + expect(fs.readFileSync(tmpFile.name, {encoding: 'utf8'}).trimEnd().split('\n').length) + .toBe(writeCount); + }); + }); +}); diff --git a/src/shadowbox/infrastructure/file_read.ts b/src/shadowbox/infrastructure/file.ts similarity index 65% rename from src/shadowbox/infrastructure/file_read.ts rename to src/shadowbox/infrastructure/file.ts index 08aa657f..665bf890 100644 --- a/src/shadowbox/infrastructure/file_read.ts +++ b/src/shadowbox/infrastructure/file.ts @@ -18,7 +18,7 @@ import * as fs from 'fs'; // Throws any other error except file not found. export function readFileIfExists(filename: string): string { try { - return fs.readFileSync(filename, {encoding: 'utf8'}) || null; + return fs.readFileSync(filename, {encoding: 'utf8'}) ?? null; } catch (err) { // err.code will be 'ENOENT' if the file is not found, this is expected. if (err.code === 'ENOENT') { @@ -28,3 +28,13 @@ export function readFileIfExists(filename: string): string { } } } + +// Write to temporary file, then move that temporary file to the +// persistent location, to avoid accidentally breaking the metrics file. +// Use *Sync calls for atomic operations, to guard against corrupting +// these files. +export function atomicWriteFileSync(filename: string, filebody: string) { + const tempFilename = `${filename}.${Date.now()}`; + fs.writeFileSync(tempFilename, filebody, {encoding: 'utf8'}); + fs.renameSync(tempFilename, filename); +} diff --git a/src/shadowbox/infrastructure/json_config.ts b/src/shadowbox/infrastructure/json_config.ts index cd882fca..29d40cf9 100644 --- a/src/shadowbox/infrastructure/json_config.ts +++ b/src/shadowbox/infrastructure/json_config.ts @@ -12,9 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -import * as fs from 'fs'; - -import * as file_read from './file_read'; +import * as file from './file'; import * as logging from './logging'; export interface JsonConfig { @@ -25,7 +23,7 @@ export interface JsonConfig { } export function loadFileConfig(filename: string): JsonConfig { - const text = file_read.readFileIfExists(filename); + const text = file.readFileIfExists(filename); let dataJson = {} as T; if (text) { dataJson = JSON.parse(text) as T; @@ -33,6 +31,7 @@ export function loadFileConfig(filename: string): JsonConfig { return new FileConfig(filename, dataJson); } + // FileConfig is a JsonConfig backed by a filesystem file. export class FileConfig implements JsonConfig { constructor(private filename: string, private dataJson: T) {} @@ -42,14 +41,8 @@ export class FileConfig implements JsonConfig { } write() { - // Write to temporary file, then move that temporary file to the - // persistent location, to avoid accidentally breaking the metrics file. - // Use *Sync calls for atomic operations, to guard against corrupting - // these files. - const tempFilename = `${this.filename}.${Date.now()}`; try { - fs.writeFileSync(tempFilename, JSON.stringify(this.dataJson), {encoding: 'utf8'}); - fs.renameSync(tempFilename, this.filename); + file.atomicWriteFileSync(this.filename, JSON.stringify(this.dataJson)); } catch (error) { // TODO: Stop swallowing the exception and handle it in the callers. logging.error(`Error writing config ${this.filename} ${error}`); diff --git a/src/shadowbox/package.json b/src/shadowbox/package.json index 77888e92..e39629ca 100644 --- a/src/shadowbox/package.json +++ b/src/shadowbox/package.json @@ -28,6 +28,8 @@ "@types/randomstring": "^1.1.6", "@types/restify": "^8.4.2", "@types/restify-cors-middleware": "^1.0.1", + "@types/tmp": "^0.2.1", + "tmp": "^0.2.1", "ts-loader": "^7.0.4", "webpack": "^4.43.0", "webpack-cli": "^3.3.11" diff --git a/src/shadowbox/server/outline_shadowsocks_server.ts b/src/shadowbox/server/outline_shadowsocks_server.ts index 69e8b0b6..4fe1b03d 100644 --- a/src/shadowbox/server/outline_shadowsocks_server.ts +++ b/src/shadowbox/server/outline_shadowsocks_server.ts @@ -13,11 +13,11 @@ // limitations under the License. import * as child_process from 'child_process'; -import * as fs from 'fs'; import * as jsyaml from 'js-yaml'; import * as mkdirp from 'mkdirp'; import * as path from 'path'; +import * as file from '../infrastructure/file'; import * as logging from '../infrastructure/logging'; import {ShadowsocksAccessKey, ShadowsocksServer} from '../model/shadowsocks_server'; @@ -68,16 +68,18 @@ export class OutlineShadowsocksServer implements ShadowsocksServer { key.id} is not supported: use an AEAD cipher instead.`); continue; } + keysJson.keys.push(key); } - const ymlTxt = jsyaml.safeDump(keysJson, {'sortKeys': true}); + mkdirp.sync(path.dirname(this.configFilename)); - fs.writeFile(this.configFilename, ymlTxt, 'utf-8', (err) => { - if (err) { - reject(err); - } + + try { + file.atomicWriteFileSync(this.configFilename, jsyaml.safeDump(keysJson, {sortKeys: true})); resolve(); - }); + } catch (error) { + reject(error); + } }); } diff --git a/yarn.lock b/yarn.lock index 054779da..0f1b4be5 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1038,6 +1038,11 @@ resolved "https://registry.yarnpkg.com/@types/tapable/-/tapable-1.0.8.tgz#b94a4391c85666c7b73299fd3ad79d4faa435310" integrity sha512-ipixuVrh2OdNmauvtT51o3d8z12p6LtFW9in7U79der/kwejjdNchQC5UMn5u/KxNoM7VHHOs/l8KS8uHxhODQ== +"@types/tmp@^0.2.1": + version "0.2.1" + resolved "https://registry.yarnpkg.com/@types/tmp/-/tmp-0.2.1.tgz#83ecf4ec22a8c218c71db25f316619fe5b986011" + integrity sha512-7cTXwKP/HLOPVgjg+YhBdQ7bMiobGMuoBmrGmqwIWJv8elC6t1DfVc/mn4fD9UE1IjhwmhaQ5pGVXkmXbH0rhg== + "@types/tough-cookie@*": version "4.0.1" resolved "https://registry.yarnpkg.com/@types/tough-cookie/-/tough-cookie-4.0.1.tgz#8f80dd965ad81f3e1bc26d6f5c727e132721ff40" @@ -9969,7 +9974,7 @@ tinycolor2@^1.1.2: resolved "https://registry.yarnpkg.com/tinycolor2/-/tinycolor2-1.4.2.tgz#3f6a4d1071ad07676d7fa472e1fac40a719d8803" integrity sha512-vJhccZPs965sV/L2sU4oRQVAos0pQXwsvTLkWYdqJ+a8Q5kPFzJTuOFwy7UniPli44NKQGAglksjvOcpo95aZA== -tmp@0.2.1: +tmp@0.2.1, tmp@^0.2.1: version "0.2.1" resolved "https://registry.yarnpkg.com/tmp/-/tmp-0.2.1.tgz#8457fc3037dcf4719c251367a1af6500ee1ccf14" integrity sha512-76SUhtfqR2Ijn+xllcI5P1oyannHNHByD80W1q447gU3mp9G9PSpGdWmjUOHRDPiHYacIk66W7ubDTuPF3BEtQ==