From be862afd1a9814197799e2c718fba0742c8e234c Mon Sep 17 00:00:00 2001
From: Scott Nonnenberg <scott@signal.org>
Date: Mon, 25 Oct 2021 12:41:44 -0700
Subject: [PATCH] base_config: Don't throw if loaded config file is empty

---
 app/base_config.ts                   | 25 +++++++---
 app/main.ts                          |  3 +-
 ts/test-node/app/base_config_test.ts | 71 ++++++++++++++++++++++++++++
 3 files changed, 90 insertions(+), 9 deletions(-)
 create mode 100644 ts/test-node/app/base_config_test.ts

diff --git a/app/base_config.ts b/app/base_config.ts
index 314e7afd0d6..d3056e2db21 100644
--- a/app/base_config.ts
+++ b/app/base_config.ts
@@ -13,6 +13,9 @@ export type ConfigType = {
   set: (keyPath: string, value: unknown) => void;
   get: (keyPath: string) => unknown;
   remove: () => void;
+
+  // Test-only
+  _getCachedValue: () => InternalConfigType | undefined;
 };
 
 export function start(
@@ -21,10 +24,11 @@ export function start(
   options?: { allowMalformedOnStartup?: boolean }
 ): ConfigType {
   let cachedValue: InternalConfigType | undefined;
+  let incomingJson: string | undefined;
 
   try {
-    const text = readFileSync(targetPath, ENCODING);
-    cachedValue = JSON.parse(text);
+    incomingJson = readFileSync(targetPath, ENCODING);
+    cachedValue = incomingJson ? JSON.parse(incomingJson) : undefined;
     console.log(`config/get: Successfully read ${name} config file`);
 
     if (!cachedValue) {
@@ -38,9 +42,15 @@ export function start(
       throw error;
     }
 
-    console.log(
-      `config/get: Did not find ${name} config file, cache is now empty object`
-    );
+    if (incomingJson) {
+      console.log(
+        `config/get: ${name} config file was malformed, starting afresh`
+      );
+    } else {
+      console.log(
+        `config/get: Did not find ${name} config file (or it was empty), cache is now empty object`
+      );
+    }
     cachedValue = Object.create(null);
   }
 
@@ -55,8 +65,8 @@ export function start(
 
     set(cachedValue, keyPath, value);
     console.log(`config/set: Saving ${name} config to disk`);
-    const text = JSON.stringify(cachedValue, null, '  ');
-    writeFileSync(targetPath, text, ENCODING);
+    const outgoingJson = JSON.stringify(cachedValue, null, '  ');
+    writeFileSync(targetPath, outgoingJson, ENCODING);
   }
 
   function remove(): void {
@@ -69,5 +79,6 @@ export function start(
     set: ourSet,
     get: ourGet,
     remove,
+    _getCachedValue: () => cachedValue,
   };
 }
diff --git a/app/main.ts b/app/main.ts
index ec4ffd2c8f6..f523719cdd7 100644
--- a/app/main.ts
+++ b/app/main.ts
@@ -31,7 +31,6 @@ import * as GlobalErrors from './global_errors';
 import { setup as setupSpellChecker } from './spell_check';
 import { redactAll, addSensitivePath } from '../ts/util/privacy';
 import { consoleLogger } from '../ts/util/consoleLogger';
-import { remove as removeUserConfig } from './user_config';
 
 import './startup_config';
 
@@ -1318,7 +1317,7 @@ const onDatabaseError = async (error: string) => {
     clipboard.writeText(`Database startup error:\n\n${redactAll(error)}`);
   } else {
     await sql.removeDB();
-    removeUserConfig();
+    userConfig.remove();
     app.relaunch();
   }
 
diff --git a/ts/test-node/app/base_config_test.ts b/ts/test-node/app/base_config_test.ts
new file mode 100644
index 00000000000..8a88ee7fcb6
--- /dev/null
+++ b/ts/test-node/app/base_config_test.ts
@@ -0,0 +1,71 @@
+// Copyright 2021 Signal Messenger, LLC
+// SPDX-License-Identifier: AGPL-3.0-only
+
+import { tmpdir } from 'os';
+import { writeFileSync, unlinkSync } from 'fs';
+
+import { v4 as generateGuid } from 'uuid';
+import { assert } from 'chai';
+
+import { start } from '../../../app/base_config';
+
+describe('base_config', () => {
+  let targetFile: string | undefined;
+
+  function getNewPath() {
+    return `${tmpdir()}/${generateGuid()}.txt`;
+  }
+
+  afterEach(() => {
+    if (targetFile) {
+      unlinkSync(targetFile);
+    }
+  });
+
+  it('does not throw if file is missing', () => {
+    const missingFile = getNewPath();
+    const { _getCachedValue } = start('test', missingFile);
+
+    assert.deepEqual(_getCachedValue(), Object.create(null));
+  });
+
+  it('successfully loads config file', () => {
+    targetFile = getNewPath();
+
+    const config = { a: 1, b: 2 };
+    writeFileSync(targetFile, JSON.stringify(config));
+    const { _getCachedValue } = start('test', targetFile);
+
+    assert.deepEqual(_getCachedValue(), config);
+  });
+
+  it('throws if file is malformed', () => {
+    targetFile = getNewPath();
+
+    writeFileSync(targetFile, '{{ malformed JSON');
+
+    const fileForClosure = targetFile;
+    assert.throws(() => start('test', fileForClosure));
+  });
+
+  it('does not throw if file is empty', () => {
+    targetFile = getNewPath();
+
+    writeFileSync(targetFile, '');
+
+    const { _getCachedValue } = start('test', targetFile);
+
+    assert.deepEqual(_getCachedValue(), Object.create(null));
+  });
+
+  it('does not throw if file is malformed, with allowMalformedOnStartup', () => {
+    targetFile = getNewPath();
+
+    writeFileSync(targetFile, '{{ malformed JSON');
+    const { _getCachedValue } = start('test', targetFile, {
+      allowMalformedOnStartup: true,
+    });
+
+    assert.deepEqual(_getCachedValue(), Object.create(null));
+  });
+});