Reject malformed saved state without losing the live history

UndoRedo.Test/SerializationTests.cs

@@ -392,6 +392,81 @@
 		Assert.IsInstanceOfType<MissingMethodException>(ex.InnerException, "The underlying reflection failure should be preserved");
 	}
 
+	[TestMethod]
+	[DataRow("""{"commands":[{"type":"x","description":"d"}],"currentPosition":0,"saveBoundaries":[],"formatVersion":"json-v1.0"}""", DisplayName = "command without metadata")]
+	[DataRow("""{"commands":null,"currentPosition":0,"saveBoundaries":[],"formatVersion":"json-v1.0"}""", DisplayName = "null commands")]
+	[DataRow("""{"commands":[null],"currentPosition":0,"saveBoundaries":[],"formatVersion":"json-v1.0"}""", DisplayName = "null command entry")]
+	[DataRow("""{"commands":[],"currentPosition":-1,"saveBoundaries":null,"formatVersion":"json-v1.0"}""", DisplayName = "null save boundaries")]
+	[DataRow("""{"commands":[],"currentPosition":-1,"saveBoundaries":[null],"formatVersion":"json-v1.0"}""", DisplayName = "null save boundary entry")]
+	[DataRow("""{"commands":[],"currentPosition":-1,"saveBoundaries":[],"formatVersion":null}""", DisplayName = "null format version")]
+	public async Task UndoRedoService_LoadStateMalformed_ReturnsFalseAndKeepsHistory(string json)
+	{
+		// Arrange: a stack with history the user would lose if a bad load cleared it
+		UndoRedoService stack = CreateService();
+		stack.SetSerializer(new JsonUndoRedoSerializer());
+		int value = 0;
+		stack.Execute(new DelegateCommand("A", () => value = 1, () => value = 0));
+		stack.Execute(new DelegateCommand("B", () => value = 2, () => value = 1));
+		stack.MarkAsSaved("after B");
+		await stack.UndoAsync().ConfigureAwait(false);
+
+		// Act
+		bool success = await stack.LoadStateAsync(System.Text.Encoding.UTF8.GetBytes(json)).ConfigureAwait(false);
+
+		// Assert
+		Assert.IsFalse(success, "LoadStateAsync should return false for data it cannot load");
+		Assert.AreEqual(2, stack.CommandCount, "A failed load should keep the existing commands");
+		Assert.AreEqual(0, stack.CurrentPosition, "A failed load should keep the existing position");
+		Assert.HasCount(1, stack.SaveBoundaries, "A failed load should keep the existing save boundaries");
+		Assert.AreEqual(1, value);
+	}
+
+	[TestMethod]
+	[DataRow(2, DisplayName = "position past the last command")]
+	[DataRow(-2, DisplayName = "position before the start")]
+	public void UndoRedoService_RestoreFromStateInvalidPosition_ReturnsFalseAndKeepsHistory(int position)
+	{
+		// Arrange
+		UndoRedoService stack = CreateService();
+		stack.Execute(new DelegateCommand("A", () => { }, () => { }));
+		stack.MarkAsSaved();
+
+		UndoRedoStackState state = new(
+			[new DelegateCommand("X", () => { }, () => { }), new DelegateCommand("Y", () => { }, () => { })],
+			position,
+			[],
+			"1.0",
+			DateTime.UtcNow);
+
+		// Act
+		bool success = stack.RestoreFromState(state);
+
+		// Assert
+		Assert.IsFalse(success, "RestoreFromState should reject a position outside the commands");
+		Assert.AreEqual(1, stack.CommandCount, "A failed restore should keep the existing commands");
+		Assert.AreEqual(0, stack.CurrentPosition, "A failed restore should keep the existing position");
+		Assert.HasCount(1, stack.SaveBoundaries, "A failed restore should keep the existing save boundaries");
+	}
+
+	[TestMethod]
+	public void UndoRedoService_RestoreFromStateNullSaveBoundaries_ReturnsFalseAndKeepsHistory()
+	{
+		// Arrange
+		UndoRedoService stack = CreateService();
+		stack.Execute(new DelegateCommand("A", () => { }, () => { }));
+		stack.MarkAsSaved();
+
+		UndoRedoStackState state = new([], -1, null!, "1.0", DateTime.UtcNow);
+
+		// Act
+		bool success = stack.RestoreFromState(state);
+
+		// Assert
+		Assert.IsFalse(success, "RestoreFromState should reject a state with no save boundaries list");
+		Assert.AreEqual(1, stack.CommandCount, "A failed restore should keep the existing commands");
+		Assert.HasCount(1, stack.SaveBoundaries, "A failed restore should keep the existing save boundaries");
+	}
+
 	private sealed class ConstructorOnlySerializableCommand(string value)
 		: BaseCommand(ChangeType.Modify, ["test"]), ISerializableCommand
 	{

UndoRedo/Services/JsonUndoRedoSerializer.cs

@@ -67,11 +67,13 @@
 		SerializableStackState serializableState = await JsonSerializer.DeserializeAsync<SerializableStackState>(stream, _options, cancellationToken).ConfigureAwait(false)
 			?? throw new InvalidOperationException("Failed to deserialize stack state");
 
-		if (!SupportsVersion(serializableState.FormatVersion))
+		if (serializableState.FormatVersion is null || !SupportsVersion(serializableState.FormatVersion))
 		{
 			throw new NotSupportedException($"Unsupported format version: {serializableState.FormatVersion}");
 		}
 
+		ValidateShape(serializableState);
+
 		List<ICommand> commands = [.. serializableState.Commands.Select(ConvertFromSerializableCommand)];
 		return new UndoRedoStackState(
 			commands,
@@ -81,6 +83,43 @@
 			serializableState.Timestamp);
 	}
 
+	/// <summary>
+	/// Rejects data that parsed as JSON but is missing fields the stack needs, such as a truncated or
+	/// hand-edited file. Throws <see cref="InvalidOperationException"/>, which the deserialization
+	/// contract already covers, so LoadStateAsync reports false instead of letting a
+	/// NullReferenceException or ArgumentNullException escape.
+	/// </summary>
+	private static void ValidateShape(SerializableStackState state)
+	{
+		if (state.Commands is null)
+		{
+			throw new InvalidOperationException("Stack state has no commands list");
+		}
+
+		if (state.SaveBoundaries is null)
+		{
+			throw new InvalidOperationException("Stack state has no save boundaries list");
+		}
+
+		if (state.SaveBoundaries.Any(boundary => boundary is null))
+		{
+			throw new InvalidOperationException("Stack state contains a null save boundary");
+		}
+
+		foreach (SerializableCommand? command in state.Commands)
+		{
+			if (command is null)
+			{
+				throw new InvalidOperationException("Stack state contains a null command");
+			}
+
+			if (command.Metadata is null)
+			{
+				throw new InvalidOperationException($"Command '{command.Description}' has no metadata");
+			}
+		}
+	}
+
 	private static SerializableCommand ConvertToSerializableCommand(ICommand command)
 	{
 		return new SerializableCommand
@@ -124,7 +163,7 @@
 					ex);
 			}
 
 			instance?.DeserializeData(serializableCommand.Data!);
 			return (ICommand)instance!;
 		}
 

UndoRedo/Services/UndoRedoService.cs

@@ -168,7 +168,7 @@
 
 			try
 			{
 				await _navigationProvider.NavigateToAsync(command.NavigationContext!, cts.Token).ConfigureAwait(false);
 			}
 			catch (OperationCanceledException)
 			{
@@ -204,7 +204,7 @@
 
 			try
 			{
 				await _navigationProvider.NavigateToAsync(command.NavigationContext!, cts.Token).ConfigureAwait(false);
 			}
 			catch (OperationCanceledException)
 			{
@@ -270,7 +270,7 @@
 
 			try
 			{
 				await _navigationProvider.NavigateToAsync(lastCommand.NavigationContext!, cts.Token).ConfigureAwait(false);
 			}
 			catch (OperationCanceledException)
 			{
@@ -346,6 +346,13 @@
 	{
 		Ensure.NotNull(state);
 
+		// Validate everything before clearing, so a state that cannot be loaded leaves the current
+		// history untouched instead of wiping it and then failing partway through.
+		if (!IsRestorable(state))
+		{
+			return false;
+		}
+
 		try
 		{
 			_stackManager.Clear();
@@ -379,4 +386,12 @@
 			return false;
 		}
 	}
+
+	private static bool IsRestorable(UndoRedoStackState state) =>
+		state.Commands is not null &&
+		state.SaveBoundaries is not null &&
+		!state.Commands.Any(command => command is null) &&
+		!state.SaveBoundaries.Any(boundary => boundary is null) &&
+		state.CurrentPosition >= -1 &&
+		state.CurrentPosition < state.Commands.Count;
 }