Preserve resource entry order based on ID

docs/guides/peimage/win32resources.md

@@ -43,9 +43,10 @@ IResourceData stringDataEntry = image.Resources
 ```
 
 Adding or replacing entries can be by either modifying the `Entries`
-property directly, or by using the `AddOrReplace` method. The latter is
-recommended as it ensures that an existing entry with the same ID is
-replaced with the new one.
+property directly, or by using the `InsertOrReplaceEntry` method.
+The latter is recommended as it ensures that an existing entry with the
+same ID is replaced with the new one, and that the sorting requirements
+according to the PE file specification are presrved.
 
 ``` csharp
 IPEImage image = ...
@@ -56,7 +57,7 @@ image.Resources.Entries.Add(newDirectory);
 ``` csharp
 IPEImage image = ...
 var newDirectory = new ResourceDirectory(ResourceType.String);
-image.Resources.AddOrReplaceEntry(newDirectory);
+image.Resources.InsertOrReplaceEntry(newDirectory);
 ```
 
 Similarly, removing can be done by modifying the `Entries` directory, or
@@ -99,7 +100,7 @@ var data = new ResourceData(id: 1033, contents: new DataSegment(new byte[] { ...
 image.Resources
     .GetDirectory(ResourceType.String)
     .GetDirectory(251)
-    .AddOrReplaceEntry(data);
+    .InsertOrReplaceEntry(data);
 ```
 
 ## Example Traversal

src/AsmResolver.PE.Win32Resources/IWin32Resource.cs

@@ -2,14 +2,14 @@ namespace AsmResolver.PE.Win32Resources
 {
     /// <summary>
     /// Provides a high level view of a native win32 resource that can be stored in the resources data directory of a
-    /// portable executable (PE) image. 
+    /// portable executable (PE) image.
     /// </summary>
     public interface IWin32Resource
     {
         /// <summary>
         /// Serializes the win32 resource to the provided root win32 resource data directory.
         /// </summary>
         /// <param name="rootDirectory">The root directory to submit the changes to.</param>
-        void WriteToDirectory(IResourceDirectory rootDirectory);
+        void InsertIntoDirectory(IResourceDirectory rootDirectory);
     }
-}
+}

src/AsmResolver.PE.Win32Resources/Icon/IconResource.cs

@@ -81,13 +81,13 @@ public IconGroupDirectory this[uint id]
         public IEnumerable<IconGroupDirectory> GetIconGroups() => _entries.Values;
 
         /// <inheritdoc />
-        public void WriteToDirectory(IResourceDirectory rootDirectory)
+        public void InsertIntoDirectory(IResourceDirectory rootDirectory)
         {
             // Construct new directory.
             var newGroupIconDirectory = new ResourceDirectory(ResourceType.GroupIcon);
             foreach (var entry in _entries)
             {
-                newGroupIconDirectory.Entries.Add(new ResourceDirectory(entry.Key)
+                newGroupIconDirectory.InsertOrReplaceEntry(new ResourceDirectory(entry.Key)
                     {Entries = {new ResourceData(0u, entry.Value)}});
             }
 
@@ -97,14 +97,14 @@ public void WriteToDirectory(IResourceDirectory rootDirectory)
             {
                 foreach (var (groupEntry, iconEntry) in entry.Value.GetIconEntries())
                 {
-                    newIconDirectory.Entries.Add(new ResourceDirectory(groupEntry.Id)
+                    newIconDirectory.InsertOrReplaceEntry(new ResourceDirectory(groupEntry.Id)
                         {Entries = {new ResourceData(1033, iconEntry)}});
                 }
             }
 
             // Insert.
-            rootDirectory.AddOrReplaceEntry(newGroupIconDirectory);
-            rootDirectory.AddOrReplaceEntry(newIconDirectory);
+            rootDirectory.InsertOrReplaceEntry(newGroupIconDirectory);
+            rootDirectory.InsertOrReplaceEntry(newIconDirectory);
         }
     }
 }

src/AsmResolver.PE.Win32Resources/Version/VersionInfoResource.cs

@@ -271,24 +271,24 @@ protected override void WriteValue(IBinaryStreamWriter writer)
         }
 
         /// <inheritdoc />
-        public void WriteToDirectory(IResourceDirectory rootDirectory)
+        public void InsertIntoDirectory(IResourceDirectory rootDirectory)
         {
             // Add version directory if it doesn't exist yet.
             if (!rootDirectory.TryGetDirectory(ResourceType.Version, out var versionDirectory))
             {
                 versionDirectory = new ResourceDirectory(ResourceType.Version);
-                rootDirectory.Entries.Add(versionDirectory);
+                rootDirectory.InsertOrReplaceEntry(versionDirectory);
             }
 
             // Add category directory if it doesn't exist yet.
             if (!versionDirectory.TryGetDirectory(1, out var categoryDirectory))
             {
                 categoryDirectory = new ResourceDirectory(1);
-                versionDirectory.Entries.Add(categoryDirectory);
+                versionDirectory.InsertOrReplaceEntry(categoryDirectory);
             }
 
             // Insert / replace data entry.
-            categoryDirectory.AddOrReplaceEntry(new ResourceData((uint) Lcid, this));
+            categoryDirectory.InsertOrReplaceEntry(new ResourceData((uint) Lcid, this));
         }
     }
 }

src/AsmResolver.PE/Win32Resources/IResourceDirectory.cs

@@ -136,10 +136,10 @@ IList<IResourceEntry> Entries
         bool TryGetData(uint id, [NotNullWhen(true)] out IResourceData? data);
 
         /// <summary>
-        /// Replaces an existing entry with the same ID with the provided entry, or adds the new entry to the directory.
+        /// Replaces an existing entry with the same ID with the provided entry, or inserts the new entry into the directory.
         /// </summary>
         /// <param name="entry">The entry to store in the directory.</param>
-        void AddOrReplaceEntry(IResourceEntry entry);
+        void InsertOrReplaceEntry(IResourceEntry entry);
 
         /// <summary>
         /// Removes an entry in the directory by its unique identifier.

src/AsmResolver.PE/Win32Resources/ResourceDirectory.cs

@@ -127,16 +127,26 @@ public IList<IResourceEntry> Entries
             }
         }
 
-        private int GetEntryIndex(uint id)
+        private bool TryGetEntryIndex(uint id, out int index)
         {
             for (int i = 0; i < Entries.Count; i++)
             {
                 var candidate = Entries[i];
                 if (candidate.Id == id)
-                    return i;
+                {
+                    index = i;
+                    return true;
+                }
+
+                if (candidate.Id > id)
+                {
+                    index = i;
+                    return false;
+                }
             }
 
-            return -1;
+            index = Entries.Count;
+            return false;
         }
 
         /// <inheritdoc />
@@ -174,15 +184,14 @@ public IResourceData GetData(uint id)
         /// <inheritdoc />
         public bool TryGetEntry(uint id, [NotNullWhen(true)] out IResourceEntry? entry)
         {
-            int index = GetEntryIndex(id);
-            if (index != -1)
+            if (!TryGetEntryIndex(id, out int index))
             {
-                entry = Entries[index];
-                return true;
+                entry = null;
+                return false;
             }
 
-            entry = null;
-            return false;
+            entry = Entries[index];
+            return true;
         }
 
         /// <inheritdoc />
@@ -216,20 +225,18 @@ public bool TryGetData(uint id, [NotNullWhen(true)] out IResourceData? data)
         }
 
         /// <inheritdoc />
-        public void AddOrReplaceEntry(IResourceEntry entry)
+        public void InsertOrReplaceEntry(IResourceEntry entry)
         {
-            int index = GetEntryIndex(entry.Id);
-            if (index == -1)
-                Entries.Add(entry);
-            else
+            if (TryGetEntryIndex(entry.Id, out int index))
                 Entries[index] = entry;
+            else
+                Entries.Insert(index, entry);
         }
 
         /// <inheritdoc />
         public bool RemoveEntry(uint id)
         {
-            int index = GetEntryIndex(id);
-            if (index == -1)
+            if (!TryGetEntryIndex(id, out int index))
                 return false;
 
             Entries.RemoveAt(index);

test/AsmResolver.PE.Tests/Win32Resources/ResourceDirectoryTest.cs

@@ -1,4 +1,5 @@
 using System.Collections.Generic;
+using System.Linq;
 using AsmResolver.PE.Win32Resources;
 using Xunit;
 
@@ -217,7 +218,7 @@ public void AddNewDirectory()
             Assert.Empty(root.Entries);
 
             var directory = new ResourceDirectory(ResourceType.String);
-            root.AddOrReplaceEntry(directory);
+            root.InsertOrReplaceEntry(directory);
 
             Assert.Same(directory, Assert.Single(root.Entries));
         }
@@ -233,7 +234,7 @@ public void AddSecondDirectory()
             Assert.Single(root.Entries);
 
             var directory = new ResourceDirectory(5678u);
-            root.AddOrReplaceEntry(directory);
+            root.InsertOrReplaceEntry(directory);
 
             Assert.Equal(2, root.Entries.Count);
         }
@@ -249,7 +250,7 @@ public void ReplaceDirectoryWithDirectory()
             var oldDirectory = root.GetDirectory(1234u);
 
             var newDirectory = new ResourceDirectory(1234u);
-            root.AddOrReplaceEntry(newDirectory);
+            root.InsertOrReplaceEntry(newDirectory);
 
             Assert.NotSame(oldDirectory, root.GetEntry(1234u));
             Assert.Same(newDirectory, Assert.Single(root.Entries));
@@ -266,7 +267,7 @@ public void ReplaceDirectoryWithData()
             var oldDirectory = root.GetDirectory(1234u);
 
             var newEntry = new ResourceData(1234u, new DataSegment(new byte[] { 1, 2, 3, 4 }));
-            root.AddOrReplaceEntry(newEntry);
+            root.InsertOrReplaceEntry(newEntry);
 
             Assert.NotSame(oldDirectory, root.GetEntry(1234u));
             Assert.Same(newEntry, Assert.Single(root.Entries));
@@ -300,5 +301,22 @@ public void RemoveExistingEntry()
             Assert.Single(root.Entries);
         }
 
+        [Theory]
+        [InlineData(ResourceType.Icon, new[] {ResourceType.Icon, ResourceType.String, ResourceType.Version})]
+        [InlineData(ResourceType.RcData, new[] {ResourceType.String, ResourceType.RcData, ResourceType.Version})]
+        [InlineData(ResourceType.Manifest, new[] {ResourceType.String, ResourceType.Version, ResourceType.Manifest})]
+        [InlineData(ResourceType.String, new[] {ResourceType.String, ResourceType.Version})]
+        public void InsertShouldPreserveOrder(ResourceType insertedEntry, ResourceType[] expected)
+        {
+            var image = new PEImage();
+
+            image.Resources = new ResourceDirectory(0u);
+            image.Resources.Entries.Add(new ResourceDirectory(ResourceType.String));
+            image.Resources.Entries.Add(new ResourceDirectory(ResourceType.Version));
+            image.Resources.InsertOrReplaceEntry(new ResourceDirectory(insertedEntry));
+
+            Assert.Equal(expected, image.Resources.Entries.Select(x => (ResourceType) x.Id));
+        }
+
     }
 }

test/AsmResolver.PE.Win32Resources.Tests/Icon/IconResourceTest.cs

@@ -40,7 +40,7 @@ public void PersistentIconResources()
                 iconGroup.RemoveEntry(4);
                 iconGroup.Count--;
             }
-            iconResource.WriteToDirectory(resources);
+            iconResource.InsertIntoDirectory(resources);
 
             // Rebuild.
             using var stream = new MemoryStream();

test/AsmResolver.PE.Win32Resources.Tests/Version/VersionInfoResourceTest.cs

@@ -188,7 +188,7 @@ public void PersistentVersionResource()
             Assert.NotNull(versionInfo);
 
             versionInfo.FixedVersionInfo.ProductVersion = new System.Version(1, 2, 3, 4);
-            versionInfo.WriteToDirectory(resources);
+            versionInfo.InsertIntoDirectory(resources);
 
             // Rebuild
             using var stream = new MemoryStream();
@@ -216,7 +216,7 @@ public void VersionInfoAlignment()
             var versionInfo = VersionInfoResource.FromDirectory(image.Resources!)!;
             var info = versionInfo.GetChild<StringFileInfo>(StringFileInfo.StringFileInfoKey);
             info.Tables[0][StringTable.FileDescriptionKey] = "This is a test application";
-            versionInfo.WriteToDirectory(image.Resources!);
+            versionInfo.InsertIntoDirectory(image.Resources!);
 
             // Replace section.
             var resourceBuffer = new ResourceDirectoryBuffer();