From 57d67dd124e5cdc6bef6b11d088ed05d27192405 Mon Sep 17 00:00:00 2001 From: Rob Mensching Date: Fri, 26 Aug 2022 15:02:13 -0700 Subject: 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 --- .../Bind/AttachPatchTransformsCommand.cs | 24 ++++++-- .../Bind/GetFileFacadesFromTransforms.cs | 6 +- .../Bind/PatchConstants.cs | 11 ++++ .../Bind/UpdateTransformsWithFileFacades.cs | 6 +- .../WindowsInstallerBackendWarnings.cs | 10 +-- src/wix/WixToolset.Core/Compiler.cs | 4 -- .../WixToolsetTest.CoreIntegration/PatchFixture.cs | 71 +++++++++++++++------- .../TestData/PatchBaselineIdTooLong/Package.wxs | 28 +++++++++ .../TestData/PatchBaselineIdTooLong/Patch.wxs | 16 +++++ 9 files changed, 132 insertions(+), 44 deletions(-) create mode 100644 src/wix/WixToolset.Core.WindowsInstaller/Bind/PatchConstants.cs create mode 100644 src/wix/test/WixToolsetTest.CoreIntegration/TestData/PatchBaselineIdTooLong/Package.wxs create mode 100644 src/wix/test/WixToolsetTest.CoreIntegration/TestData/PatchBaselineIdTooLong/Patch.wxs diff --git a/src/wix/WixToolset.Core.WindowsInstaller/Bind/AttachPatchTransformsCommand.cs b/src/wix/WixToolset.Core.WindowsInstaller/Bind/AttachPatchTransformsCommand.cs index 76bcd532..66078d8d 100644 --- a/src/wix/WixToolset.Core.WindowsInstaller/Bind/AttachPatchTransformsCommand.cs +++ b/src/wix/WixToolset.Core.WindowsInstaller/Bind/AttachPatchTransformsCommand.cs @@ -174,14 +174,26 @@ namespace WixToolset.Core.WindowsInstaller.Bind productCodes.Add(productCode); validTransform.Add(Tuple.Create(productCode, mainTransform.Transform)); - // attach these transforms to the patch object - // TODO: is this an acceptable way to auto-generate transform stream names? - var transformName = mainTransform.Baseline + "." + validTransform.Count.ToString(CultureInfo.InvariantCulture); - subStorages.Add(new SubStorage(transformName, mainTransform.Transform)); - subStorages.Add(new SubStorage("#" + transformName, pairedTransform)); + // Attach the main and paired transforms to the patch object. + var baseTransformName = mainTransform.Baseline; + var countSuffix = "." + validTransform.Count.ToString(CultureInfo.InvariantCulture); + + if (PatchConstants.PairedPatchTransformPrefix.Length + baseTransformName.Length + countSuffix.Length > PatchConstants.MaxPatchTransformName) + { + var trimmedTransformName = baseTransformName.Substring(0, PatchConstants.MaxPatchTransformName - PatchConstants.PairedPatchTransformPrefix.Length - countSuffix.Length); + + this.Messaging.Write(WindowsInstallerBackendWarnings.LongPatchBaselineIdTrimmed(baselineSymbol.SourceLineNumbers, baseTransformName, trimmedTransformName)); + baseTransformName = trimmedTransformName; + } + + var transformName = baseTransformName + countSuffix; + subStorages.Add(new SubStorage(transformName, mainTransform.Transform)); transformNames.Add(":" + transformName); - transformNames.Add(":#" + transformName); + + var pairedTransformName = PatchConstants.PairedPatchTransformPrefix + transformName; + subStorages.Add(new SubStorage(pairedTransformName, pairedTransform)); + transformNames.Add(":" + pairedTransformName); } if (validTransform.Count == 0) diff --git a/src/wix/WixToolset.Core.WindowsInstaller/Bind/GetFileFacadesFromTransforms.cs b/src/wix/WixToolset.Core.WindowsInstaller/Bind/GetFileFacadesFromTransforms.cs index 2ac563ac..441038ec 100644 --- a/src/wix/WixToolset.Core.WindowsInstaller/Bind/GetFileFacadesFromTransforms.cs +++ b/src/wix/WixToolset.Core.WindowsInstaller/Bind/GetFileFacadesFromTransforms.cs @@ -40,10 +40,10 @@ namespace WixToolset.Core.WindowsInstaller.Bind //var patchActualFileTable = this.Output.EnsureTable(this.TableDefinitions["File"]); // Index paired transforms by name without their "#" prefix. - var pairedTransforms = this.SubStorages.Where(s => s.Name.StartsWith("#")).ToDictionary(s => s.Name, s => s.Data); + var pairedTransforms = this.SubStorages.Where(s => s.Name.StartsWith(PatchConstants.PairedPatchTransformPrefix)).ToDictionary(s => s.Name, s => s.Data); // Enumerate through main transforms. - foreach (var substorage in this.SubStorages.Where(s => !s.Name.StartsWith("#"))) + foreach (var substorage in this.SubStorages.Where(s => !s.Name.StartsWith(PatchConstants.PairedPatchTransformPrefix))) { var mainTransform = substorage.Data; var mainFileTable = mainTransform.Tables["File"]; @@ -54,7 +54,7 @@ namespace WixToolset.Core.WindowsInstaller.Bind } // Index File table of pairedTransform - var pairedTransform = pairedTransforms["#" + substorage.Name]; + var pairedTransform = pairedTransforms[PatchConstants.PairedPatchTransformPrefix + substorage.Name]; var pairedFileRows = new RowDictionary(pairedTransform.Tables["File"]); foreach (FileRow mainFileRow in mainFileTable.Rows.Where(f => f.Operation != RowOperation.Delete)) diff --git a/src/wix/WixToolset.Core.WindowsInstaller/Bind/PatchConstants.cs b/src/wix/WixToolset.Core.WindowsInstaller/Bind/PatchConstants.cs new file mode 100644 index 00000000..d37c7764 --- /dev/null +++ b/src/wix/WixToolset.Core.WindowsInstaller/Bind/PatchConstants.cs @@ -0,0 +1,11 @@ +// 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. + +namespace WixToolset.Core.WindowsInstaller.Bind +{ + internal static class PatchConstants + { + public const int MaxPatchTransformName = 31; + + public static readonly string PairedPatchTransformPrefix = "#"; + } +} diff --git a/src/wix/WixToolset.Core.WindowsInstaller/Bind/UpdateTransformsWithFileFacades.cs b/src/wix/WixToolset.Core.WindowsInstaller/Bind/UpdateTransformsWithFileFacades.cs index 981fa0a4..ed865450 100644 --- a/src/wix/WixToolset.Core.WindowsInstaller/Bind/UpdateTransformsWithFileFacades.cs +++ b/src/wix/WixToolset.Core.WindowsInstaller/Bind/UpdateTransformsWithFileFacades.cs @@ -52,16 +52,16 @@ namespace WixToolset.Core.WindowsInstaller.Bind var patchMediaRows = new RowDictionary(this.Output.Tables["Media"]); // Index paired transforms by name without the "#" prefix. - var pairedTransforms = this.SubStorages.Where(s => s.Name.StartsWith("#")).ToDictionary(s => s.Name, s => s.Data); + var pairedTransforms = this.SubStorages.Where(s => s.Name.StartsWith(PatchConstants.PairedPatchTransformPrefix)).ToDictionary(s => s.Name, s => s.Data); // Copy File bind data into substorages - foreach (var substorage in this.SubStorages.Where(s => !s.Name.StartsWith("#"))) + foreach (var substorage in this.SubStorages.Where(s => !s.Name.StartsWith(PatchConstants.PairedPatchTransformPrefix))) { var mainTransform = substorage.Data; var mainMsiFileHashIndex = new RowDictionary(mainTransform.Tables["MsiFileHash"]); - var pairedTransform = pairedTransforms["#" + substorage.Name]; + var pairedTransform = pairedTransforms[PatchConstants.PairedPatchTransformPrefix + substorage.Name]; // Copy Media.LastSequence. var pairedMediaTable = pairedTransform.Tables["Media"]; diff --git a/src/wix/WixToolset.Core.WindowsInstaller/WindowsInstallerBackendWarnings.cs b/src/wix/WixToolset.Core.WindowsInstaller/WindowsInstallerBackendWarnings.cs index d0986a4d..bc61c59c 100644 --- a/src/wix/WixToolset.Core.WindowsInstaller/WindowsInstallerBackendWarnings.cs +++ b/src/wix/WixToolset.Core.WindowsInstaller/WindowsInstallerBackendWarnings.cs @@ -6,10 +6,10 @@ namespace WixToolset.Core.WindowsInstaller internal static class WindowsInstallerBackendWarnings { - //public static Message ReplaceThisWithTheFirstWarning(SourceLineNumber sourceLineNumbers) - //{ - // return Message(sourceLineNumbers, Ids.ReplaceThisWithTheFirstWarning, "format string", arg1, arg2); - //} + internal static Message LongPatchBaselineIdTrimmed(SourceLineNumber sourceLineNumbers, string baseTransformName, string trimmedTransformName) + { + 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); + } private static Message Message(SourceLineNumber sourceLineNumber, Ids id, string format, params object[] args) { @@ -18,7 +18,7 @@ namespace WixToolset.Core.WindowsInstaller public enum Ids { - // ReplaceThisWithTheFirstWarning = 7100, + LongPatchBaselineIdTrimmed = 7100, } // last available is 7499. 7500 is WindowsInstallerBackendErrors. } } diff --git a/src/wix/WixToolset.Core/Compiler.cs b/src/wix/WixToolset.Core/Compiler.cs index 7361f501..d04ae491 100644 --- a/src/wix/WixToolset.Core/Compiler.cs +++ b/src/wix/WixToolset.Core/Compiler.cs @@ -7745,10 +7745,6 @@ namespace WixToolset.Core this.Core.Write(ErrorMessages.ExpectedAttribute(sourceLineNumbers, node.Name.LocalName, "Id")); id = Identifier.Invalid; } - else if (27 < id.Id.Length) - { - this.Core.Write(ErrorMessages.IdentifierTooLongError(sourceLineNumbers, node.Name.LocalName, "Id", id.Id, 27)); - } if (!String.IsNullOrEmpty(baselineFile) || !String.IsNullOrEmpty(updateFile)) { diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/PatchFixture.cs b/src/wix/test/WixToolsetTest.CoreIntegration/PatchFixture.cs index 53c9ae4d..a26472fa 100644 --- a/src/wix/test/WixToolsetTest.CoreIntegration/PatchFixture.cs +++ b/src/wix/test/WixToolsetTest.CoreIntegration/PatchFixture.cs @@ -25,7 +25,7 @@ namespace WixToolsetTest.CoreIntegration [Fact] public void CanBuildSimplePatch() { - var folder = TestData.Get(@"TestData\PatchSingle"); + var folder = TestData.Get(@"TestData", "PatchSingle"); using (var fs = new DisposableFileSystem()) { @@ -57,7 +57,7 @@ namespace WixToolsetTest.CoreIntegration [Fact] public void CanBuildSimplePatchWithNoFileChanges() { - var folder = TestData.Get(@"TestData\PatchNoFileChanges"); + var folder = TestData.Get(@"TestData", "PatchNoFileChanges"); using (var fs = new DisposableFileSystem()) { @@ -86,6 +86,31 @@ namespace WixToolsetTest.CoreIntegration } } + [Fact] + public void CanBuildSimplePatchWithBaselineIdTooLong() + { + var folder = TestData.Get(@"TestData", "PatchBaselineIdTooLong"); + + using (var fs = new DisposableFileSystem()) + { + var baseFolder = fs.GetFolder(); + var tempFolderBaseline = Path.Combine(baseFolder, "baseline"); + var tempFolderUpdate = Path.Combine(baseFolder, "update"); + var tempFolderPatch = Path.Combine(baseFolder, "patch"); + + var baselinePdb = BuildMsi("Baseline.msi", folder, tempFolderBaseline, "1.0.0", "1.0.0", "1.0.0"); + var update1Pdb = BuildMsi("Update.msi", folder, tempFolderUpdate, "1.0.1", "1.0.1", "1.0.1"); + var patchPdb = BuildMsp("Patch1.msp", folder, tempFolderPatch, "1.0.1", bindpaths: new[] { Path.GetDirectoryName(baselinePdb), Path.GetDirectoryName(update1Pdb) }, hasNoFiles: true, warningsAsErrors: false); + var patchPath = Path.ChangeExtension(patchPdb, ".msp"); + + var doc = GetExtractPatchXml(patchPath); + WixAssert.StringEqual("{11111111-2222-3333-4444-555555555555}", doc.Root.Element(PatchNamespace + "TargetProductCode").Value); + + var names = Query.GetSubStorageNames(patchPath); + WixAssert.CompareLineByLine(new[] { "#ThisBaseLineIdIsTooLongAndGe.1", "ThisBaseLineIdIsTooLongAndGe.1" }, names); + } + } + [Fact(Skip = "https://github.com/wixtoolset/issues/issues/6387")] public void CanBuildPatchFromProductWithFilesFromWixlib() { @@ -110,7 +135,7 @@ namespace WixToolsetTest.CoreIntegration [Fact] public void CanBuildBundleWithNonSpecificPatches() { - var folder = TestData.Get(@"TestData\PatchNonSpecific"); + var folder = TestData.Get(@"TestData", "PatchNonSpecific"); using (var fs = new DisposableFileSystem()) { @@ -125,23 +150,23 @@ namespace WixToolsetTest.CoreIntegration var bundleBPdb = BuildBundle("BundleB.exe", Path.Combine(folder, "BundleB"), tempFolder); var bundleCPdb = BuildBundle("BundleC.exe", Path.Combine(folder, "BundleC"), tempFolder); - VerifyPatchTargetCodes(bundleAPdb, new[] + VerifyPatchTargetCodesInBurnManifest(bundleAPdb, new[] { "", }); - VerifyPatchTargetCodes(bundleBPdb, new[] + VerifyPatchTargetCodesInBurnManifest(bundleBPdb, new[] { "", "", }); - VerifyPatchTargetCodes(bundleCPdb, new string[0]); + VerifyPatchTargetCodesInBurnManifest(bundleCPdb, new string[0]); } } [Fact] public void CanBuildBundleWithSlipstreamPatch() { - var folder = TestData.Get(@"TestData\PatchSingle"); + var folder = TestData.Get(@"TestData", "PatchSingle"); using (var fs = new DisposableFileSystem()) { @@ -167,20 +192,6 @@ namespace WixToolsetTest.CoreIntegration } } - private static void VerifyPatchTargetCodes(string pdbPath, string[] expected) - { - using (var wixOutput = WixOutput.Read(pdbPath)) - { - var manifestData = wixOutput.GetData(BurnConstants.BurnManifestWixOutputStreamName); - var doc = new XmlDocument(); - doc.LoadXml(manifestData); - var nsmgr = BundleExtractor.GetBurnNamespaceManager(doc, "w"); - var patchTargetCodes = doc.SelectNodes("/w:BurnManifest/w:PatchTargetCode", nsmgr).GetTestXmlLines(); - - WixAssert.CompareLineByLine(expected, patchTargetCodes); - } - } - private static string BuildMsi(string outputName, string sourceFolder, string baseFolder, string defineV, string defineA, string defineB) { var extensionPath = Path.GetFullPath(new Uri(typeof(ExampleExtensionFactory).Assembly.CodeBase).LocalPath); @@ -204,7 +215,7 @@ namespace WixToolsetTest.CoreIntegration return Path.ChangeExtension(outputPath, ".wixpdb"); } - private static string BuildMsp(string outputName, string sourceFolder, string baseFolder, string defineV, IEnumerable bindpaths = null, bool hasNoFiles = false) + private static string BuildMsp(string outputName, string sourceFolder, string baseFolder, string defineV, IEnumerable bindpaths = null, bool hasNoFiles = false, bool warningsAsErrors = true) { var outputPath = Path.Combine(baseFolder, Path.Combine("bin", outputName)); @@ -225,7 +236,7 @@ namespace WixToolsetTest.CoreIntegration args.Add(additionaBindPath); } - var result = WixRunner.Execute(args.ToArray()); + var result = WixRunner.Execute(warningsAsErrors, args.ToArray()); result.AssertSuccess(); @@ -266,6 +277,20 @@ namespace WixToolsetTest.CoreIntegration return XDocument.Parse(buffer.ToString()); } + private static void VerifyPatchTargetCodesInBurnManifest(string pdbPath, string[] expected) + { + using (var wixOutput = WixOutput.Read(pdbPath)) + { + var manifestData = wixOutput.GetData(BurnConstants.BurnManifestWixOutputStreamName); + var doc = new XmlDocument(); + doc.LoadXml(manifestData); + var nsmgr = BundleExtractor.GetBurnNamespaceManager(doc, "w"); + var patchTargetCodes = doc.SelectNodes("/w:BurnManifest/w:PatchTargetCode", nsmgr).GetTestXmlLines(); + + WixAssert.CompareLineByLine(expected, patchTargetCodes); + } + } + [DllImport("msi.dll", EntryPoint = "MsiExtractPatchXMLDataW", CharSet = CharSet.Unicode, ExactSpelling = true)] private static extern int MsiExtractPatchXMLData(string szPatchPath, int dwReserved, StringBuilder szXMLData, ref int pcchXMLData); diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/PatchBaselineIdTooLong/Package.wxs b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/PatchBaselineIdTooLong/Package.wxs new file mode 100644 index 00000000..1c8a34c5 --- /dev/null +++ b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/PatchBaselineIdTooLong/Package.wxs @@ -0,0 +1,28 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/TestData/PatchBaselineIdTooLong/Patch.wxs b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/PatchBaselineIdTooLong/Patch.wxs new file mode 100644 index 00000000..fdad0b87 --- /dev/null +++ b/src/wix/test/WixToolsetTest.CoreIntegration/TestData/PatchBaselineIdTooLong/Patch.wxs @@ -0,0 +1,16 @@ + + + + + + + + + + -- cgit v1.2.3-55-g6feb