From e4138d48cf912d7184a78919970c633b1256d3da Mon Sep 17 00:00:00 2001 From: Paul Higinbotham Date: Tue, 16 Nov 2021 11:33:02 -0800 Subject: [PATCH] More code clean up for next release --- src/code/FindHelper.cs | 3 +- src/code/PSResourceInfo.cs | 88 +++++++++++----------------------- src/code/RepositorySettings.cs | 51 +++++++++++--------- src/code/Utils.cs | 22 ++++----- 4 files changed, 69 insertions(+), 95 deletions(-) diff --git a/src/code/FindHelper.cs b/src/code/FindHelper.cs index 143590345..d126df242 100644 --- a/src/code/FindHelper.cs +++ b/src/code/FindHelper.cs @@ -247,7 +247,8 @@ private IEnumerable SearchFromRepository( resourceSearch = repository.GetResourceAsync().GetAwaiter().GetResult(); resourceMetadata = repository.GetResourceAsync().GetAwaiter().GetResult(); } - catch (Exception e){ + catch (Exception e) + { Utils.WriteVerboseOnCmdlet(_cmdletPassedIn, "Error retrieving resource from repository: " + e.Message); } diff --git a/src/code/PSResourceInfo.cs b/src/code/PSResourceInfo.cs index 5d681825c..483662a1a 100644 --- a/src/code/PSResourceInfo.cs +++ b/src/code/PSResourceInfo.cs @@ -1,17 +1,16 @@ -using System.Text.RegularExpressions; -using System.Linq; // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +using NuGet.Versioning; +using NuGet.Protocol.Core.Types; using System; using System.Collections; using System.Collections.Generic; -using Dbg = System.Diagnostics.Debug; using System.Globalization; +using System.Linq; using System.Management.Automation; -using NuGet.Packaging; -using NuGet.Protocol.Core.Types; -using NuGet.Versioning; + +using Dbg = System.Diagnostics.Debug; namespace Microsoft.PowerShell.PowerShellGet.UtilClasses { @@ -20,9 +19,6 @@ namespace Microsoft.PowerShell.PowerShellGet.UtilClasses [Flags] public enum ResourceType { - // 00001 -> M - // 00100 -> C - // 00101 -> M, C None = 0x0, Module = 0x1, Script = 0x2, @@ -204,18 +200,18 @@ public sealed class PSCommandResourceInfo { // this object will represent a Command or DSCResource // included by the PSResourceInfo property - #region Properties + public string Name { get; } public PSResourceInfo ParentResource { get; } + #endregion #region Constructor /// /// Constructor - /// /// /// Name of the command or DSC resource /// the parent module resource the command or dsc resource belongs to @@ -325,6 +321,7 @@ private PSResourceInfo( #endregion #region Private fields + private static readonly char[] Delimeter = {' ', ','}; #endregion @@ -498,7 +495,6 @@ private static Version GetVersionInfo( return GetProperty(nameof(PSResourceInfo.Version), psObjectInfo); } - public static bool TryConvert( IPackageSearchMetadata metadataToParse, out PSResourceInfo psGetInfo, @@ -620,38 +616,11 @@ private static T GetProperty( } } - private static string GetPrereleaseLabel(Version version) - { - string versionAsString = version.ToString(); - - if (!versionAsString.Contains("-")) - { - // no prerelease label present - return String.Empty; - } - - string[] prereleaseParsed = versionAsString.Split('-'); - if (prereleaseParsed.Length <= 1) - { - return String.Empty; - } - - string prereleaseString = prereleaseParsed[1]; - Regex prereleasePattern = new Regex("^[a-zA-Z0-9]+$"); - if (!prereleasePattern.IsMatch(prereleaseString)) - { - return String.Empty; - } - - return prereleaseString; - } - private static Dependency[] GetDependencies(ArrayList dependencyInfos) { List dependenciesFound = new List(); if (dependencyInfos == null) { return dependenciesFound.ToArray(); } - foreach(PSObject dependencyObj in dependencyInfos) { // The dependency object can be a string or a hashtable @@ -734,14 +703,12 @@ private static Dependency[] GetDependencies(ArrayList dependencyInfos) else if (dependencyObj.Properties["Name"] != null) { string name = dependencyObj.Properties["Name"].Value.ToString(); - - string version = string.Empty; VersionRange versionRange = VersionRange.All; - if (dependencyObj.Properties["VersionRange"] != null) { - version = dependencyObj.Properties["VersionRange"].Value.ToString(); - VersionRange.TryParse(version, out versionRange); + VersionRange.TryParse( + dependencyObj.Properties["VersionRange"].Value.ToString(), + out versionRange); } dependenciesFound.Add(new Dependency(name, versionRange)); @@ -756,7 +723,6 @@ private static string ConcatenateVersionWithPrerelease(string version, string pr return Utils.GetNormalizedVersionString(version, prerelease); } - #region Parse Metadata private static methods private static string ParseMetadataAuthor(IPackageSearchMetadata pkg) @@ -782,10 +748,11 @@ private static Dependency[] ParseMetadataDependencies(IPackageSearchMetadata pkg depVersionRange = pkgDependencyItem.VersionRange; } - Dependency currentDependency = new Dependency(pkgDependencyItem.Id, depVersionRange); - dependencies.Add(currentDependency); + dependencies.Add( + new Dependency(pkgDependencyItem.Id, depVersionRange)); } } + return dependencies.ToArray(); } @@ -828,13 +795,12 @@ private static Uri ParseMetadataProjectUri(IPackageSearchMetadata pkg) private static DateTime? ParseMetadataPublishedDate(IPackageSearchMetadata pkg) { - DateTime? publishDate = null; - DateTimeOffset? pkgPublishedDate = pkg.Published; - if (pkgPublishedDate.HasValue) + if (pkg.Published.HasValue) { - publishDate = pkgPublishedDate.Value.DateTime; + return pkg.Published.Value.DateTime; } - return publishDate; + + return null; } private static string[] ParseMetadataTags(IPackageSearchMetadata pkg) @@ -842,11 +808,12 @@ private static string[] ParseMetadataTags(IPackageSearchMetadata pkg) return pkg.Tags.Split(Delimeter, StringSplitOptions.RemoveEmptyEntries); } - private static ResourceType ParseMetadataType(IPackageSearchMetadata pkg, - string repoName, - ResourceType? pkgType, - out ArrayList commandNames, - out ArrayList dscResourceNames) + private static ResourceType ParseMetadataType( + IPackageSearchMetadata pkg, + string repoName, + ResourceType? pkgType, + out ArrayList commandNames, + out ArrayList dscResourceNames) { // possible type combinations: // M, C @@ -875,7 +842,7 @@ private static ResourceType ParseMetadataType(IPackageSearchMetadata pkg, // if Name contains wildcard, currently Script and Module tags should be set properly, but need to account for Command and DscResource types too // if Name does not contain wildcard, GetMetadataAsync() was used, PSGallery only is searched (and pkg will successfully be found // and returned from there) before PSGalleryScripts can be searched - foreach(string tag in tags) + foreach (string tag in tags) { if(String.Equals(tag, "PSScript", StringComparison.InvariantCultureIgnoreCase)) { @@ -883,11 +850,13 @@ private static ResourceType ParseMetadataType(IPackageSearchMetadata pkg, currentPkgType &= ~ResourceType.Module; currentPkgType |= ResourceType.Script; } + if (tag.StartsWith("PSCommand_", StringComparison.InvariantCultureIgnoreCase)) { currentPkgType |= ResourceType.Command; commandNames.Add(tag.Split('_')[1]); } + if (tag.StartsWith("PSDscResource_", StringComparison.InvariantCultureIgnoreCase)) { currentPkgType |= ResourceType.DscResource; @@ -904,6 +873,7 @@ private static Version ParseMetadataVersion(IPackageSearchMetadata pkg) { return pkg.Identity.Version.Version; } + return null; } @@ -999,7 +969,7 @@ public static void WritePSGetResourceInfo( { if (psObjectGetInfo.BaseObject is PSResourceInfo psGetInfo) { - if (! psGetInfo.TryWrite(filePath, out string errorMsg)) + if (!psGetInfo.TryWrite(filePath, out string errorMsg)) { throw new PSInvalidOperationException(errorMsg); } diff --git a/src/code/RepositorySettings.cs b/src/code/RepositorySettings.cs index 511c9d651..a51ab4416 100644 --- a/src/code/RepositorySettings.cs +++ b/src/code/RepositorySettings.cs @@ -7,17 +7,15 @@ using System.IO; using System.Linq; using System.Management.Automation; +using System.Xml; using System.Xml.Linq; -using Dbg = System.Diagnostics.Debug; - namespace Microsoft.PowerShell.PowerShellGet.UtilClasses { /// /// The class contains basic information of a repository path settings as well as methods to - /// perform CRUD operations on the repository store file. + /// perform Create/Read/Update/Delete operations on the repository store file. /// - internal static class RepositorySettings { #region Members @@ -52,8 +50,7 @@ public static void CheckRepositoryStore() } XDocument newRepoXML = new XDocument( - new XElement("configuration") - ); + new XElement("configuration")); newRepoXML.Save(FullRepositoryPath); } catch (Exception e) @@ -69,7 +66,7 @@ public static void CheckRepositoryStore() // Open file (which should exist now), if cannot/is corrupted then throw error try { - XDocument.Load(FullRepositoryPath); + LoadXDocument(FullRepositoryPath); } catch (Exception e) { @@ -84,13 +81,10 @@ public static void CheckRepositoryStore() /// public static PSRepositoryInfo Add(string repoName, Uri repoURL, int repoPriority, bool repoTrusted) { - Dbg.Assert(!string.IsNullOrEmpty(repoName), "Repository name cannot be null or empty"); - Dbg.Assert(!string.IsNullOrEmpty(repoURL.ToString()), "Repository URL cannot be null or empty"); - try { // Open file - XDocument doc = XDocument.Load(FullRepositoryPath); + XDocument doc = LoadXDocument(FullRepositoryPath); if (FindRepositoryElement(doc, repoName) != null) { throw new PSInvalidOperationException(String.Format("The PSResource Repository '{0}' already exists.", repoName)); @@ -128,13 +122,11 @@ public static PSRepositoryInfo Add(string repoName, Uri repoURL, int repoPriorit /// public static PSRepositoryInfo Update(string repoName, Uri repoURL, int repoPriority, bool? repoTrusted) { - Dbg.Assert(!string.IsNullOrEmpty(repoName), "Repository name cannot be null or empty"); - PSRepositoryInfo updatedRepo; try { // Open file - XDocument doc = XDocument.Load(FullRepositoryPath); + XDocument doc = LoadXDocument(FullRepositoryPath); XElement node = FindRepositoryElement(doc, repoName); if (node == null) { @@ -196,18 +188,11 @@ public static PSRepositoryInfo Update(string repoName, Uri repoURL, int repoPrio public static void Remove(string[] repoNames, out string[] errorList) { List tempErrorList = new List(); - - // Check to see if information we're trying to remove from the repository is valid - if (repoNames == null || repoNames.Length == 0) - { - throw new ArgumentException("Repository name cannot be null or empty"); - } - XDocument doc; try { // Open file - doc = XDocument.Load(FullRepositoryPath); + doc = LoadXDocument(FullRepositoryPath); } catch (Exception e) { @@ -244,7 +229,7 @@ public static List Read(string[] repoNames, out string[] error try { // Open file - doc = XDocument.Load(FullRepositoryPath); + doc = LoadXDocument(FullRepositoryPath); } catch (Exception e) { @@ -323,6 +308,26 @@ private static XElement FindRepositoryElement(XDocument doc, string name) StringComparison.InvariantCultureIgnoreCase)).FirstOrDefault(); } + private static readonly XmlReaderSettings XDocReaderSettings = new XmlReaderSettings() + { + DtdProcessing = DtdProcessing.Prohibit, // Disallow any DTD elements + XmlResolver = null, // Do not resolve external links + CheckCharacters = true, + IgnoreComments = true, + IgnoreProcessingInstructions = true, + IgnoreWhitespace = true, + MaxCharactersFromEntities = 1024, + MaxCharactersInDocument = 512 * 1024 * 1024, // 512M characters = 1GB + ValidationFlags = System.Xml.Schema.XmlSchemaValidationFlags.None, + ValidationType = ValidationType.None + }; + + private static XDocument LoadXDocument(string filePath) + { + using var xmlReader = XmlReader.Create(filePath, XDocReaderSettings); + return XDocument.Load(xmlReader); + } + #endregion } } diff --git a/src/code/Utils.cs b/src/code/Utils.cs index 307d5569a..6d6496fdb 100644 --- a/src/code/Utils.cs +++ b/src/code/Utils.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +using NuGet.Versioning; using System; using System.Collections; using System.Collections.Generic; @@ -9,7 +10,6 @@ using System.Management.Automation; using System.Management.Automation.Language; using System.Runtime.InteropServices; -using NuGet.Versioning; namespace Microsoft.PowerShell.PowerShellGet.UtilClasses { @@ -138,7 +138,6 @@ public static string GetNormalizedVersionString( // versionString: "1.2.0" prerelease: "alpha1" return versionString + "-" + prerelease; } - else if (numVersionDigits == 4) { // versionString: "1.2.0.0" prerelease: "alpha1" @@ -229,8 +228,7 @@ public static bool TryCreateValidUrl( string uriString, PSCmdlet cmdletPassedIn, out Uri uriResult, - out ErrorRecord errorRecord - ) + out ErrorRecord errorRecord) { errorRecord = null; if (Uri.TryCreate(uriString, UriKind.Absolute, out uriResult)) @@ -241,7 +239,7 @@ out ErrorRecord errorRecord Exception ex; try { - // This is needed for a relative path urlstring. Does not throw error for an absolute path + // This is needed for a relative path urlstring. Does not throw error for an absolute path. var filePath = cmdletPassedIn.SessionState.Path.GetResolvedPSPathFromPSPath(uriString)[0].Path; if (Uri.TryCreate(filePath, UriKind.Absolute, out uriResult)) { @@ -306,12 +304,10 @@ public static string GetInstalledPackageName(string pkgPath) // ex: ./PowerShell/Scripts/TestScript.ps1 return Path.GetFileNameWithoutExtension(pkgPath); } - else - { - // expecting the full version module path - // ex: ./PowerShell/Modules/TestModule/1.0.0 - return new DirectoryInfo(pkgPath).Parent.Name; - } + + // expecting the full version module path + // ex: ./PowerShell/Modules/TestModule/1.0.0 + return new DirectoryInfo(pkgPath).Parent.Name; } public static List GetAllResourcePaths( @@ -388,7 +384,9 @@ public static List GetAllResourcePaths( } // Find all potential installation paths given a scope - public static List GetAllInstallationPaths(PSCmdlet psCmdlet, ScopeType scope) + public static List GetAllInstallationPaths( + PSCmdlet psCmdlet, + ScopeType scope) { GetStandardPlatformPaths( psCmdlet,