@joebigelow / wix / commits / 57d67dd1

Trim patch baseline identifiers when they get too long

The PatchBaseline/@Id value is used as part of the name for the patch transforms that get stored as substorages. Substorage name length are very limited. Rather than error, we'll now trim the value and let the user know via a warning. Fixes 4434

Rob Mensching committed Aug 26, 2022 at 15:02 UTC 57d67dd124e5cdc6bef6b11d088ed05d27192405
9 files changed +132 -44
src/wix/WixToolset.Core.WindowsInstaller/Bind/AttachPatchTransformsCommand.cs
+18 -6
@@ -174,14 +174,26 @@ namespace WixToolset.Core.WindowsInstaller.Bind
174 productCodes.Add(productCode);
175 validTransform.Add(Tuple.Create(productCode, mainTransform.Transform));
176
177 - // attach these transforms to the patch object
178 - // TODO: is this an acceptable way to auto-generate transform stream names?
179 - var transformName = mainTransform.Baseline + "." + validTransform.Count.ToString(CultureInfo.InvariantCulture);
180 - subStorages.Add(new SubStorage(transformName, mainTransform.Transform));
181 - subStorages.Add(new SubStorage("#" + transformName, pairedTransform));
177 + // Attach the main and paired transforms to the patch object.
178 + var baseTransformName = mainTransform.Baseline;
179 + var countSuffix = "." + validTransform.Count.ToString(CultureInfo.InvariantCulture);
180 +
181 + if (PatchConstants.PairedPatchTransformPrefix.Length + baseTransformName.Length + countSuffix.Length > PatchConstants.MaxPatchTransformName)
182 + {
183 + var trimmedTransformName = baseTransformName.Substring(0, PatchConstants.MaxPatchTransformName - PatchConstants.PairedPatchTransformPrefix.Length - countSuffix.Length);
184 +
185 + this.Messaging.Write(WindowsInstallerBackendWarnings.LongPatchBaselineIdTrimmed(baselineSymbol.SourceLineNumbers, baseTransformName, trimmedTransformName));
186
187 + baseTransformName = trimmedTransformName;
188 + }
189 +
190 + var transformName = baseTransformName + countSuffix;
191 + subStorages.Add(new SubStorage(transformName, mainTransform.Transform));
192 transformNames.Add(":" + transformName);
184 - transformNames.Add(":#" + transformName);
193 +
194 + var pairedTransformName = PatchConstants.PairedPatchTransformPrefix + transformName;
195 + subStorages.Add(new SubStorage(pairedTransformName, pairedTransform));
196 + transformNames.Add(":" + pairedTransformName);
197 }
198
199 if (validTransform.Count == 0)
src/wix/WixToolset.Core.WindowsInstaller/Bind/GetFileFacadesFromTransforms.cs
+3 -3
@@ -40,10 +40,10 @@ namespace WixToolset.Core.WindowsInstaller.Bind
40 //var patchActualFileTable = this.Output.EnsureTable(this.TableDefinitions["File"]);
41
42 // Index paired transforms by name without their "#" prefix.
43 - var pairedTransforms = this.SubStorages.Where(s => s.Name.StartsWith("#")).ToDictionary(s => s.Name, s => s.Data);
43 + var pairedTransforms = this.SubStorages.Where(s => s.Name.StartsWith(PatchConstants.PairedPatchTransformPrefix)).ToDictionary(s => s.Name, s => s.Data);
44
45 // Enumerate through main transforms.
46 - foreach (var substorage in this.SubStorages.Where(s => !s.Name.StartsWith("#")))
46 + foreach (var substorage in this.SubStorages.Where(s => !s.Name.StartsWith(PatchConstants.PairedPatchTransformPrefix)))
47 {
48 var mainTransform = substorage.Data;
49 var mainFileTable = mainTransform.Tables["File"];
@@ -54,7 +54,7 @@ namespace WixToolset.Core.WindowsInstaller.Bind
54 }
55
56 // Index File table of pairedTransform
57 - var pairedTransform = pairedTransforms["#" + substorage.Name];
57 + var pairedTransform = pairedTransforms[PatchConstants.PairedPatchTransformPrefix + substorage.Name];
58 var pairedFileRows = new RowDictionary<FileRow>(pairedTransform.Tables["File"]);
59
60 foreach (FileRow mainFileRow in mainFileTable.Rows.Where(f => f.Operation != RowOperation.Delete))
src/wix/WixToolset.Core.WindowsInstaller/Bind/PatchConstants.cs new
+11
@@ -0,0 +1,11 @@
1 +// Copyright (c) .NET Foundation and contributors. All rights reserved. Licensed under the Microsoft Reciprocal License. See LICENSE.TXT file in the project root for full license information.
2 +
3 +namespace WixToolset.Core.WindowsInstaller.Bind
4 +{
5 + internal static class PatchConstants
6 + {
7 + public const int MaxPatchTransformName = 31;
8 +
9 + public static readonly string PairedPatchTransformPrefix = "#";
10 + }
11 +}
src/wix/WixToolset.Core.WindowsInstaller/Bind/UpdateTransformsWithFileFacades.cs
+3 -3
@@ -52,16 +52,16 @@ namespace WixToolset.Core.WindowsInstaller.Bind
52 var patchMediaRows = new RowDictionary<MediaRow>(this.Output.Tables["Media"]);
53
54 // Index paired transforms by name without the "#" prefix.
55 - var pairedTransforms = this.SubStorages.Where(s => s.Name.StartsWith("#")).ToDictionary(s => s.Name, s => s.Data);
55 + var pairedTransforms = this.SubStorages.Where(s => s.Name.StartsWith(PatchConstants.PairedPatchTransformPrefix)).ToDictionary(s => s.Name, s => s.Data);
56
57 // Copy File bind data into substorages
58 - foreach (var substorage in this.SubStorages.Where(s => !s.Name.StartsWith("#")))
58 + foreach (var substorage in this.SubStorages.Where(s => !s.Name.StartsWith(PatchConstants.PairedPatchTransformPrefix)))
59 {
60 var mainTransform = substorage.Data;
61
62 var mainMsiFileHashIndex = new RowDictionary<Row>(mainTransform.Tables["MsiFileHash"]);
63
64 - var pairedTransform = pairedTransforms["#" + substorage.Name];
64 + var pairedTransform = pairedTransforms[PatchConstants.PairedPatchTransformPrefix + substorage.Name];
65
66 // Copy Media.LastSequence.
67 var pairedMediaTable = pairedTransform.Tables["Media"];
src/wix/WixToolset.Core.WindowsInstaller/WindowsInstallerBackendWarnings.cs
+5 -5
@@ -6,10 +6,10 @@ namespace WixToolset.Core.WindowsInstaller
6
7 internal static class WindowsInstallerBackendWarnings
8 {
9 - //public static Message ReplaceThisWithTheFirstWarning(SourceLineNumber sourceLineNumbers)
10 - //{
11 - // return Message(sourceLineNumbers, Ids.ReplaceThisWithTheFirstWarning, "format string", arg1, arg2);
12 - //}
9 + internal static Message LongPatchBaselineIdTrimmed(SourceLineNumber sourceLineNumbers, string baseTransformName, string trimmedTransformName)
10 + {
11 + return Message(sourceLineNumbers, Ids.LongPatchBaselineIdTrimmed, "The PatchBaseline/@Id='{0}' is too long. It is recommended to use short identifiers like 'RTM' and 'SP1'. The identifier has been trimmed to '{1}' so the patch can be created.", baseTransformName, trimmedTransformName);
12 + }
13
14 private static Message Message(SourceLineNumber sourceLineNumber, Ids id, string format, params object[] args)
15 {
@@ -18,7 +18,7 @@ namespace WixToolset.Core.WindowsInstaller
18
19 public enum Ids
20 {
21 - // ReplaceThisWithTheFirstWarning = 7100,
21 + LongPatchBaselineIdTrimmed = 7100,
22 } // last available is 7499. 7500 is WindowsInstallerBackendErrors.
23 }
24 }
src/wix/WixToolset.Core/Compiler.cs
-4
@@ -7745,10 +7745,6 @@ namespace WixToolset.Core
7745 this.Core.Write(ErrorMessages.ExpectedAttribute(sourceLineNumbers, node.Name.LocalName, "Id"));
7746 id = Identifier.Invalid;
7747 }
7748 - else if (27 < id.Id.Length)
7749 - {
7750 - this.Core.Write(ErrorMessages.IdentifierTooLongError(sourceLineNumbers, node.Name.LocalName, "Id", id.Id, 27));
7751 - }
7748
7749 if (!String.IsNullOrEmpty(baselineFile) || !String.IsNullOrEmpty(updateFile))
7750 {
src/wix/test/WixToolsetTest.CoreIntegration/PatchFixture.cs
+48 -23
@@ -25,7 +25,7 @@ namespace WixToolsetTest.CoreIntegration
25 [Fact]
26 public void CanBuildSimplePatch()
27 {
28 - var folder = TestData.Get(@"TestData\PatchSingle");
28 + var folder = TestData.Get(@"TestData", "PatchSingle");
29
30 using (var fs = new DisposableFileSystem())
31 {
@@ -57,7 +57,7 @@ namespace WixToolsetTest.CoreIntegration
57 [Fact]
58 public void CanBuildSimplePatchWithNoFileChanges()
59 {
60 - var folder = TestData.Get(@"TestData\PatchNoFileChanges");
60 + var folder = TestData.Get(@"TestData", "PatchNoFileChanges");
61
62 using (var fs = new DisposableFileSystem())
63 {
@@ -86,6 +86,31 @@ namespace WixToolsetTest.CoreIntegration
86 }
87 }
88
89 + [Fact]
90 + public void CanBuildSimplePatchWithBaselineIdTooLong()
91 + {
92 + var folder = TestData.Get(@"TestData", "PatchBaselineIdTooLong");
93 +
94 + using (var fs = new DisposableFileSystem())
95 + {
96 + var baseFolder = fs.GetFolder();
97 + var tempFolderBaseline = Path.Combine(baseFolder, "baseline");
98 + var tempFolderUpdate = Path.Combine(baseFolder, "update");
99 + var tempFolderPatch = Path.Combine(baseFolder, "patch");
100 +
101 + var baselinePdb = BuildMsi("Baseline.msi", folder, tempFolderBaseline, "1.0.0", "1.0.0", "1.0.0");
102 + var update1Pdb = BuildMsi("Update.msi", folder, tempFolderUpdate, "1.0.1", "1.0.1", "1.0.1");
103 + var patchPdb = BuildMsp("Patch1.msp", folder, tempFolderPatch, "1.0.1", bindpaths: new[] { Path.GetDirectoryName(baselinePdb), Path.GetDirectoryName(update1Pdb) }, hasNoFiles: true, warningsAsErrors: false);
104 + var patchPath = Path.ChangeExtension(patchPdb, ".msp");
105 +
106 + var doc = GetExtractPatchXml(patchPath);
107 + WixAssert.StringEqual("{11111111-2222-3333-4444-555555555555}", doc.Root.Element(PatchNamespace + "TargetProductCode").Value);
108 +
109 + var names = Query.GetSubStorageNames(patchPath);
110 + WixAssert.CompareLineByLine(new[] { "#ThisBaseLineIdIsTooLongAndGe.1", "ThisBaseLineIdIsTooLongAndGe.1" }, names);
111 + }
112 + }
113 +
114 [Fact(Skip = "https://github.com/wixtoolset/issues/issues/6387")]
115 public void CanBuildPatchFromProductWithFilesFromWixlib()
116 {
@@ -110,7 +135,7 @@ namespace WixToolsetTest.CoreIntegration
135 [Fact]
136 public void CanBuildBundleWithNonSpecificPatches()
137 {
113 - var folder = TestData.Get(@"TestData\PatchNonSpecific");
138 + var folder = TestData.Get(@"TestData", "PatchNonSpecific");
139
140 using (var fs = new DisposableFileSystem())
141 {
@@ -125,23 +150,23 @@ namespace WixToolsetTest.CoreIntegration
150 var bundleBPdb = BuildBundle("BundleB.exe", Path.Combine(folder, "BundleB"), tempFolder);
151 var bundleCPdb = BuildBundle("BundleC.exe", Path.Combine(folder, "BundleC"), tempFolder);
152
128 - VerifyPatchTargetCodes(bundleAPdb, new[]
153 + VerifyPatchTargetCodesInBurnManifest(bundleAPdb, new[]
154 {
155 "<PatchTargetCode TargetCode='{26309973-0A5E-4979-B142-98A6E064EDC0}' Product='yes' />",
156 });
132 - VerifyPatchTargetCodes(bundleBPdb, new[]
157 + VerifyPatchTargetCodesInBurnManifest(bundleBPdb, new[]
158 {
159 "<PatchTargetCode TargetCode='{26309973-0A5E-4979-B142-98A6E064EDC0}' Product='yes' />",
160 "<PatchTargetCode TargetCode='{32B0396A-CE36-4570-B16E-F88FA42DC409}' Product='no' />",
161 });
137 - VerifyPatchTargetCodes(bundleCPdb, new string[0]);
162 + VerifyPatchTargetCodesInBurnManifest(bundleCPdb, new string[0]);
163 }
164 }
165
166 [Fact]
167 public void CanBuildBundleWithSlipstreamPatch()
168 {
144 - var folder = TestData.Get(@"TestData\PatchSingle");
169 + var folder = TestData.Get(@"TestData", "PatchSingle");
170
171 using (var fs = new DisposableFileSystem())
172 {
@@ -167,20 +192,6 @@ namespace WixToolsetTest.CoreIntegration
192 }
193 }
194
170 - private static void VerifyPatchTargetCodes(string pdbPath, string[] expected)
171 - {
172 - using (var wixOutput = WixOutput.Read(pdbPath))
173 - {
174 - var manifestData = wixOutput.GetData(BurnConstants.BurnManifestWixOutputStreamName);
175 - var doc = new XmlDocument();
176 - doc.LoadXml(manifestData);
177 - var nsmgr = BundleExtractor.GetBurnNamespaceManager(doc, "w");
178 - var patchTargetCodes = doc.SelectNodes("/w:BurnManifest/w:PatchTargetCode", nsmgr).GetTestXmlLines();
179 -
180 - WixAssert.CompareLineByLine(expected, patchTargetCodes);
181 - }
182 - }
183 -
195 private static string BuildMsi(string outputName, string sourceFolder, string baseFolder, string defineV, string defineA, string defineB)
196 {
197 var extensionPath = Path.GetFullPath(new Uri(typeof(ExampleExtensionFactory).Assembly.CodeBase).LocalPath);
@@ -204,7 +215,7 @@ namespace WixToolsetTest.CoreIntegration
215 return Path.ChangeExtension(outputPath, ".wixpdb");
216 }
217
207 - private static string BuildMsp(string outputName, string sourceFolder, string baseFolder, string defineV, IEnumerable<string> bindpaths = null, bool hasNoFiles = false)
218 + private static string BuildMsp(string outputName, string sourceFolder, string baseFolder, string defineV, IEnumerable<string> bindpaths = null, bool hasNoFiles = false, bool warningsAsErrors = true)
219 {
220 var outputPath = Path.Combine(baseFolder, Path.Combine("bin", outputName));
221
@@ -225,7 +236,7 @@ namespace WixToolsetTest.CoreIntegration
236 args.Add(additionaBindPath);
237 }
238
228 - var result = WixRunner.Execute(args.ToArray());
239 + var result = WixRunner.Execute(warningsAsErrors, args.ToArray());
240
241 result.AssertSuccess();
242
@@ -266,6 +277,20 @@ namespace WixToolsetTest.CoreIntegration
277 return XDocument.Parse(buffer.ToString());
278 }
279
280 + private static void VerifyPatchTargetCodesInBurnManifest(string pdbPath, string[] expected)
281 + {
282 + using (var wixOutput = WixOutput.Read(pdbPath))
283 + {
284 + var manifestData = wixOutput.GetData(BurnConstants.BurnManifestWixOutputStreamName);
285 + var doc = new XmlDocument();
286 + doc.LoadXml(manifestData);
287 + var nsmgr = BundleExtractor.GetBurnNamespaceManager(doc, "w");
288 + var patchTargetCodes = doc.SelectNodes("/w:BurnManifest/w:PatchTargetCode", nsmgr).GetTestXmlLines();
289 +
290 + WixAssert.CompareLineByLine(expected, patchTargetCodes);
291 + }
292 + }
293 +
294 [DllImport("msi.dll", EntryPoint = "MsiExtractPatchXMLDataW", CharSet = CharSet.Unicode, ExactSpelling = true)]
295 private static extern int MsiExtractPatchXMLData(string szPatchPath, int dwReserved, StringBuilder szXMLData, ref int pcchXMLData);
296
src/wix/test/WixToolsetTest.CoreIntegration/TestData/PatchBaselineIdTooLong/Package.wxs new
+28
@@ -0,0 +1,28 @@
1 +<Wix xmlns="http://wixtoolset.org/schemas/v4/wxs">
2 + <Package Name="~Test Package" Version="$(var.V)" Manufacturer="Example Corporation" Language="1033" UpgradeCode="{6B8097B9-A5D0-4BDE-B21E-AF6622DDCA01}" Scope="perMachine" ProductCode="{11111111-2222-3333-4444-555555555555}">
3 + <MajorUpgrade DowngradeErrorMessage="Newer version already installed." />
4 + <MediaTemplate EmbedCab="yes" />
5 +
6 + <CustomAction Id="CAFromExtension" DllEntry="DoesntExist" BinaryRef="BinFromWir" />
7 +
8 + <StandardDirectory Id="ProgramFilesFolder">
9 + <Directory Id="INSTALLFOLDER" Name="~Test App" />
10 + </StandardDirectory>
11 +
12 + <Feature Id="Main">
13 + <ComponentGroupRef Id="Components" />
14 + </Feature>
15 + </Package>
16 +
17 + <Fragment>
18 + <ComponentGroup Id="Components" Directory="INSTALLFOLDER">
19 + <Component>
20 + <File Source="$(sys.SOURCEFILEPATH)" />
21 + </Component>
22 +
23 + <Component>
24 + <RegistryValue Root="HKLM" Key="SOFTWARE\!(bind.property.ProductName)\Patch" Name="Version" Value="$(var.V)" />
25 + </Component>
26 + </ComponentGroup>
27 + </Fragment>
28 +</Wix>
src/wix/test/WixToolsetTest.CoreIntegration/TestData/PatchBaselineIdTooLong/Patch.wxs new
+16
@@ -0,0 +1,16 @@
1 +<Wix xmlns='http://wixtoolset.org/schemas/v4/wxs'>
2 + <Patch
3 + AllowRemoval="yes"
4 + DisplayName="~Test Patch v$(var.V)"
5 + Description="~Test Small Update Patch v$(var.V)"
6 + MoreInfoURL="http://www.example.com/"
7 + Manufacturer="Example Corporation"
8 + Classification="Update">
9 +
10 + <Media Id="1" Cabinet="foo.cab">
11 + <PatchBaseline Id="ThisBaseLineIdIsTooLongAndGetsTrimmed" BaselineFile="Baseline.wixpdb" UpdateFile="Update.wixpdb" />
12 + </Media>
13 +
14 + <PatchFamily Id='SequenceFamily' Version='$(var.V)' />
15 + </Patch>
16 +</Wix>