diff --git a/src/EncDotNet.S57/S57Document.cs b/src/EncDotNet.S57/S57Document.cs index 69bb4f3..731a6b8 100644 --- a/src/EncDotNet.S57/S57Document.cs +++ b/src/EncDotNet.S57/S57Document.cs @@ -83,7 +83,17 @@ public IEnumerable GetFeaturesByObjectCode(S57ObjectCode objec /// (FSPC, FFPC, VRPC, SGCC) when present. /// /// + /// + /// A control field's 1-based index and count must address entries that exist in the + /// base record's array (S-57 Part 3, 8.4.3). An insert may target any position from 1 + /// to one past the last entry; a delete or modify must lie entirely within the array. + /// A control field that falls outside these bounds means the update does not match + /// the base cell, and no partial result is returned. + /// /// + /// An update control field (FSPC, FFPC, VRPC or SGCC) + /// has an index or count outside the bounds of the base record's array. The message names + /// the record (RCNM/RCID) and the control field. public S57Document ApplyChanges(S57Document update) { var features = ApplyRecordChanges( @@ -186,8 +196,8 @@ private static S57FeatureRecord ApplyFeatureModify(S57FeatureRecord baseRecord, UpdateInstruction = updateRecord.UpdateInstruction, Attributes = MergeAttributes(baseRecord.Attributes, updateRecord.Attributes), NationalAttributes = MergeAttributes(baseRecord.NationalAttributes, updateRecord.NationalAttributes), - SpatialPointers = ApplyArrayUpdate([.. baseRecord.SpatialPointers], [.. updateRecord.SpatialPointers], updateRecord.SpatialPointerControl), - FeaturePointers = ApplyArrayUpdate([.. baseRecord.FeaturePointers], [.. updateRecord.FeaturePointers], updateRecord.FeaturePointerControl), + SpatialPointers = ApplyArrayUpdate([.. baseRecord.SpatialPointers], [.. updateRecord.SpatialPointers], updateRecord.SpatialPointerControl, baseRecord.RecordName, S57FieldTags.FSPC), + FeaturePointers = ApplyArrayUpdate([.. baseRecord.FeaturePointers], [.. updateRecord.FeaturePointers], updateRecord.FeaturePointerControl, baseRecord.RecordName, S57FieldTags.FFPC), }; } @@ -199,9 +209,9 @@ private static S57VectorRecord ApplyVectorModify(S57VectorRecord baseRecord, S57 RecordVersion = updateRecord.RecordVersion, UpdateInstruction = updateRecord.UpdateInstruction, Attributes = MergeAttributes(baseRecord.Attributes, updateRecord.Attributes), - VectorPointers = ApplyArrayUpdate([.. baseRecord.VectorPointers], [.. updateRecord.VectorPointers], updateRecord.VectorPointerControl), - Coordinates2D = ApplyArrayUpdate([.. baseRecord.Coordinates2D], [.. updateRecord.Coordinates2D], updateRecord.CoordinateControl), - Soundings = ApplyArrayUpdate([.. baseRecord.Soundings], [.. updateRecord.Soundings], updateRecord.CoordinateControl), + VectorPointers = ApplyArrayUpdate([.. baseRecord.VectorPointers], [.. updateRecord.VectorPointers], updateRecord.VectorPointerControl, baseRecord.RecordName, S57FieldTags.VRPC), + Coordinates2D = ApplyArrayUpdate([.. baseRecord.Coordinates2D], [.. updateRecord.Coordinates2D], updateRecord.CoordinateControl, baseRecord.RecordName, S57FieldTags.SGCC), + Soundings = ApplyArrayUpdate([.. baseRecord.Soundings], [.. updateRecord.Soundings], updateRecord.CoordinateControl, baseRecord.RecordName, S57FieldTags.SGCC), }; } @@ -262,7 +272,9 @@ private static IReadOnlyList MergeAttributes( private static ImmutableArray ApplyArrayUpdate( ImmutableArray baseArray, ImmutableArray updateData, - S57FieldUpdateControl? control) + S57FieldUpdateControl? control, + S57RecordName recordName, + string controlFieldTag) where T : struct { if (control is null) @@ -275,16 +287,25 @@ private static ImmutableArray ApplyArrayUpdate( // Convert from 1-based S-57 index to 0-based. int index = ctrl.Index - 1; + // SGCC controls both SG2D and SG3D, so the splice helpers leave an array untouched when + // the update carries no data for it (or, for a delete, the base has none) before + // checking the range against it. return ctrl.UpdateInstruction switch { - S57UpdateInstruction.Insert => SpliceInsert(baseArray, updateData, index), - S57UpdateInstruction.Delete => SpliceDelete(baseArray, index, ctrl.Count), - S57UpdateInstruction.Modify => SpliceModify(baseArray, updateData, index, ctrl.Count), + S57UpdateInstruction.Insert => SpliceInsert(baseArray, updateData, index, ctrl, recordName, controlFieldTag), + S57UpdateInstruction.Delete => SpliceDelete(baseArray, index, ctrl, recordName, controlFieldTag), + S57UpdateInstruction.Modify => SpliceModify(baseArray, updateData, index, ctrl, recordName, controlFieldTag), _ => baseArray }; } - private static ImmutableArray SpliceInsert(ImmutableArray baseArray, ImmutableArray items, int index) + private static ImmutableArray SpliceInsert( + ImmutableArray baseArray, + ImmutableArray items, + int index, + S57FieldUpdateControl ctrl, + S57RecordName recordName, + string controlFieldTag) where T : struct { if (items.IsDefaultOrEmpty) @@ -292,21 +313,40 @@ private static ImmutableArray SpliceInsert(ImmutableArray baseArray, Im return baseArray; } + // An insert may target any existing position or one past the end (an append). + if (index < 0 || index > baseArray.Length) + { + throw OutOfRange(ctrl, baseArray.Length, recordName, controlFieldTag); + } + return baseArray.InsertRange(index, items); } - private static ImmutableArray SpliceDelete(ImmutableArray baseArray, int index, int count) + private static ImmutableArray SpliceDelete( + ImmutableArray baseArray, + int index, + S57FieldUpdateControl ctrl, + S57RecordName recordName, + string controlFieldTag) where T : struct { - if (count <= 0 || baseArray.IsDefaultOrEmpty) + if (ctrl.Count <= 0 || baseArray.IsDefaultOrEmpty) { return baseArray; } - return baseArray.RemoveRange(index, count); + EnsureRangeInBounds(baseArray.Length, index, ctrl, recordName, controlFieldTag); + + return baseArray.RemoveRange(index, ctrl.Count); } - private static ImmutableArray SpliceModify(ImmutableArray baseArray, ImmutableArray items, int index, int count) + private static ImmutableArray SpliceModify( + ImmutableArray baseArray, + ImmutableArray items, + int index, + S57FieldUpdateControl ctrl, + S57RecordName recordName, + string controlFieldTag) where T : struct { if (items.IsDefaultOrEmpty) @@ -314,8 +354,37 @@ private static ImmutableArray SpliceModify(ImmutableArray baseArray, Im return baseArray; } + EnsureRangeInBounds(baseArray.Length, index, ctrl, recordName, controlFieldTag); + // Modify = delete the existing range, then insert the new items at the same position. - var withRemoved = baseArray.RemoveRange(index, count); + var withRemoved = baseArray.RemoveRange(index, ctrl.Count); return withRemoved.InsertRange(index, items); } + + private static void EnsureRangeInBounds( + int length, + int index, + S57FieldUpdateControl ctrl, + S57RecordName recordName, + string controlFieldTag) + { + // Written as a subtraction so a large count cannot overflow index + count. + if (index < 0 || index > length || ctrl.Count < 0 || ctrl.Count > length - index) + { + throw OutOfRange(ctrl, length, recordName, controlFieldTag); + } + } + + private static InvalidDataException OutOfRange( + S57FieldUpdateControl ctrl, + int length, + S57RecordName recordName, + string controlFieldTag) + { + return new InvalidDataException( + $"Update control field {controlFieldTag} on record {recordName} " + + $"({ctrl.UpdateInstruction} at index {ctrl.Index}, count {ctrl.Count}) is outside " + + $"the base record's {length} entries (S-57 Part 3, 8.4.3). " + + "The update does not match the base cell."); + } } diff --git a/tests/EncDotNet.UnitTests/S57DocumentApplyChangesTests.cs b/tests/EncDotNet.UnitTests/S57DocumentApplyChangesTests.cs index b90a2c1..6c4a027 100644 --- a/tests/EncDotNet.UnitTests/S57DocumentApplyChangesTests.cs +++ b/tests/EncDotNet.UnitTests/S57DocumentApplyChangesTests.cs @@ -908,6 +908,222 @@ public void ApplyChanges_VrpcDelete_RemovesVectorPointersAtIndex() #endregion + #region Out-of-Range Update Controls + + [Fact] + public void ApplyChanges_SgccModifyBeyondBaseCoordinates_ThrowsNamingRecordAndField() + { + var baseDoc = new S57Document + { + FeatureRecords = ImmutableArray.Empty, + VectorRecords = + [ + CreateVector(1, coordinates: [Coord(10, 20), Coord(30, 40), Coord(50, 60)]) + ] + }; + var update = new S57Document + { + FeatureRecords = ImmutableArray.Empty, + VectorRecords = + [ + CreateVector(1, ruin: S57UpdateInstruction.Modify, + coordinateControl: new S57FieldUpdateControl + { + UpdateInstruction = S57UpdateInstruction.Modify, + Index = 2, + Count = 5 // only 2 coordinates from index 2 onward + }, + coordinates: [Coord(999, 888)]) + ] + }; + + var ex = Assert.Throws(() => baseDoc.ApplyChanges(update)); + + Assert.Contains(S57FieldTags.SGCC, ex.Message); + Assert.Contains($"RCNM={S57RecordNameCodes.Edge}, RCID=1", ex.Message); + } + + [Fact] + public void ApplyChanges_FspcDeleteBeyondBasePointers_ThrowsNamingRecordAndField() + { + var baseDoc = new S57Document + { + FeatureRecords = + [ + CreateFeature(7, + spatialPointers: + [ + SpatialPtr(S57RecordNameCodes.Edge, 1), + SpatialPtr(S57RecordNameCodes.Edge, 2) + ]) + ], + VectorRecords = ImmutableArray.Empty + }; + var update = new S57Document + { + FeatureRecords = + [ + CreateFeature(7, ruin: S57UpdateInstruction.Modify, + spatialPointerControl: new S57FieldUpdateControl + { + UpdateInstruction = S57UpdateInstruction.Delete, + Index = 3, // past the last of 2 pointers + Count = 1 + }) + ], + VectorRecords = ImmutableArray.Empty + }; + + var ex = Assert.Throws(() => baseDoc.ApplyChanges(update)); + + Assert.Contains(S57FieldTags.FSPC, ex.Message); + Assert.Contains($"RCNM={S57RecordNameCodes.Feature}, RCID=7", ex.Message); + } + + [Fact] + public void ApplyChanges_FfpcInsertBeyondAppendPosition_ThrowsNamingRecordAndField() + { + var basePtr = new S57FeaturePointer + { + Name = S57RecordName.FromRcnmRcid(S57RecordNameCodes.Feature, 100), + Relationship = S57RelationshipIndicator.Master, + Comment = "" + }; + var newPtr = basePtr with { Name = S57RecordName.FromRcnmRcid(S57RecordNameCodes.Feature, 200) }; + var baseDoc = new S57Document + { + FeatureRecords = [CreateFeature(1, featurePointers: [basePtr])], + VectorRecords = ImmutableArray.Empty + }; + var update = new S57Document + { + FeatureRecords = + [ + CreateFeature(1, ruin: S57UpdateInstruction.Modify, + featurePointerControl: new S57FieldUpdateControl + { + UpdateInstruction = S57UpdateInstruction.Insert, + Index = 3, // 1 pointer, so 2 (append) is the furthest valid position + Count = 1 + }, + featurePointers: [newPtr]) + ], + VectorRecords = ImmutableArray.Empty + }; + + var ex = Assert.Throws(() => baseDoc.ApplyChanges(update)); + + Assert.Contains(S57FieldTags.FFPC, ex.Message); + Assert.Contains($"RCNM={S57RecordNameCodes.Feature}, RCID=1", ex.Message); + } + + [Fact] + public void ApplyChanges_VrpcIndexZero_Throws() + { + var ptr = new S57VectorPointer + { + Name = S57RecordName.FromRcnmRcid(S57RecordNameCodes.ConnectedNode, 1), + Orientation = S57Orientation.Forward, + Usage = S57UsageIndicator.NotApplicable, + Topology = S57TopologyIndicator.Beginning, + Mask = S57MaskingIndicator.NotApplicable + }; + var baseDoc = new S57Document + { + FeatureRecords = ImmutableArray.Empty, + VectorRecords = [CreateVector(10, vectorPointers: [ptr])] + }; + var update = new S57Document + { + FeatureRecords = ImmutableArray.Empty, + VectorRecords = + [ + CreateVector(10, ruin: S57UpdateInstruction.Modify, + vectorPointerControl: new S57FieldUpdateControl + { + UpdateInstruction = S57UpdateInstruction.Delete, + Index = 0, // indices are 1-based + Count = 1 + }) + ] + }; + + var ex = Assert.Throws(() => baseDoc.ApplyChanges(update)); + + Assert.Contains(S57FieldTags.VRPC, ex.Message); + } + + [Fact] + public void ApplyChanges_SgccModifyCoveringLastCoordinate_Succeeds() + { + var baseDoc = new S57Document + { + FeatureRecords = ImmutableArray.Empty, + VectorRecords = + [ + CreateVector(1, coordinates: [Coord(10, 20), Coord(30, 40), Coord(50, 60)]) + ] + }; + var update = new S57Document + { + FeatureRecords = ImmutableArray.Empty, + VectorRecords = + [ + CreateVector(1, ruin: S57UpdateInstruction.Modify, + coordinateControl: new S57FieldUpdateControl + { + UpdateInstruction = S57UpdateInstruction.Modify, + Index = 2, + Count = 2 + }, + coordinates: [Coord(777, 0), Coord(888, 0)]) + ] + }; + + var result = baseDoc.ApplyChanges(update); + + var coords = result.VectorRecords[0].Coordinates2D; + Assert.Equal([10, 777, 888], coords.Select(c => c.X)); + } + + [Fact] + public void ApplyChanges_FfpcInsertAtAppendPosition_Succeeds() + { + var basePtr = new S57FeaturePointer + { + Name = S57RecordName.FromRcnmRcid(S57RecordNameCodes.Feature, 100), + Relationship = S57RelationshipIndicator.Master, + Comment = "" + }; + var newPtr = basePtr with { Name = S57RecordName.FromRcnmRcid(S57RecordNameCodes.Feature, 200) }; + var baseDoc = new S57Document + { + FeatureRecords = [CreateFeature(1, featurePointers: [basePtr])], + VectorRecords = ImmutableArray.Empty + }; + var update = new S57Document + { + FeatureRecords = + [ + CreateFeature(1, ruin: S57UpdateInstruction.Modify, + featurePointerControl: new S57FieldUpdateControl + { + UpdateInstruction = S57UpdateInstruction.Insert, + Index = 2, + Count = 1 + }, + featurePointers: [newPtr]) + ], + VectorRecords = ImmutableArray.Empty + }; + + var result = baseDoc.ApplyChanges(update); + + Assert.Equal([100, 200], result.FeatureRecords[0].FeaturePointers.Select(p => p.Name.RecordId)); + } + + #endregion + #region Empty Update Document [Fact]