Fix when xmlns placed directly on PermissionEx

This is NOT recommended in v4 and its converter will move namespaces to
the root element, but WiX v3 allowed it so we'll continued to do so in
v4.

Fixes 7010

4 files changed, 237 insertions, 18 deletions

diff --git a/src/ext/Util/test/WixToolsetTest.Util/TestData/PermissionEx/PackageComponents.wxs b/src/ext/Util/test/WixToolsetTest.Util/TestData/PermissionEx/PackageComponents.wxs
index 0634d7d4..09f0a724 100644
--- a/src/ext/Util/test/WixToolsetTest.Util/TestData/PermissionEx/PackageComponents.wxs
+++ b/src/ext/Util/test/WixToolsetTest.Util/TestData/PermissionEx/PackageComponents.wxs
@@ -1,4 +1,4 @@
-<Wix xmlns="http://wixtoolset.org/schemas/v4/wxs" xmlns:util="http://wixtoolset.org/schemas/v4/wxs/util">
+<Wix xmlns="http://wixtoolset.org/schemas/v4/wxs" xmlns:util="http://wixtoolset.org/schemas/v4/wxs/util">
     <Fragment>
         <ComponentGroup Id="ProductComponents" Directory="INSTALLFOLDER">
             <Component>
@@ -15,7 +15,12 @@
                     <util:PermissionEx User="Everyone" GenericAll="yes" />
                 </RegistryKey>
                 <RegistryValue Root="HKLM" Key="TestRegistryValueKey" Value="abc">
-                    <util:PermissionEx User="Everyone" GenericAll="yes" />
+                    <!--
+                      Example of placing the namespace on the extension element. This is NOT recommended
+                      in v4 and its converter will move namespaces to the root, but WiX v3 allowed it so
+                      we'll continued to do so for now.
+                    -->
+                    <PermissionEx User="Everyone" GenericAll="yes" xmlns="http://wixtoolset.org/schemas/v4/wxs/util" />
                 </RegistryValue>
             </Component>
         </ComponentGroup>
diff --git a/src/ext/Util/wixext/UtilCompiler.cs b/src/ext/Util/wixext/UtilCompiler.cs
index 47f82ca6..96b2ee0a 100644
--- a/src/ext/Util/wixext/UtilCompiler.cs
+++ b/src/ext/Util/wixext/UtilCompiler.cs
@@ -1286,6 +1286,8 @@ namespace WixToolset.Util
             var bits = new BitArray(32);
             string user = null;
 
+            var validBitNames = new HashSet<string>(UtilConstants.StandardPermissions.Concat(UtilConstants.GenericPermissions).Concat(UtilConstants.FolderPermissions));
+
             foreach (var attrib in element.Attributes())
             {
                 if (String.IsNullOrEmpty(attrib.Name.NamespaceName) || this.Namespace == attrib.Name.Namespace)
@@ -1297,18 +1299,18 @@ namespace WixToolset.Util
                             this.ParseHelper.CreateSimpleReference(section, sourceLineNumbers, UtilSymbolDefinitions.User, user);
                             break;
                         default:
-                            var attribValue = this.ParseHelper.GetAttributeYesNoValue(sourceLineNumbers, attrib);
-                            if (!this.TrySetBitFromName(UtilConstants.StandardPermissions, attrib.Name.LocalName, attribValue, bits, 16))
+                            if (validBitNames.Contains(attrib.Name.LocalName))
                             {
-                                if (!this.TrySetBitFromName(UtilConstants.GenericPermissions, attrib.Name.LocalName, attribValue, bits, 28))
+                                var attribValue = this.ParseHelper.GetAttributeYesNoValue(sourceLineNumbers, attrib);
+                                if (this.TrySetBitFromName(UtilConstants.StandardPermissions, attrib.Name.LocalName, attribValue, bits, 16) ||
+                                    this.TrySetBitFromName(UtilConstants.GenericPermissions, attrib.Name.LocalName, attribValue, bits, 28) ||
+                                    this.TrySetBitFromName(UtilConstants.FolderPermissions, attrib.Name.LocalName, attribValue, bits, 0))
                                 {
-                                    if (!this.TrySetBitFromName(UtilConstants.FolderPermissions, attrib.Name.LocalName, attribValue, bits, 0))
-                                    {
-                                        this.ParseHelper.UnexpectedAttribute(element, attrib);
-                                        break;
-                                    }
+                                    break;
                                 }
                             }
+
+                            this.ParseHelper.UnexpectedAttribute(element, attrib);
                             break;
                     }
                 }
@@ -2477,6 +2479,8 @@ namespace WixToolset.Util
                     break;
             }
 
+            var validBitNames = new HashSet<string>(UtilConstants.StandardPermissions.Concat(UtilConstants.GenericPermissions).Concat(specialPermissions));
+
             foreach (var attrib in element.Attributes())
             {
                 if (String.IsNullOrEmpty(attrib.Name.NamespaceName) || this.Namespace == attrib.Name.Namespace)
@@ -2500,18 +2504,18 @@ namespace WixToolset.Util
                             user = this.ParseHelper.GetAttributeValue(sourceLineNumbers, attrib);
                             break;
                         default:
-                            var attribValue = this.ParseHelper.GetAttributeYesNoValue(sourceLineNumbers, attrib);
-                            if (!this.TrySetBitFromName(UtilConstants.StandardPermissions, attrib.Name.LocalName, attribValue, bits, 16))
+                            if (validBitNames.Contains(attrib.Name.LocalName))
                             {
-                                if (!this.TrySetBitFromName(UtilConstants.GenericPermissions, attrib.Name.LocalName, attribValue, bits, 28))
+                                var attribValue = this.ParseHelper.GetAttributeYesNoValue(sourceLineNumbers, attrib);
+                                if (this.TrySetBitFromName(UtilConstants.StandardPermissions, attrib.Name.LocalName, attribValue, bits, 16) ||
+                                    this.TrySetBitFromName(UtilConstants.GenericPermissions, attrib.Name.LocalName, attribValue, bits, 28) ||
+                                    this.TrySetBitFromName(specialPermissions, attrib.Name.LocalName, attribValue, bits, 0))
                                 {
-                                    if (!this.TrySetBitFromName(specialPermissions, attrib.Name.LocalName, attribValue, bits, 0))
-                                    {
-                                        this.ParseHelper.UnexpectedAttribute(element, attrib);
-                                        break;
-                                    }
+                                    break;
                                 }
                             }
+
+                            this.ParseHelper.UnexpectedAttribute(element, attrib);
                             break;
                     }
                 }
diff --git a/src/wix/WixToolset.Converters/WixConverter.cs b/src/wix/WixToolset.Converters/WixConverter.cs
index 37b99a95..941a5062 100644
--- a/src/wix/WixToolset.Converters/WixConverter.cs
+++ b/src/wix/WixToolset.Converters/WixConverter.cs
@@ -402,6 +402,7 @@ namespace WixToolset.Converters
             // Start converting the nodes at the top.
             this.ConvertNodes(document.Nodes(), 0);
             this.RemoveUnusedNamespaces(document.Root);
+            this.MoveNamespacesToRoot(document.Root);
         }
 
         private void Format(XDocument document)
@@ -424,6 +425,7 @@ namespace WixToolset.Converters
             // Start converting the nodes at the top.
             this.ConvertNodes(document.Nodes(), 0);
             this.RemoveUnusedNamespaces(document.Root);
+            this.MoveNamespacesToRoot(document.Root);
         }
 
         private bool TryOpenSourceFile(string sourceFile, out XDocument document)
@@ -2253,6 +2255,50 @@ namespace WixToolset.Converters
             }
         }
 
+        private void MoveNamespacesToRoot(XElement root)
+        {
+            var rootNamespaces = new HashSet<string>();
+            var rootNamespacePrefixes = new HashSet<string>();
+            var nonRootDeclarations = new List<XAttribute>();
+
+            VisitElement(root, x =>
+            {
+                if (x is XAttribute a && a.IsNamespaceDeclaration)
+                {
+                    if (x.Parent == root)
+                    {
+                        rootNamespaces.Add(a.Value);
+                        rootNamespacePrefixes.Add(a.Name.LocalName);
+                    }
+                    else
+                    {
+                        nonRootDeclarations.Add(a);
+                    }
+                }
+
+                return true;
+            });
+
+            foreach (var declaration in nonRootDeclarations)
+            {
+                if (this.OnInformation(ConverterTestType.MoveNamespacesToRoot, declaration, "Namespace should be defined on the root. The '{0}' namespace was move to the root element.", declaration.Value))
+                {
+                    if (!rootNamespaces.Contains(declaration.Value))
+                    {
+                        var prefix = GetNamespacePrefix(declaration, rootNamespacePrefixes);
+
+                        var rootDeclaration = new XAttribute(XNamespace.Xmlns + prefix, declaration.Value);
+                        root.Add(rootDeclaration);
+
+                        rootNamespaces.Add(rootDeclaration.Value);
+                        rootNamespacePrefixes.Add(rootDeclaration.Name.LocalName);
+                    }
+
+                    declaration.Remove();
+                }
+            }
+        }
+
         private int ReportMessages(XDocument document, bool saved)
         {
             var conversionCount = this.ConversionMessages.Count;
@@ -2457,6 +2503,83 @@ namespace WixToolset.Converters
             return result;
         }
 
+        private static string GetNamespacePrefix(XAttribute declaration, HashSet<string> usedPrefixes)
+        {
+            var baseNamespace = String.Empty;
+
+            switch (declaration.Value)
+            {
+                case "http://wixtoolset.org/schemas/v4/wxs/bal":
+                    baseNamespace = "bal";
+                    break;
+
+                case "http://wixtoolset.org/schemas/v4/wxs/complus":
+                    baseNamespace = "complus";
+                    break;
+
+                case "http://wixtoolset.org/schemas/v4/wxs/dependency":
+                    baseNamespace = "dependency";
+                    break;
+
+                case "http://wixtoolset.org/schemas/v4/wxs/difxapp":
+                    baseNamespace = "difx";
+                    break;
+
+                case "http://wixtoolset.org/schemas/v4/wxs/directx":
+                    baseNamespace = "directx";
+                    break;
+
+                case "http://wixtoolset.org/schemas/v4/wxs/firewall":
+                    baseNamespace = "fw";
+                    break;
+
+                case "http://wixtoolset.org/schemas/v4/wxs/http":
+                    baseNamespace = "http";
+                    break;
+
+                case "http://wixtoolset.org/schemas/v4/wxs/iis":
+                    baseNamespace = "iis";
+                    break;
+
+                case "http://wixtoolset.org/schemas/v4/wxs/msmq":
+                    baseNamespace = "msmq";
+                    break;
+
+                case "http://wixtoolset.org/schemas/v4/wxs/netfx":
+                    baseNamespace = "netfx";
+                    break;
+
+                case "http://wixtoolset.org/schemas/v4/wxs/powershell":
+                    baseNamespace = "ps";
+                    break;
+
+                case "http://wixtoolset.org/schemas/v4/wxs/sql":
+                    baseNamespace = "sql";
+                    break;
+
+                case "http://wixtoolset.org/schemas/v4/wxs/ui":
+                    baseNamespace = "ui";
+                    break;
+
+                case "http://wixtoolset.org/schemas/v4/wxs/util":
+                    baseNamespace = "util";
+                    break;
+
+                case "http://wixtoolset.org/schemas/v4/wxs/vs":
+                    baseNamespace = "vs";
+                    break;
+            }
+
+            var ns = baseNamespace;
+
+            for (var i = 1; usedPrefixes.Contains(ns); ++i)
+            {
+                ns = baseNamespace + i;
+            }
+
+            return ns;
+        }
+
         private static string LowercaseFirstChar(string value)
         {
             if (!String.IsNullOrEmpty(value))
@@ -2928,6 +3051,11 @@ namespace WixToolset.Converters
             /// The MsuPackage element contains obsolete '{0}' attribute. MSU packages are now always permanent because Windows no longer supports silently removing MSUs. The attribute will be removed.
             /// </summary>
             MsuPackagePermanentObsolete,
+
+            /// <summary>
+            /// Namespace should be defined on the root. The '{0}' namespace was move to the root element.
+            /// </summary>
+            MoveNamespacesToRoot,
         }
     }
 }
diff --git a/src/wix/test/WixToolsetTest.Converters/FirewallExtensionFixture.cs b/src/wix/test/WixToolsetTest.Converters/FirewallExtensionFixture.cs
index 3d3fde16..2c1179d7 100644
--- a/src/wix/test/WixToolsetTest.Converters/FirewallExtensionFixture.cs
+++ b/src/wix/test/WixToolsetTest.Converters/FirewallExtensionFixture.cs
@@ -3,6 +3,7 @@
 namespace WixToolsetTest.Converters
 {
     using System;
+    using System.Linq;
     using System.Xml.Linq;
     using WixInternal.TestSupport;
     using WixToolset.Converters;
@@ -43,5 +44,86 @@ namespace WixToolsetTest.Converters
             var actualLines = UnformattedDocumentLines(document);
             WixAssert.CompareLineByLine(expected, actualLines);
         }
+
+        [Fact]
+        public void FixNamespacePlacement()
+        {
+            var parse = String.Join(Environment.NewLine,
+                "<Wix xmlns='http://schemas.microsoft.com/wix/2006/wi'>",
+                "  <Fragment>",
+                "    <RemoteAddress xmlns='http://schemas.microsoft.com/wix/FirewallExtension'>",
+                "      127.0.0.1",
+                "    </RemoteAddress>",
+                "  </Fragment>",
+                "</Wix>");
+
+            var expected = new[]
+            {
+                "<Wix xmlns=\"http://wixtoolset.org/schemas/v4/wxs\" xmlns:fw=\"http://wixtoolset.org/schemas/v4/wxs/firewall\">",
+                "  <Fragment>",
+                "    <fw:RemoteAddress Value=\"127.0.0.1\" />",
+                "  </Fragment>",
+                "</Wix>"
+            };
+
+            var document = XDocument.Parse(parse, LoadOptions.PreserveWhitespace | LoadOptions.SetLineInfo);
+
+            var messaging = new MockMessaging();
+            var converter = new WixConverter(messaging, 2, null, null);
+
+            var errors = converter.ConvertDocument(document);
+            WixAssert.CompareLineByLine(new[]
+            {
+                "[Converted] The namespace 'http://schemas.microsoft.com/wix/2006/wi' is out of date.  It must be 'http://wixtoolset.org/schemas/v4/wxs'. (XmlnsValueWrong)",
+                "[Converted] The namespace 'http://schemas.microsoft.com/wix/FirewallExtension' is out of date.  It must be 'http://wixtoolset.org/schemas/v4/wxs/firewall'. (XmlnsValueWrong)",
+                "[Converted] Using RemoteAddress element text is deprecated. Use the 'Value' attribute instead. (InnerTextDeprecated)",
+                "[Converted] Namespace should be defined on the root. The 'http://wixtoolset.org/schemas/v4/wxs/firewall' namespace was move to the root element. (MoveNamespacesToRoot)"
+            }, messaging.Messages.Select(m => m.ToString()).ToArray());
+            Assert.Equal(4, errors);
+
+            var actualLines = UnformattedDocumentLines(document);
+            WixAssert.CompareLineByLine(expected, actualLines);
+        }
+
+        [Fact]
+        public void FixNamespacePlacementWhenItExists()
+        {
+             //xmlns:abc='http://schemas.microsoft.com/wix/FirewallExtension'
+            var parse = String.Join(Environment.NewLine,
+                "<Wix xmlns='http://schemas.microsoft.com/wix/2006/wi'>",
+                "  <Fragment>",
+                "    <RemoteAddress xmlns='http://schemas.microsoft.com/wix/FirewallExtension'>",
+                "      127.0.0.1",
+                "    </RemoteAddress>",
+                "  </Fragment>",
+                "</Wix>");
+
+            var expected = new[]
+            {
+                "<Wix xmlns=\"http://wixtoolset.org/schemas/v4/wxs\" xmlns:fw=\"http://wixtoolset.org/schemas/v4/wxs/firewall\">",
+                "  <Fragment>",
+                "    <fw:RemoteAddress Value=\"127.0.0.1\" />",
+                "  </Fragment>",
+                "</Wix>"
+            };
+
+            var document = XDocument.Parse(parse, LoadOptions.PreserveWhitespace | LoadOptions.SetLineInfo);
+
+            var messaging = new MockMessaging();
+            var converter = new WixConverter(messaging, 2, null, null);
+
+            var errors = converter.ConvertDocument(document);
+            WixAssert.CompareLineByLine(new[]
+            {
+                "[Converted] The namespace 'http://schemas.microsoft.com/wix/2006/wi' is out of date.  It must be 'http://wixtoolset.org/schemas/v4/wxs'. (XmlnsValueWrong)",
+                "[Converted] The namespace 'http://schemas.microsoft.com/wix/FirewallExtension' is out of date.  It must be 'http://wixtoolset.org/schemas/v4/wxs/firewall'. (XmlnsValueWrong)",
+                "[Converted] Using RemoteAddress element text is deprecated. Use the 'Value' attribute instead. (InnerTextDeprecated)",
+                "[Converted] Namespace should be defined on the root. The 'http://wixtoolset.org/schemas/v4/wxs/firewall' namespace was move to the root element. (MoveNamespacesToRoot)"
+            }, messaging.Messages.Select(m => m.ToString()).ToArray());
+            Assert.Equal(4, errors);
+
+            var actualLines = UnformattedDocumentLines(document);
+            WixAssert.CompareLineByLine(expected, actualLines);
+        }
     }
 }