Skip to content

Validate update control ranges in ApplyChanges #29

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
philliphoff merged 1 commit into from
Oct 11, 2026
Merged

Validate update control ranges in ApplyChanges #29

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 84 additions & 15 deletions src/EncDotNet.S57/S57Document.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,17 @@ public IEnumerable<S57FeatureRecord> GetFeaturesByObjectCode(S57ObjectCode objec
/// (FSPC, FFPC, VRPC, SGCC) when present.</description></item>
/// </list>
/// </para>
/// <para>
/// 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.
/// </para>
/// </remarks>
/// <exception cref="InvalidDataException">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.</exception>
public S57Document ApplyChanges(S57Document update)
{
var features = ApplyRecordChanges(
Expand Down Expand Up @@ -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),
};
}

Expand All @@ -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),
};
}

Expand Down Expand Up @@ -262,7 +272,9 @@ private static IReadOnlyList<S57AttributeValue> MergeAttributes(
private static ImmutableArray<T> ApplyArrayUpdate<T>(
ImmutableArray<T> baseArray,
ImmutableArray<T> updateData,
S57FieldUpdateControl? control)
S57FieldUpdateControl? control,
S57RecordName recordName,
string controlFieldTag)
where T : struct
{
if (control is null)
Expand All @@ -275,47 +287,104 @@ private static ImmutableArray<T> ApplyArrayUpdate<T>(
// 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<T> SpliceInsert<T>(ImmutableArray<T> baseArray, ImmutableArray<T> items, int index)
private static ImmutableArray<T> SpliceInsert<T>(
ImmutableArray<T> baseArray,
ImmutableArray<T> items,
int index,
S57FieldUpdateControl ctrl,
S57RecordName recordName,
string controlFieldTag)
where T : struct
{
if (items.IsDefaultOrEmpty)
{
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<T> SpliceDelete<T>(ImmutableArray<T> baseArray, int index, int count)
private static ImmutableArray<T> SpliceDelete<T>(
ImmutableArray<T> 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<T> SpliceModify<T>(ImmutableArray<T> baseArray, ImmutableArray<T> items, int index, int count)
private static ImmutableArray<T> SpliceModify<T>(
ImmutableArray<T> baseArray,
ImmutableArray<T> items,
int index,
S57FieldUpdateControl ctrl,
S57RecordName recordName,
string controlFieldTag)
where T : struct
{
if (items.IsDefaultOrEmpty)
{
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.");
}
}
216 changes: 216 additions & 0 deletions tests/EncDotNet.UnitTests/S57DocumentApplyChangesTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<S57FeatureRecord>.Empty,
VectorRecords =
[
CreateVector(1, coordinates: [Coord(10, 20), Coord(30, 40), Coord(50, 60)])
]
};
var update = new S57Document
{
FeatureRecords = ImmutableArray<S57FeatureRecord>.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<InvalidDataException>(() => 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<S57VectorRecord>.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<S57VectorRecord>.Empty
};

var ex = Assert.Throws<InvalidDataException>(() => 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<S57VectorRecord>.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<S57VectorRecord>.Empty
};

var ex = Assert.Throws<InvalidDataException>(() => 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<S57FeatureRecord>.Empty,
VectorRecords = [CreateVector(10, vectorPointers: [ptr])]
};
var update = new S57Document
{
FeatureRecords = ImmutableArray<S57FeatureRecord>.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<InvalidDataException>(() => baseDoc.ApplyChanges(update));

Assert.Contains(S57FieldTags.VRPC, ex.Message);
}

[Fact]
public void ApplyChanges_SgccModifyCoveringLastCoordinate_Succeeds()
{
var baseDoc = new S57Document
{
FeatureRecords = ImmutableArray<S57FeatureRecord>.Empty,
VectorRecords =
[
CreateVector(1, coordinates: [Coord(10, 20), Coord(30, 40), Coord(50, 60)])
]
};
var update = new S57Document
{
FeatureRecords = ImmutableArray<S57FeatureRecord>.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<S57VectorRecord>.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<S57VectorRecord>.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]
Expand Down
Loading