Skip to content

A list group overlay throws on an entry that is not a group #717

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

Open
WhiskyTangoFawks wants to merge 3 commits into
base: dev
Choose a base branch
from
Open

A list group overlay throws on an entry that is not a group #717

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3bc4fe0
A list group overlay throws on an entry that is not a group
WhiskyTangoFawks Oct 12, 2026
d198e53
Test the list group location parse once per game, from its constants
WhiskyTangoFawks Oct 24, 2026
cc0df11
A malformed list group or sub-block throws ModGroupsMalformedException
WhiskyTangoFawks Oct 31, 2026
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
142 changes: 142 additions & 0 deletions Mutagen.Bethesda.Core.UnitTests/Plugins/Binary/ListGroupOverlayTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
using Mutagen.Bethesda.Plugins;
using Mutagen.Bethesda.Plugins.Binary.Overlay;
using Mutagen.Bethesda.Plugins.Binary.Streams;
using Mutagen.Bethesda.Plugins.Binary.Translations;
using Mutagen.Bethesda.Plugins.Exceptions;
using Mutagen.Bethesda.Plugins.Masters;
using Mutagen.Bethesda.Plugins.Meta;
using Mutagen.Bethesda.Plugins.Records;
using Mutagen.Bethesda.Plugins.Records.Internals;
using Mutagen.Bethesda.Testing.AutoData;
using Shouldly;
using Xunit;

namespace Mutagen.Bethesda.UnitTests.Plugins.Binary;

public abstract class AListGroupOverlayTests
{
protected abstract GameRelease Release { get; }

private GameConstants Constants => GameConstants.Get(Release);

private byte[] Written(Action<MutagenWriter> write)
{
var stream = new MemoryStream();
using (var writer = new MutagenWriter(stream, Constants))
{
write(writer);
}
return stream.ToArray();
}

private byte[] ListGroupHolding(Action<MutagenWriter> writeEntry) => Written(writer =>
{
using (HeaderExport.Group(writer, RecordTypes.GRUP))
{
writer.WriteZeros(Constants.GroupConstants.LengthAfterLength);
writeEntry(writer);
}
});

private void WriteGroupShorterThanItsHeader(MutagenWriter writer)
{
writer.Write(RecordTypes.GRUP.TypeInt);
writer.WriteZeros(Constants.GroupConstants.LengthAfterType);
}

private ParsingMeta Meta(ModKey modKey) => new(
Constants,
modKey,
SeparatedMasterPackage.NotSeparate(new MasterReferenceCollection(modKey)));

private IReadOnlyList<IMajorRecordGetter> ParseLocations(byte[] listGroup, ModKey modKey)
{
var meta = Meta(modKey);
var stream = new OverlayStream(listGroup, meta);
stream.Position = Constants.GroupConstants.HeaderLength;
return GroupListOverlay<IMajorRecordGetter>.Factory(
stream,
listGroup,
new BinaryOverlayFactoryPackage(meta),
offset: 0);
}

private IReadOnlyList<int> ParseSubBlockLocations(byte[] subBlocks, ModKey modKey)
{
return PluginBinaryOverlay.ParseRecordLocations(
new OverlayStream(subBlocks, Meta(modKey)),
trigger: RecordTypes.GRUP,
constants: Constants.GroupConstants,
skipHeader: false);
}

[Theory, MutagenAutoData]
public void RecordWhereAGroupBelongs_ThrowsNamingTheRecordType(ModKey modKey)
{
var listGroup = ListGroupHolding(writer =>
{
using (HeaderExport.Record(writer, RecordTypes.MISC))
{
writer.WriteZeros(Constants.MajorConstants.LengthAfterLength);
}
});

Should.Throw<ModGroupsMalformedException>(() => ParseLocations(listGroup, modKey))
.Message.ShouldContain(RecordTypes.MISC.Type);
}

[Theory, MutagenAutoData]
public void GroupShorterThanItsHeader_ThrowsAsMalformed(ModKey modKey)
{
var listGroup = ListGroupHolding(WriteGroupShorterThanItsHeader);

Should.Throw<ModGroupsMalformedException>(() => ParseLocations(listGroup, modKey));
}

[Theory, MutagenAutoData]
public void SubBlockShorterThanItsHeader_ThrowsAsMalformed(ModKey modKey)
{
var subBlocks = Written(WriteGroupShorterThanItsHeader);

Should.Throw<ModGroupsMalformedException>(() => ParseSubBlockLocations(subBlocks, modKey));
}

[Theory, MutagenAutoData]
public void EmptyGroup_ReadsAsOneEntry(ModKey modKey)
{
var listGroup = ListGroupHolding(writer =>
{
using (HeaderExport.Group(writer, RecordTypes.GRUP))
{
writer.WriteZeros(Constants.GroupConstants.LengthAfterLength);
}
});

ParseLocations(listGroup, modKey).Count.ShouldBe(1);
}
}

public class OblivionListGroupOverlayTests : AListGroupOverlayTests
{
protected override GameRelease Release => GameRelease.Oblivion;
}

public class Fallout3ListGroupOverlayTests : AListGroupOverlayTests
{
protected override GameRelease Release => GameRelease.Fallout3;
}

public class SkyrimListGroupOverlayTests : AListGroupOverlayTests
{
protected override GameRelease Release => GameRelease.SkyrimSE;
}

public class Fallout4ListGroupOverlayTests : AListGroupOverlayTests
{
protected override GameRelease Release => GameRelease.Fallout4;
}

public class StarfieldListGroupOverlayTests : AListGroupOverlayTests
{
protected override GameRelease Release => GameRelease.Starfield;
}
17 changes: 13 additions & 4 deletions Mutagen.Bethesda.Core/Plugins/Binary/Overlay/AListGroupOverlay.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
using Mutagen.Bethesda.Plugins.Binary.Headers;
using Mutagen.Bethesda.Plugins.Binary.Translations;
using Mutagen.Bethesda.Plugins.Exceptions;
using Mutagen.Bethesda.Plugins.Internals;
using Noggog;
using System.Buffers.Binary;
using System.Collections;
using System.IO.Compression;
using Loqui;
using Mutagen.Bethesda.Plugins.Meta;
using Mutagen.Bethesda.Plugins.Records;
using Mutagen.Bethesda.Plugins.Assets;

Expand Down Expand Up @@ -61,7 +61,6 @@ public static GroupListOverlay<T> Factory(
IBinaryReadStream stream,
ReadOnlyMemorySlice<byte> data,
BinaryOverlayFactoryPackage package,
ObjectType objectType,
int offset)
{
List<int> locations = new List<int>();
Expand All @@ -73,9 +72,19 @@ public static GroupListOverlay<T> Factory(
// Parse locations
while (stream.Position < finalPos)
{
VariableHeader meta = package.MetaData.Constants.VariableHeader(stream.RemainingMemory, objectType);
var groupHeader = package.MetaData.Constants.GroupHeader(stream.RemainingMemory);
if (!groupHeader.IsGroup)
{
var majorMeta = package.MetaData.Constants.MajorRecordHeader(stream.RemainingMemory);
var formKey = FormKey.Factory(package.MetaData.MasterReferences, majorMeta.FormID, reference: false);
throw new ModGroupsMalformedException(package.MetaData.ModKey, $"Unexpected type encountered when parsing group locations: {majorMeta.RecordType} {formKey}");
}
if (groupHeader.TotalLength < groupHeader.HeaderLength)
{
throw new ModGroupsMalformedException(package.MetaData.ModKey, $"Group length {groupHeader.TotalLength} is shorter than its header");
}
locations.Add(checked((int)stream.Position - offset));
stream.Position += checked((int)meta.TotalLength);
stream.Position += checked((int)groupHeader.TotalLength);
}

return new GroupListOverlay<T>(
Expand Down
4 changes: 4 additions & 0 deletions Mutagen.Bethesda.Core/Plugins/Binary/Overlay/PluginBinaryOverlay.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,10 @@ public static IReadOnlyList<int> ParseRecordLocations(
var varMeta = stream.MetaData.Constants.GetVariableHeader(stream, constants);
var recType = translationParams.ConvertToStandard(varMeta.RecordType);
if (recType != trigger) break;
if (varMeta.TotalLength < varMeta.HeaderLength)
{
throw new ModGroupsMalformedException(stream.MetaData.ModKey, $"Group length {varMeta.TotalLength} is shorter than its header");
}
if (skipHeader)
{
stream.Position += varMeta.HeaderLength;
Expand Down
4 changes: 1 addition & 3 deletions Mutagen.Bethesda.Fallout3/Records/Fallout3ListGroup.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
using Mutagen.Bethesda.Plugins.Binary.Overlay;
using Mutagen.Bethesda.Plugins.Binary.Streams;
using Mutagen.Bethesda.Plugins.Binary.Translations;
using Mutagen.Bethesda.Plugins.Meta;
using Mutagen.Bethesda.Plugins.Records;
using Mutagen.Bethesda.Translations.Binary;
using Noggog;
Expand Down Expand Up @@ -63,8 +62,7 @@ partial void CustomFactoryEnd(
stream,
_recordData,
_package,
offset: offset,
objectType: ObjectType.Group);
offset: offset);
}

public IEnumerator<T> GetEnumerator() => Records.GetEnumerator();
Expand Down
4 changes: 1 addition & 3 deletions Mutagen.Bethesda.Fallout4/Records/Fallout4ListGroup.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
using Mutagen.Bethesda.Plugins.Binary.Overlay;
using Mutagen.Bethesda.Plugins.Binary.Streams;
using Mutagen.Bethesda.Plugins.Binary.Translations;
using Mutagen.Bethesda.Plugins.Meta;
using Mutagen.Bethesda.Plugins.Records;
using Mutagen.Bethesda.Translations.Binary;
using Noggog;
Expand Down Expand Up @@ -63,8 +62,7 @@ partial void CustomFactoryEnd(
stream,
_recordData,
_package,
offset: offset,
objectType: ObjectType.Group);
offset: offset);
}

public IEnumerator<T> GetEnumerator() => Records.GetEnumerator();
Expand Down
4 changes: 1 addition & 3 deletions Mutagen.Bethesda.Oblivion/Records/OblivionListGroup.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
using Mutagen.Bethesda.Plugins.Binary.Overlay;
using Mutagen.Bethesda.Plugins.Binary.Streams;
using Mutagen.Bethesda.Plugins.Binary.Translations;
using Mutagen.Bethesda.Plugins.Meta;
using Mutagen.Bethesda.Plugins.Records;
using Mutagen.Bethesda.Translations.Binary;
using Noggog;
Expand Down Expand Up @@ -63,8 +62,7 @@ partial void CustomFactoryEnd(
stream,
_recordData,
_package,
offset: offset,
objectType: ObjectType.Group);
offset: offset);
}

public IEnumerator<T> GetEnumerator() => Records.GetEnumerator();
Expand Down
4 changes: 1 addition & 3 deletions Mutagen.Bethesda.Skyrim/Records/ListGroup.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
using Mutagen.Bethesda.Plugins.Binary.Overlay;
using Mutagen.Bethesda.Plugins.Binary.Streams;
using Mutagen.Bethesda.Plugins.Binary.Translations;
using Mutagen.Bethesda.Plugins.Meta;
using Mutagen.Bethesda.Plugins.Records;
using Mutagen.Bethesda.Translations.Binary;
using Noggog;
Expand Down Expand Up @@ -64,8 +63,7 @@ partial void CustomFactoryEnd(
stream,
_recordData,
_package,
offset: offset,
objectType: ObjectType.Group);
offset: offset);
}

public IEnumerator<T> GetEnumerator() => Records.GetEnumerator();
Expand Down
4 changes: 1 addition & 3 deletions Mutagen.Bethesda.Starfield/Records/StarfieldListGroup.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
using Mutagen.Bethesda.Plugins.Binary.Overlay;
using Mutagen.Bethesda.Plugins.Binary.Streams;
using Mutagen.Bethesda.Plugins.Binary.Translations;
using Mutagen.Bethesda.Plugins.Meta;
using Mutagen.Bethesda.Plugins.Records;
using Mutagen.Bethesda.Translations.Binary;
using Noggog;
Expand Down Expand Up @@ -63,8 +62,7 @@ partial void CustomFactoryEnd(
stream,
_recordData,
_package,
offset: offset,
objectType: ObjectType.Group);
offset: offset);
}

public IEnumerator<T> GetEnumerator() => Records.GetEnumerator();
Expand Down
Loading