fix(config): never let app.json I/O stop startup and reject numeric enum values
This commit is contained in:
1 parent
7f277d1630
commit
1140cd500b
2 files changed
+60
-11
No files matched your search
@@ -53,6 +53,39 @@ public class AppConfigStoreTests {
|
|||||||
Assert.AreEqual(AppConfig.CurrentFormatVersion, store.Load().Version);
|
Assert.AreEqual(AppConfig.CurrentFormatVersion, store.Load().Version);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[TestMethod]
|
||||||
|
public void LoadNumericEnumValueIsRejectedAndBackedUp() {
|
||||||
|
using var directory = new TempDirectory();
|
||||||
|
string path = Path.Combine(directory.FullPath, "app.json");
|
||||||
|
|
||||||
|
// Integer enum values are not accepted; the file is treated like corrupt.
|
||||||
|
File.WriteAllText(path, """{ "theme": 1 }""");
|
||||||
|
var store = new AppConfigStore(path);
|
||||||
|
|
||||||
|
AppConfig config = store.Load();
|
||||||
|
|
||||||
|
Assert.AreEqual(ThemeOption.FollowSystem, config.Theme);
|
||||||
|
Assert.IsTrue(File.Exists(path + ".bak"));
|
||||||
|
}
|
||||||
|
|
||||||
|
[TestMethod]
|
||||||
|
public void LoadUnreadableFileReturnsDefaults() {
|
||||||
|
// File sharing is only reliably enforced on Windows, so this branch is exercised there.
|
||||||
|
if (!OperatingSystem.IsWindows()) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
using var directory = new TempDirectory();
|
||||||
|
string path = Path.Combine(directory.FullPath, "app.json");
|
||||||
|
File.WriteAllText(path, """{ "version": 1 }""");
|
||||||
|
|
||||||
|
// Hold the file exclusively so the store's read fails with an I/O error.
|
||||||
|
using FileStream hold = File.Open(path, FileMode.Open, FileAccess.Read, FileShare.None);
|
||||||
|
var store = new AppConfigStore(path);
|
||||||
|
|
||||||
|
Assert.AreEqual(AppConfig.CurrentFormatVersion, store.Load().Version);
|
||||||
|
}
|
||||||
|
|
||||||
[TestMethod]
|
[TestMethod]
|
||||||
public void LoadReadsKebabCaseKeysAndEnumValues() {
|
public void LoadReadsKebabCaseKeysAndEnumValues() {
|
||||||
using var directory = new TempDirectory();
|
using var directory = new TempDirectory();
|
||||||
|
|||||||
@@ -4,18 +4,23 @@ using Newtonsoft.Json.Converters;
|
|||||||
namespace YKanBan.Storage.AppData;
|
namespace YKanBan.Storage.AppData;
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Loads and saves app.json with graceful degradation: a missing file yields
|
/// Loads and saves app.json with graceful degradation: a missing or unreadable
|
||||||
/// defaults; a corrupt file is backed up to app.json.bak and then defaults are
|
/// file yields defaults; a corrupt file is backed up to app.json.bak and then
|
||||||
/// used. Saving writes the file directly (no atomic replace) — losing the last
|
/// defaults are used. Saving writes the file directly (no atomic replace) —
|
||||||
/// session's settings to a crash is an accepted trade-off.
|
/// losing the last session's settings to a crash is an accepted trade-off.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
public sealed class AppConfigStore {
|
public sealed class AppConfigStore {
|
||||||
private static readonly JsonSerializerSettings SerializerSettings = new() {
|
private static readonly JsonSerializerSettings SerializerSettings = new() {
|
||||||
Formatting = Formatting.Indented,
|
Formatting = Formatting.Indented,
|
||||||
NullValueHandling = NullValueHandling.Ignore,
|
NullValueHandling = NullValueHandling.Ignore,
|
||||||
|
|
||||||
// Enums round-trip through the kebab-case strings declared via [EnumMember].
|
// Enums round-trip through the kebab-case strings declared via [EnumMember];
|
||||||
Converters = { new StringEnumConverter() },
|
// numeric enum values are rejected like any other unknown value.
|
||||||
|
// YYC MARK: non-enum fields (bool/int/string) are still coerced by
|
||||||
|
// Newtonsoft (for example 0 or "true" to bool, "500" or 500.0 to int);
|
||||||
|
// a wrongly typed non-enum value is accepted and converted rather than
|
||||||
|
// treated as invalid. That leniency is accepted here.
|
||||||
|
Converters = { new StringEnumConverter { AllowIntegerValues = false } },
|
||||||
};
|
};
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
@@ -31,17 +36,24 @@ public sealed class AppConfigStore {
|
|||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Loads the configuration. A missing file yields defaults; a corrupt file
|
/// Loads the configuration. A missing file yields defaults; a corrupt file
|
||||||
/// is backed up to app.json.bak and then defaults are used.
|
/// is backed up to app.json.bak and then defaults are used; a file that
|
||||||
|
/// exists but cannot be read (I/O or permission error) also yields defaults.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
/// <returns>The loaded configuration, or defaults when none could be read.</returns>
|
/// <returns>The loaded configuration, or defaults when none could be read.</returns>
|
||||||
/// <exception cref="IOException">The file exists but cannot be read.</exception>
|
|
||||||
public AppConfig Load() {
|
public AppConfig Load() {
|
||||||
// Missing file is a normal first-run state: run with defaults.
|
// Missing file is a normal first-run state: run with defaults.
|
||||||
if (!File.Exists(FilePath)) {
|
if (!File.Exists(FilePath)) {
|
||||||
return new AppConfig();
|
return new AppConfig();
|
||||||
}
|
}
|
||||||
|
|
||||||
string json = File.ReadAllText(FilePath);
|
string json;
|
||||||
|
try {
|
||||||
|
json = File.ReadAllText(FilePath);
|
||||||
|
} catch (Exception exception) when (exception is IOException or UnauthorizedAccessException) {
|
||||||
|
// A bad app.json must not stop startup.
|
||||||
|
return new AppConfig();
|
||||||
|
}
|
||||||
|
|
||||||
try {
|
try {
|
||||||
// A JSON null literal deserializes to null; treat it like defaults.
|
// A JSON null literal deserializes to null; treat it like defaults.
|
||||||
AppConfig? config = JsonConvert.DeserializeObject<AppConfig>(json, SerializerSettings);
|
AppConfig? config = JsonConvert.DeserializeObject<AppConfig>(json, SerializerSettings);
|
||||||
@@ -54,8 +66,12 @@ public sealed class AppConfigStore {
|
|||||||
config.Confirmations ??= new ConfirmationSettings();
|
config.Confirmations ??= new ConfirmationSettings();
|
||||||
return config;
|
return config;
|
||||||
} catch (JsonException) {
|
} catch (JsonException) {
|
||||||
// Corrupt: keep a copy for inspection, then continue with defaults.
|
// Corrupt: keep a copy for inspection (best effort), then continue with defaults.
|
||||||
File.Copy(FilePath, FilePath + ".bak", overwrite: true);
|
try {
|
||||||
|
File.Copy(FilePath, FilePath + ".bak", overwrite: true);
|
||||||
|
} catch (Exception exception) when (exception is IOException or UnauthorizedAccessException) {
|
||||||
|
// The backup is only a diagnostic aid; starting with defaults matters more.
|
||||||
|
}
|
||||||
return new AppConfig();
|
return new AppConfig();
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in new issue
Block a user