From 946590906e0e531253fe2ff16adb31b50980f7b3 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Wed, 24 Jul 2024 11:25:01 -0700 Subject: [PATCH 1/8] Update the ResourceData API This is a subtle change in behavior to match the undocumented semantic of Win32 Resource APIs. The missing semantic is that all resource type/name strings are transparently converted to uppercase when calling any of the Win32 Resource APIs. --- .../ResourceData.UpdateResourceDataModel.cs | 16 ++++++++++++++-- .../Microsoft.NET.HostModel/ComHost/ComHost.cs | 2 +- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/coreclr/tools/Common/Compiler/Win32Resources/ResourceData.UpdateResourceDataModel.cs b/src/coreclr/tools/Common/Compiler/Win32Resources/ResourceData.UpdateResourceDataModel.cs index 8ee6aaebc6d2d9..40e5d5c97b03f0 100644 --- a/src/coreclr/tools/Common/Compiler/Win32Resources/ResourceData.UpdateResourceDataModel.cs +++ b/src/coreclr/tools/Common/Compiler/Win32Resources/ResourceData.UpdateResourceDataModel.cs @@ -26,6 +26,9 @@ private void AddResourceInternal(object name, object type, ushort language, byte } else { + Debug.Assert(type is string); + // Undocumented semantic for Win32 Resource APIs + type = ((string)type).ToUpperInvariant(); if (!_resTypeHeadName.TryGetValue((string)type, out resType)) { resType = new ResType(); @@ -45,6 +48,9 @@ private void AddResourceInternal(object name, object type, ushort language, byte } else { + Debug.Assert(name is string); + // Undocumented semantic for Win32 Resource APIs + name = ((string)name).ToUpperInvariant(); if (!resType.NameHeadName.TryGetValue((string)name, out resName)) { resName = new ResName(); @@ -63,8 +69,11 @@ private byte[] FindResourceInternal(object name, object type, ushort language) { _resTypeHeadID.TryGetValue((ushort)type, out resType); } - if (type is string) + else { + Debug.Assert(type is string); + // Undocumented semantic for Win32 Resource APIs + type = ((string)type).ToUpperInvariant(); _resTypeHeadName.TryGetValue((string)type, out resType); } @@ -77,8 +86,11 @@ private byte[] FindResourceInternal(object name, object type, ushort language) { resType.NameHeadID.TryGetValue((ushort)name, out resName); } - if (name is string) + else { + Debug.Assert(name is string); + // Undocumented semantic for Win32 Resource APIs + name = ((string)name).ToUpperInvariant(); resType.NameHeadName.TryGetValue((string)name, out resName); } diff --git a/src/installer/managed/Microsoft.NET.HostModel/ComHost/ComHost.cs b/src/installer/managed/Microsoft.NET.HostModel/ComHost/ComHost.cs index f344da03bd76bb..4b6d31fdc76e3c 100644 --- a/src/installer/managed/Microsoft.NET.HostModel/ComHost/ComHost.cs +++ b/src/installer/managed/Microsoft.NET.HostModel/ComHost/ComHost.cs @@ -57,7 +57,7 @@ public static void Create( if (tlbFileBytes.Length == 0) throw new InvalidTypeLibraryException(typeLibrary.Value); - updater.AddResource(tlbFileBytes, "typelib", (IntPtr)typeLibrary.Key); + updater.AddResource(tlbFileBytes, "TYPELIB", (IntPtr)typeLibrary.Key); } catch (FileNotFoundException ex) { From c70f613f1439aa73e1146d11d186c07a55a4a55a Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Wed, 24 Jul 2024 11:57:58 -0700 Subject: [PATCH 2/8] More precisely implement the upper semantic --- .../ResourceData.UpdateResourceDataModel.cs | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/coreclr/tools/Common/Compiler/Win32Resources/ResourceData.UpdateResourceDataModel.cs b/src/coreclr/tools/Common/Compiler/Win32Resources/ResourceData.UpdateResourceDataModel.cs index 40e5d5c97b03f0..ec7a33aed1ccf7 100644 --- a/src/coreclr/tools/Common/Compiler/Win32Resources/ResourceData.UpdateResourceDataModel.cs +++ b/src/coreclr/tools/Common/Compiler/Win32Resources/ResourceData.UpdateResourceDataModel.cs @@ -27,8 +27,7 @@ private void AddResourceInternal(object name, object type, ushort language, byte else { Debug.Assert(type is string); - // Undocumented semantic for Win32 Resource APIs - type = ((string)type).ToUpperInvariant(); + type = ToUpperForResource((string)type); if (!_resTypeHeadName.TryGetValue((string)type, out resType)) { resType = new ResType(); @@ -49,8 +48,7 @@ private void AddResourceInternal(object name, object type, ushort language, byte else { Debug.Assert(name is string); - // Undocumented semantic for Win32 Resource APIs - name = ((string)name).ToUpperInvariant(); + name = ToUpperForResource((string)name); if (!resType.NameHeadName.TryGetValue((string)name, out resName)) { resName = new ResName(); @@ -72,8 +70,7 @@ private byte[] FindResourceInternal(object name, object type, ushort language) else { Debug.Assert(type is string); - // Undocumented semantic for Win32 Resource APIs - type = ((string)type).ToUpperInvariant(); + type = ToUpperForResource((string)type); _resTypeHeadName.TryGetValue((string)type, out resType); } @@ -89,8 +86,7 @@ private byte[] FindResourceInternal(object name, object type, ushort language) else { Debug.Assert(name is string); - // Undocumented semantic for Win32 Resource APIs - name = ((string)name).ToUpperInvariant(); + name = ToUpperForResource((string)name); resType.NameHeadName.TryGetValue((string)name, out resName); } @@ -102,5 +98,16 @@ private byte[] FindResourceInternal(object name, object type, ushort language) return (byte[])resLanguage.DataEntry.Clone(); } + + private static string ToUpperForResource(string str) + { + // Undocumented semantic for Win32 Resource APIs + StringBuilder builder = new(str.Length); + foreach (char c in str) + { + builder.Append('a' <= c && c <= 'z' ? char.ToUpper(c) : c); + } + return builder.ToString(); + } } } From d763cf0a4cef64f789b701f5ec295acaec672365 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Wed, 24 Jul 2024 13:28:48 -0700 Subject: [PATCH 3/8] Missed namespaces --- .../Win32Resources/ResourceData.UpdateResourceDataModel.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/coreclr/tools/Common/Compiler/Win32Resources/ResourceData.UpdateResourceDataModel.cs b/src/coreclr/tools/Common/Compiler/Win32Resources/ResourceData.UpdateResourceDataModel.cs index ec7a33aed1ccf7..57c4c6634e3427 100644 --- a/src/coreclr/tools/Common/Compiler/Win32Resources/ResourceData.UpdateResourceDataModel.cs +++ b/src/coreclr/tools/Common/Compiler/Win32Resources/ResourceData.UpdateResourceDataModel.cs @@ -3,6 +3,7 @@ using System; using System.Collections; +using System.Diagnostics; #if HOST_MODEL namespace Microsoft.NET.HostModel.Win32Resources @@ -102,7 +103,7 @@ private byte[] FindResourceInternal(object name, object type, ushort language) private static string ToUpperForResource(string str) { // Undocumented semantic for Win32 Resource APIs - StringBuilder builder = new(str.Length); + System.Text.StringBuilder builder = new(str.Length); foreach (char c in str) { builder.Append('a' <= c && c <= 'z' ? char.ToUpper(c) : c); From 0bb8b65f98705279b8a8d2694b7b118a30d45caa Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Wed, 24 Jul 2024 14:14:37 -0700 Subject: [PATCH 4/8] Feedback --- .../Win32Resources/ResourceData.UpdateResourceDataModel.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/tools/Common/Compiler/Win32Resources/ResourceData.UpdateResourceDataModel.cs b/src/coreclr/tools/Common/Compiler/Win32Resources/ResourceData.UpdateResourceDataModel.cs index 57c4c6634e3427..e96713ecce2cb5 100644 --- a/src/coreclr/tools/Common/Compiler/Win32Resources/ResourceData.UpdateResourceDataModel.cs +++ b/src/coreclr/tools/Common/Compiler/Win32Resources/ResourceData.UpdateResourceDataModel.cs @@ -78,7 +78,7 @@ private byte[] FindResourceInternal(object name, object type, ushort language) if (resType == null) return null; - ResName resName = null; + ResName resName; if (name is ushort) { @@ -106,7 +106,7 @@ private static string ToUpperForResource(string str) System.Text.StringBuilder builder = new(str.Length); foreach (char c in str) { - builder.Append('a' <= c && c <= 'z' ? char.ToUpper(c) : c); + builder.Append('a' <= c && c <= 'z' ? (char)(c + ('A' - 'a')) : c); } return builder.ToString(); } From 943383068cadbaa63e65a1de5a524b6719862b41 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Wed, 24 Jul 2024 14:17:14 -0700 Subject: [PATCH 5/8] Add comment. --- .../Win32Resources/ResourceData.UpdateResourceDataModel.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/coreclr/tools/Common/Compiler/Win32Resources/ResourceData.UpdateResourceDataModel.cs b/src/coreclr/tools/Common/Compiler/Win32Resources/ResourceData.UpdateResourceDataModel.cs index e96713ecce2cb5..ce359c240f9fba 100644 --- a/src/coreclr/tools/Common/Compiler/Win32Resources/ResourceData.UpdateResourceDataModel.cs +++ b/src/coreclr/tools/Common/Compiler/Win32Resources/ResourceData.UpdateResourceDataModel.cs @@ -103,6 +103,8 @@ private byte[] FindResourceInternal(object name, object type, ushort language) private static string ToUpperForResource(string str) { // Undocumented semantic for Win32 Resource APIs + // This ToUpper logic should only be applied to the + // latin range from 'a' to 'z'. System.Text.StringBuilder builder = new(str.Length); foreach (char c in str) { From ce9630c15843fbd7353ee0e931c7ca69f5294fe3 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Wed, 24 Jul 2024 14:48:58 -0700 Subject: [PATCH 6/8] Fix build. --- .../Win32Resources/ResourceData.UpdateResourceDataModel.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/tools/Common/Compiler/Win32Resources/ResourceData.UpdateResourceDataModel.cs b/src/coreclr/tools/Common/Compiler/Win32Resources/ResourceData.UpdateResourceDataModel.cs index ce359c240f9fba..c8863981ad37cd 100644 --- a/src/coreclr/tools/Common/Compiler/Win32Resources/ResourceData.UpdateResourceDataModel.cs +++ b/src/coreclr/tools/Common/Compiler/Win32Resources/ResourceData.UpdateResourceDataModel.cs @@ -62,7 +62,7 @@ private void AddResourceInternal(object name, object type, ushort language, byte private byte[] FindResourceInternal(object name, object type, ushort language) { - ResType resType = null; + ResType resType; if (type is ushort) { From ff32bcc64437a03d6ca8ca2fd2d02949980801e6 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Wed, 24 Jul 2024 16:44:28 -0700 Subject: [PATCH 7/8] Add test --- .../ResourceUpdaterTests.cs | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/installer/tests/Microsoft.NET.HostModel.Tests/ResourceUpdaterTests.cs b/src/installer/tests/Microsoft.NET.HostModel.Tests/ResourceUpdaterTests.cs index 6aefb0f53ea007..a0722abbc22cbd 100644 --- a/src/installer/tests/Microsoft.NET.HostModel.Tests/ResourceUpdaterTests.cs +++ b/src/installer/tests/Microsoft.NET.HostModel.Tests/ResourceUpdaterTests.cs @@ -78,6 +78,37 @@ void AddResource_AddToPEWithoutRsrc() } } + [Fact] + void AddResource_DifferentCasingInAPIs() + { + using var tempFile = CreateTestPEFileWithoutRsrc(); + + using (var updater = new ResourceUpdater(tempFile.Stream, true)) + { + updater.AddResource("Test Resource1"u8.ToArray(), "testtype1", 0); + updater.AddResource("Test Resource2"u8.ToArray(), "TESTTYPE2", 0); + updater.AddResource("Test Resource3"u8.ToArray(), "\u0165ESTTYPE3", 0); + updater.Update(); + } + + tempFile.Stream.Seek(0, SeekOrigin.Begin); + + using (var reader = new PEReader(tempFile.Stream, PEStreamOptions.LeaveOpen)) + { + var resourceReader = new ResourceData(reader); + byte[]? testType1 = resourceReader.FindResource(0, "TESTTYPE1", 0); + Assert.Equal("Test Resource1"u8.ToArray(), testType1); + byte[]? testType2 = resourceReader.FindResource(0, "testtype2", 0); + Assert.Equal("Test Resource2"u8.ToArray(), testType2); + byte[]? testType3 = resourceReader.FindResource(0, "\u0165esttype3", 0); + Assert.Equal("Test Resource3"u8.ToArray(), testType3); + + // Characters out of 'a' - 'z' are left unaltered + byte[]? missing = resourceReader.FindResource(0, "\u0164ESTTYPE3", 0); + Assert.Null(missing); + } + } + [Fact] void AddResource_AddToExistingRsrc() { From 94334701273bf1d89b5baabcc5af8c306e4be30a Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Thu, 25 Jul 2024 08:37:35 -0700 Subject: [PATCH 8/8] Remove API and test changes. Add comment to API about Win32 API behavior. --- .../ResourceData.UpdateResourceDataModel.cs | 17 ---------- .../Compiler/Win32Resources/ResourceData.cs | 24 ++++++++++++++ .../ResourceUpdaterTests.cs | 31 ------------------- 3 files changed, 24 insertions(+), 48 deletions(-) diff --git a/src/coreclr/tools/Common/Compiler/Win32Resources/ResourceData.UpdateResourceDataModel.cs b/src/coreclr/tools/Common/Compiler/Win32Resources/ResourceData.UpdateResourceDataModel.cs index c8863981ad37cd..3da313f0ff7527 100644 --- a/src/coreclr/tools/Common/Compiler/Win32Resources/ResourceData.UpdateResourceDataModel.cs +++ b/src/coreclr/tools/Common/Compiler/Win32Resources/ResourceData.UpdateResourceDataModel.cs @@ -28,7 +28,6 @@ private void AddResourceInternal(object name, object type, ushort language, byte else { Debug.Assert(type is string); - type = ToUpperForResource((string)type); if (!_resTypeHeadName.TryGetValue((string)type, out resType)) { resType = new ResType(); @@ -49,7 +48,6 @@ private void AddResourceInternal(object name, object type, ushort language, byte else { Debug.Assert(name is string); - name = ToUpperForResource((string)name); if (!resType.NameHeadName.TryGetValue((string)name, out resName)) { resName = new ResName(); @@ -71,7 +69,6 @@ private byte[] FindResourceInternal(object name, object type, ushort language) else { Debug.Assert(type is string); - type = ToUpperForResource((string)type); _resTypeHeadName.TryGetValue((string)type, out resType); } @@ -87,7 +84,6 @@ private byte[] FindResourceInternal(object name, object type, ushort language) else { Debug.Assert(name is string); - name = ToUpperForResource((string)name); resType.NameHeadName.TryGetValue((string)name, out resName); } @@ -99,18 +95,5 @@ private byte[] FindResourceInternal(object name, object type, ushort language) return (byte[])resLanguage.DataEntry.Clone(); } - - private static string ToUpperForResource(string str) - { - // Undocumented semantic for Win32 Resource APIs - // This ToUpper logic should only be applied to the - // latin range from 'a' to 'z'. - System.Text.StringBuilder builder = new(str.Length); - foreach (char c in str) - { - builder.Append('a' <= c && c <= 'z' ? (char)(c + ('A' - 'a')) : c); - } - return builder.ToString(); - } } } diff --git a/src/coreclr/tools/Common/Compiler/Win32Resources/ResourceData.cs b/src/coreclr/tools/Common/Compiler/Win32Resources/ResourceData.cs index ff6b7fb9ee6f4b..007374e35a8602 100644 --- a/src/coreclr/tools/Common/Compiler/Win32Resources/ResourceData.cs +++ b/src/coreclr/tools/Common/Compiler/Win32Resources/ResourceData.cs @@ -60,6 +60,10 @@ public ResourceData(Internal.TypeSystem.Ecma.EcmaModule ecmaModule, Func /// Find a resource in the resource data /// + /// + /// The Win32 APIs typcially perform an uppercase transform on string arguments - during add and find. + /// If the resource will be read by Win32 APIs, it is recommended to make the resource name upper case. + /// public byte[] FindResource(string name, string type, ushort language) { return FindResourceInternal(name, type, language); @@ -68,6 +72,10 @@ public byte[] FindResource(string name, string type, ushort language) /// /// Find a resource in the resource data /// + /// + /// The Win32 APIs typcially perform an uppercase transform on string arguments - during add and find. + /// If the resource will be read by Win32 APIs, it is recommended to make the resource name upper case. + /// public byte[] FindResource(ushort name, string type, ushort language) { return FindResourceInternal(name, type, language); @@ -76,6 +84,10 @@ public byte[] FindResource(ushort name, string type, ushort language) /// /// Find a resource in the resource data /// + /// + /// The Win32 APIs typcially perform an uppercase transform on string arguments - during add and find. + /// If the resource will be read by Win32 APIs, it is recommended to make the resource name upper case. + /// public byte[] FindResource(string name, ushort type, ushort language) { return FindResourceInternal(name, type, language); @@ -92,16 +104,28 @@ public byte[] FindResource(ushort name, ushort type, ushort language) /// /// Add or update resource /// + /// + /// The Win32 APIs typcially perform an uppercase transform on string arguments - during add and find. + /// If the resource will be read by Win32 APIs, it is recommended to make the resource name upper case. + /// public void AddResource(string name, string type, ushort language, byte[] data) => AddResourceInternal(name, type, language, data); /// /// Add or update resource /// + /// + /// The Win32 APIs typcially perform an uppercase transform on string arguments - during add and find. + /// If the resource will be read by Win32 APIs, it is recommended to make the resource name upper case. + /// public void AddResource(string name, ushort type, ushort language, byte[] data) => AddResourceInternal(name, type, language, data); /// /// Add or update resource /// + /// + /// The Win32 APIs typcially perform an uppercase transform on string arguments - during add and find. + /// If the resource will be read by Win32 APIs, it is recommended to make the resource name upper case. + /// public void AddResource(ushort name, string type, ushort language, byte[] data) => AddResourceInternal(name, type, language, data); /// diff --git a/src/installer/tests/Microsoft.NET.HostModel.Tests/ResourceUpdaterTests.cs b/src/installer/tests/Microsoft.NET.HostModel.Tests/ResourceUpdaterTests.cs index a0722abbc22cbd..6aefb0f53ea007 100644 --- a/src/installer/tests/Microsoft.NET.HostModel.Tests/ResourceUpdaterTests.cs +++ b/src/installer/tests/Microsoft.NET.HostModel.Tests/ResourceUpdaterTests.cs @@ -78,37 +78,6 @@ void AddResource_AddToPEWithoutRsrc() } } - [Fact] - void AddResource_DifferentCasingInAPIs() - { - using var tempFile = CreateTestPEFileWithoutRsrc(); - - using (var updater = new ResourceUpdater(tempFile.Stream, true)) - { - updater.AddResource("Test Resource1"u8.ToArray(), "testtype1", 0); - updater.AddResource("Test Resource2"u8.ToArray(), "TESTTYPE2", 0); - updater.AddResource("Test Resource3"u8.ToArray(), "\u0165ESTTYPE3", 0); - updater.Update(); - } - - tempFile.Stream.Seek(0, SeekOrigin.Begin); - - using (var reader = new PEReader(tempFile.Stream, PEStreamOptions.LeaveOpen)) - { - var resourceReader = new ResourceData(reader); - byte[]? testType1 = resourceReader.FindResource(0, "TESTTYPE1", 0); - Assert.Equal("Test Resource1"u8.ToArray(), testType1); - byte[]? testType2 = resourceReader.FindResource(0, "testtype2", 0); - Assert.Equal("Test Resource2"u8.ToArray(), testType2); - byte[]? testType3 = resourceReader.FindResource(0, "\u0165esttype3", 0); - Assert.Equal("Test Resource3"u8.ToArray(), testType3); - - // Characters out of 'a' - 'z' are left unaltered - byte[]? missing = resourceReader.FindResource(0, "\u0164ESTTYPE3", 0); - Assert.Null(missing); - } - } - [Fact] void AddResource_AddToExistingRsrc() {