From f5eca290af2a48c39a0c0bde052fc3474b5316dc Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 26 Sep 2026 19:25:36 +0000 Subject: [PATCH] fix: reject saved state with save boundaries outside the commands [patch] IsRestorable checked that save boundaries were non-null but never checked their positions, so a corrupt state with boundaries at -7 or 42 loaded and left phantom save points behind. Require every boundary to sit between -1 and the last command, so RestoreFromState and LoadStateAsync reject the state and keep the live history. Fixes #90 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01N4m2HJkrsY88XBtTqTKAkE --- UndoRedo.Test/SerializationTests.cs | 78 ++++++++++++++++++++++++++++ UndoRedo/Services/UndoRedoService.cs | 1 + 2 files changed, 79 insertions(+) diff --git a/UndoRedo.Test/SerializationTests.cs b/UndoRedo.Test/SerializationTests.cs index 7d8b1c5..acd01ff 100644 --- a/UndoRedo.Test/SerializationTests.cs +++ b/UndoRedo.Test/SerializationTests.cs @@ -467,6 +467,84 @@ public void UndoRedoService_RestoreFromStateNullSaveBoundaries_ReturnsFalseAndKe Assert.HasCount(1, stack.SaveBoundaries, "A failed restore should keep the existing save boundaries"); } + [TestMethod] + [DataRow(-7, DisplayName = "boundary before the start")] + [DataRow(-2, DisplayName = "boundary one before the start")] + [DataRow(2, DisplayName = "boundary at the command count")] + [DataRow(42, DisplayName = "boundary far past the last command")] + public void UndoRedoService_RestoreFromStateInvalidBoundaryPosition_ReturnsFalseAndKeepsHistory(int boundaryPosition) + { + // Arrange + UndoRedoService stack = CreateService(); + stack.Execute(new DelegateCommand("A", () => { }, () => { })); + stack.MarkAsSaved(); + + UndoRedoStackState state = new( + [new DelegateCommand("X", () => { }, () => { }), new DelegateCommand("Y", () => { }, () => { })], + 1, + [new SaveBoundary(boundaryPosition)], + "1.0", + DateTime.UtcNow); + + // Act + bool success = stack.RestoreFromState(state); + + // Assert + Assert.IsFalse(success, "RestoreFromState should reject a save boundary 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"); + Assert.AreEqual(0, stack.SaveBoundaries[0].Position, "A failed restore should keep the existing save boundary position"); + } + + [TestMethod] + [DataRow(-1, DisplayName = "boundary at the initial position")] + [DataRow(1, DisplayName = "boundary at the last command")] + public void UndoRedoService_RestoreFromStateBoundaryAtEdge_Restores(int boundaryPosition) + { + // Arrange + UndoRedoService stack = CreateService(); + UndoRedoStackState state = new( + [new DelegateCommand("X", () => { }, () => { }), new DelegateCommand("Y", () => { }, () => { })], + 1, + [new SaveBoundary(boundaryPosition)], + "1.0", + DateTime.UtcNow); + + // Act + bool success = stack.RestoreFromState(state); + + // Assert + Assert.IsTrue(success, "RestoreFromState should accept a save boundary at -1 or at the last command"); + Assert.HasCount(1, stack.SaveBoundaries); + Assert.AreEqual(boundaryPosition, stack.SaveBoundaries[0].Position); + } + + [TestMethod] + public async Task UndoRedoService_LoadStateAsyncInvalidBoundaryPosition_ReturnsFalseAndKeepsHistory() + { + // Arrange + JsonUndoRedoSerializer serializer = new(); + byte[] data = await serializer.SerializeAsync( + [new TestSerializableCommand("X")], + 0, + [new SaveBoundary(-7), new SaveBoundary(42)]).ConfigureAwait(false); + + UndoRedoService stack = CreateService(); + stack.SetSerializer(new JsonUndoRedoSerializer()); + stack.Execute(new DelegateCommand("A", () => { }, () => { })); + stack.MarkAsSaved(); + + // Act + bool success = await stack.LoadStateAsync(data).ConfigureAwait(false); + + // Assert + Assert.IsFalse(success, "LoadStateAsync should reject save boundaries outside the commands"); + Assert.AreEqual(1, stack.CommandCount, "A failed load should keep the existing commands"); + Assert.HasCount(1, stack.SaveBoundaries, "A failed load should keep the existing save boundaries"); + Assert.AreEqual(0, stack.SaveBoundaries[0].Position, "A failed load should keep the existing save boundary position"); + } + private sealed class ConstructorOnlySerializableCommand(string value) : BaseCommand(ChangeType.Modify, ["test"]), ISerializableCommand { diff --git a/UndoRedo/Services/UndoRedoService.cs b/UndoRedo/Services/UndoRedoService.cs index 8a76cc7..f1f59ae 100644 --- a/UndoRedo/Services/UndoRedoService.cs +++ b/UndoRedo/Services/UndoRedoService.cs @@ -396,6 +396,7 @@ 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.SaveBoundaries.All(boundary => boundary.Position >= -1 && boundary.Position < state.Commands.Count) && state.CurrentPosition >= -1 && state.CurrentPosition < state.Commands.Count; }