From 2ff1ab30850c10790bd3738c210cb6669fe6f298 Mon Sep 17 00:00:00 2001 From: Quoc Truong Date: Mon, 20 Jul 2015 15:21:33 -0700 Subject: [PATCH] Checks for UTF8 encoding in help file --- Engine/Helper.cs | 11 ++ Engine/ScriptAnalyzer.cs | 121 +++++++++++------- Rules/ScriptAnalyzerBuiltinRules.csproj | 1 + Rules/Strings.Designer.cs | 38 +++++- Rules/Strings.resx | 12 ++ Rules/UseUTF8EncodingForHelpFile.cs | 99 ++++++++++++++ .../UseUTF8EncodingForHelpFile.tests.ps1 | 30 +++++ Tests/Rules/about_utf16.help.txt | Bin 0 -> 2120 bytes Tests/Rules/about_utf8.help.txt | 17 +++ Tests/Rules/utf16.txt | Bin 0 -> 2120 bytes 10 files changed, 281 insertions(+), 48 deletions(-) create mode 100644 Rules/UseUTF8EncodingForHelpFile.cs create mode 100644 Tests/Rules/UseUTF8EncodingForHelpFile.tests.ps1 create mode 100644 Tests/Rules/about_utf16.help.txt create mode 100644 Tests/Rules/about_utf8.help.txt create mode 100644 Tests/Rules/utf16.txt diff --git a/Engine/Helper.cs b/Engine/Helper.cs index 347ebc521..719ddfa23 100644 --- a/Engine/Helper.cs +++ b/Engine/Helper.cs @@ -224,6 +224,17 @@ public bool IsDscResourceModule(string filePath) return false; } + /// + /// Given a filePath. Returns true if it is a powershell help file + /// + /// + /// + public bool IsHelpFile(string filePath) + { + return filePath != null && File.Exists(filePath) && Path.GetFileName(filePath).StartsWith("about_", StringComparison.OrdinalIgnoreCase) + && Path.GetFileName(filePath).EndsWith(".help.txt", StringComparison.OrdinalIgnoreCase); + } + /// /// Given an AST, checks whether dsc resource is class based or not /// diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index 55b11ebb1..e3cdf8cc7 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -908,9 +908,9 @@ private void BuildScriptPathList( bool searchRecursively, IList scriptFilePaths) { - const string ps1Suffix = "ps1"; - const string psm1Suffix = "psm1"; - const string psd1Suffix = "psd1"; + const string ps1Suffix = ".ps1"; + const string psm1Suffix = ".psm1"; + const string psd1Suffix = ".psd1"; if (Directory.Exists(path)) { @@ -935,9 +935,14 @@ private void BuildScriptPathList( } else if (File.Exists(path)) { - if ((path.Length > ps1Suffix.Length && path.Substring(path.Length - ps1Suffix.Length).Equals(ps1Suffix, StringComparison.OrdinalIgnoreCase)) || - (path.Length > psm1Suffix.Length && path.Substring(path.Length - psm1Suffix.Length).Equals(psm1Suffix, StringComparison.OrdinalIgnoreCase)) || - (path.Length > psd1Suffix.Length && path.Substring(path.Length - psd1Suffix.Length).Equals(psd1Suffix, StringComparison.OrdinalIgnoreCase))) + String fileName = Path.GetFileName(path); + if ((fileName.Length > ps1Suffix.Length && String.Equals(Path.GetExtension(path), ps1Suffix, StringComparison.OrdinalIgnoreCase)) || + (fileName.Length > psm1Suffix.Length && String.Equals(Path.GetExtension(path), psm1Suffix, StringComparison.OrdinalIgnoreCase)) || + (fileName.Length > psd1Suffix.Length && String.Equals(Path.GetExtension(path), psd1Suffix, StringComparison.OrdinalIgnoreCase))) + { + scriptFilePaths.Add(path); + } + else if (Helper.Instance.IsHelpFile(path)) { scriptFilePaths.Add(path); } @@ -964,7 +969,28 @@ private IEnumerable AnalyzeFile(string filePath) //Parse the file if (File.Exists(filePath)) { - scriptAst = Parser.ParseFile(filePath, out scriptTokens, out errors); + // processing for non help script + if (!(Path.GetFileName(filePath).StartsWith("about_") && Path.GetFileName(filePath).EndsWith(".help.txt"))) + { + scriptAst = Parser.ParseFile(filePath, out scriptTokens, out errors); + + if (errors != null && errors.Length > 0) + { + foreach (ParseError error in errors) + { + string parseErrorMessage = String.Format(CultureInfo.CurrentCulture, Strings.ParserErrorFormat, error.Extent.File, error.Message.TrimEnd('.'), error.Extent.StartLineNumber, error.Extent.StartColumnNumber); + this.outputWriter.WriteError(new ErrorRecord(new ParseException(parseErrorMessage), parseErrorMessage, ErrorCategory.ParserError, error.ErrorId)); + } + } + + if (errors.Length > 10) + { + string manyParseErrorMessage = String.Format(CultureInfo.CurrentCulture, Strings.ParserErrorMessage, System.IO.Path.GetFileName(filePath)); + this.outputWriter.WriteError(new ErrorRecord(new ParseException(manyParseErrorMessage), manyParseErrorMessage, ErrorCategory.ParserError, filePath)); + + return new List(); + } + } } else { @@ -975,23 +1001,6 @@ private IEnumerable AnalyzeFile(string filePath) return null; } - if (errors != null && errors.Length > 0) - { - foreach (ParseError error in errors) - { - string parseErrorMessage = String.Format(CultureInfo.CurrentCulture, Strings.ParserErrorFormat, error.Extent.File, error.Message.TrimEnd('.'), error.Extent.StartLineNumber, error.Extent.StartColumnNumber); - this.outputWriter.WriteError(new ErrorRecord(new ParseException(parseErrorMessage), parseErrorMessage, ErrorCategory.ParserError, error.ErrorId)); - } - } - - if (errors.Length > 10) - { - string manyParseErrorMessage = String.Format(CultureInfo.CurrentCulture, Strings.ParserErrorMessage, System.IO.Path.GetFileName(filePath)); - this.outputWriter.WriteError(new ErrorRecord(new ParseException(manyParseErrorMessage), manyParseErrorMessage, ErrorCategory.ParserError, filePath)); - - return new List(); - } - return this.AnalyzeSyntaxTree(scriptAst, scriptTokens, filePath); } @@ -1007,7 +1016,7 @@ public IEnumerable AnalyzeSyntaxTree( Token[] scriptTokens, string filePath) { - Dictionary> ruleSuppressions; + Dictionary> ruleSuppressions = new Dictionary>(); ConcurrentBag diagnostics = new ConcurrentBag(); ConcurrentBag suppressed = new ConcurrentBag(); BlockingCollection> verboseOrErrors = new BlockingCollection>(); @@ -1015,28 +1024,33 @@ public IEnumerable AnalyzeSyntaxTree( // Use a List of KVP rather than dictionary, since for a script containing inline functions with same signature, keys clash List> cmdInfoTable = new List>(); - ruleSuppressions = Helper.Instance.GetRuleSuppression(scriptAst); + bool helpFile = (scriptAst == null) && Helper.Instance.IsHelpFile(filePath); - foreach (List ruleSuppressionsList in ruleSuppressions.Values) + if (!helpFile) { - foreach (RuleSuppression ruleSuppression in ruleSuppressionsList) + ruleSuppressions = Helper.Instance.GetRuleSuppression(scriptAst); + + foreach (List ruleSuppressionsList in ruleSuppressions.Values) { - if (!String.IsNullOrWhiteSpace(ruleSuppression.Error)) + foreach (RuleSuppression ruleSuppression in ruleSuppressionsList) { - this.outputWriter.WriteError(new ErrorRecord(new ArgumentException(ruleSuppression.Error), ruleSuppression.Error, ErrorCategory.InvalidArgument, ruleSuppression)); + if (!String.IsNullOrWhiteSpace(ruleSuppression.Error)) + { + this.outputWriter.WriteError(new ErrorRecord(new ArgumentException(ruleSuppression.Error), ruleSuppression.Error, ErrorCategory.InvalidArgument, ruleSuppression)); + } } } - } - #region Run VariableAnalysis - try - { - Helper.Instance.InitializeVariableAnalysis(scriptAst); - } - catch { } - #endregion + #region Run VariableAnalysis + try + { + Helper.Instance.InitializeVariableAnalysis(scriptAst); + } + catch { } + #endregion - Helper.Instance.Tokens = scriptTokens; + Helper.Instance.Tokens = scriptTokens; + } #region Run ScriptRules //Trim down to the leaf element of the filePath and pass it to Diagnostic Record @@ -1067,6 +1081,8 @@ public IEnumerable AnalyzeSyntaxTree( } } + bool helpRule = String.Equals(scriptRule.GetName(), "PSUseUTF8EncodingForHelpFile", StringComparison.OrdinalIgnoreCase); + if ((includeRule == null || includeRegexMatch) && (excludeRule == null || !excludeRegexMatch)) { List result = new List(); @@ -1077,14 +1093,25 @@ public IEnumerable AnalyzeSyntaxTree( // We want the Engine to continue functioning even if one or more Rules throws an exception try { - var records = Helper.Instance.SuppressRule(scriptRule.GetName(), ruleSuppressions, scriptRule.AnalyzeScript(scriptAst, scriptAst.Extent.File).ToList()); - foreach (var record in records.Item2) + if (helpRule && helpFile) { - diagnostics.Add(record); + var records = scriptRule.AnalyzeScript(scriptAst, filePath); + foreach (var record in records) + { + diagnostics.Add(record); + } } - foreach (var suppressedRec in records.Item1) + else if (!helpRule && !helpFile) { - suppressed.Add(suppressedRec); + var records = Helper.Instance.SuppressRule(scriptRule.GetName(), ruleSuppressions, scriptRule.AnalyzeScript(scriptAst, scriptAst.Extent.File).ToList()); + foreach (var record in records.Item2) + { + diagnostics.Add(record); + } + foreach (var suppressedRec in records.Item1) + { + suppressed.Add(suppressedRec); + } } } catch (Exception scriptRuleException) @@ -1122,7 +1149,7 @@ public IEnumerable AnalyzeSyntaxTree( #region Run Token Rules - if (this.TokenRules != null) + if (this.TokenRules != null && !helpFile) { foreach (ITokenRule tokenRule in this.TokenRules) { @@ -1173,7 +1200,7 @@ public IEnumerable AnalyzeSyntaxTree( #endregion #region DSC Resource Rules - if (this.DSCResourceRules != null) + if (this.DSCResourceRules != null && !helpFile) { // Invoke AnalyzeDSCClass only if the ast is a class based resource if (Helper.Instance.IsDscResourceClassBased(scriptAst)) @@ -1282,7 +1309,7 @@ public IEnumerable AnalyzeSyntaxTree( #region Run External Rules - if (this.ExternalRules != null) + if (this.ExternalRules != null && !helpFile) { List exRules = new List(); diff --git a/Rules/ScriptAnalyzerBuiltinRules.csproj b/Rules/ScriptAnalyzerBuiltinRules.csproj index 32697b07f..afd0ba292 100644 --- a/Rules/ScriptAnalyzerBuiltinRules.csproj +++ b/Rules/ScriptAnalyzerBuiltinRules.csproj @@ -91,6 +91,7 @@ + diff --git a/Rules/Strings.Designer.cs b/Rules/Strings.Designer.cs index 40e597258..fcbacb9c2 100644 --- a/Rules/Strings.Designer.cs +++ b/Rules/Strings.Designer.cs @@ -1,7 +1,7 @@ //------------------------------------------------------------------------------ // // This code was generated by a tool. -// Runtime Version:4.0.30319.42000 +// Runtime Version:4.0.30319.34014 // // Changes to this file may cause incorrect behavior and will be lost if // the code is regenerated. @@ -1869,6 +1869,42 @@ internal static string UseTypeAtVariableAssignmentName { } } + /// + /// Looks up a localized string similar to Use UTF8 Encoding For Help File. + /// + internal static string UseUTF8EncodingForHelpFileCommonName { + get { + return ResourceManager.GetString("UseUTF8EncodingForHelpFileCommonName", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to PowerShell help file needs to use UTF8 Encoding.. + /// + internal static string UseUTF8EncodingForHelpFileDescription { + get { + return ResourceManager.GetString("UseUTF8EncodingForHelpFileDescription", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to File {0} has to use UTF8 instead of {1} encoding because it is a powershell help file.. + /// + internal static string UseUTF8EncodingForHelpFileError { + get { + return ResourceManager.GetString("UseUTF8EncodingForHelpFileError", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to UseUTF8EncodingForHelpFile. + /// + internal static string UseUTF8EncodingForHelpFileName { + get { + return ResourceManager.GetString("UseUTF8EncodingForHelpFileName", resourceCulture); + } + } + /// /// Looks up a localized string similar to Use verbose message in DSC resource. /// diff --git a/Rules/Strings.resx b/Rules/Strings.resx index fe00a5328..c9ea57bdf 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -732,4 +732,16 @@ AvoidUsingDeprecatedManifestFields + + Use UTF8 Encoding For Help File + + + PowerShell help file needs to use UTF8 Encoding. + + + File {0} has to use UTF8 instead of {1} encoding because it is a powershell help file. + + + UseUTF8EncodingForHelpFile + \ No newline at end of file diff --git a/Rules/UseUTF8EncodingForHelpFile.cs b/Rules/UseUTF8EncodingForHelpFile.cs new file mode 100644 index 000000000..7f301abc8 --- /dev/null +++ b/Rules/UseUTF8EncodingForHelpFile.cs @@ -0,0 +1,99 @@ +// +// Copyright (c) Microsoft Corporation. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. +// + +using System; +using System.Collections.Generic; +using System.Management.Automation.Language; +using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic; +using System.ComponentModel.Composition; +using System.Globalization; + +namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules +{ + /// + /// AvoidAlias: Check if help file uses utf8 encoding + /// + [Export(typeof(IScriptRule))] + public class UseUTF8EncodingForHelpFile : IScriptRule + { + /// + /// AnalyzeScript: check if the help file uses something other than utf8 + /// + public IEnumerable AnalyzeScript(Ast ast, string fileName) + { + if (!String.IsNullOrWhiteSpace(fileName) && Helper.Instance.IsHelpFile(fileName)) + { + using (var reader = new System.IO.StreamReader(fileName, true)) + { + reader.ReadToEnd(); + if (reader.CurrentEncoding != System.Text.Encoding.UTF8) + { + yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.UseUTF8EncodingForHelpFileError, System.IO.Path.GetFileName(fileName), reader.CurrentEncoding), + null, GetName(), DiagnosticSeverity.Warning, fileName); + } + } + } + } + + /// + /// GetName: Retrieves the name of this rule. + /// + /// The name of this rule + public string GetName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.UseUTF8EncodingForHelpFileName); + } + + /// + /// GetCommonName: Retrieves the common name of this rule. + /// + /// The common name of this rule + public string GetCommonName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.UseUTF8EncodingForHelpFileCommonName); + } + + /// + /// GetDescription: Retrieves the description of this rule. + /// + /// The description of this rule + public string GetDescription() + { + return string.Format(CultureInfo.CurrentCulture, Strings.UseUTF8EncodingForHelpFileDescription); + } + + /// + /// GetSourceType: Retrieves the type of the rule, Builtin, Managed or Module. + /// + public SourceType GetSourceType() + { + return SourceType.Builtin; + } + + /// + /// GetSeverity: Retrieves the severity of the rule: error, warning of information. + /// + /// + public RuleSeverity GetSeverity() + { + return RuleSeverity.Warning; + } + + /// + /// GetSourceName: Retrieves the name of the module/assembly the rule is from. + /// + public string GetSourceName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.SourceName); + } + } +} diff --git a/Tests/Rules/UseUTF8EncodingForHelpFile.tests.ps1 b/Tests/Rules/UseUTF8EncodingForHelpFile.tests.ps1 new file mode 100644 index 000000000..d42d57f3b --- /dev/null +++ b/Tests/Rules/UseUTF8EncodingForHelpFile.tests.ps1 @@ -0,0 +1,30 @@ +Import-Module PSScriptAnalyzer +$violationMessage = "File about_utf16.help.txt has to use UTF8 instead of System.Text.UTF32Encoding encoding because it is a powershell help file." +$violationName = "PSUseUTF8EncodingForHelpFile" +$directory = Split-Path -Parent $MyInvocation.MyCommand.Path +$violations = Invoke-ScriptAnalyzer $directory\about_utf16.help.txt | Where-Object {$_.RuleName -eq $violationName} +$noViolations = Invoke-ScriptAnalyzer $directory\about_utf8.help.txt | Where-Object {$_.RuleName -eq $violationName} +$notHelpFileViolations = Invoke-ScriptAnalyzer $directory\utf16.txt | Where-Object {$_.RuleName -eq $violationName} + +Describe "UseUTF8EncodingForHelpFile" { + Context "When there are violations" { + It "has 1 avoid use utf8 encoding violation" { + $violations.Count | Should Be 1 + } + + It "has the correct description message" { + $violations[0].Message | Should Match $violationMessage + } + + } + + Context "When there are no violations" { + It "returns no violations for correct utf8 help file" { + $noViolations.Count | Should Be 0 + } + + It "returns no violations for utf16 file that is not a help file" { + $notHelpFileViolations.Count | Should Be 0 + } + } +} \ No newline at end of file diff --git a/Tests/Rules/about_utf16.help.txt b/Tests/Rules/about_utf16.help.txt new file mode 100644 index 0000000000000000000000000000000000000000..23be5e142492d3fa0c2e92a5a20747c27e5ca3aa GIT binary patch literal 2120 zcmb`{+lo^`5QX79NMwz#wHuV5|LUi;Lm@$DLNqo#(yD z{zqXQ9)$DY4)}MW77+Ux%+MKh^&DxdFzf2~`D4WToygPM+>hA&rY`HjUT2sKd+Ch1 zjx+u>b2BtkJ<-rJ=c_<_pZ6|y_vsCJ%N5Ll9yR)Zq<;tM%yYk&wK+Zx55qQG1aEy8 zYJq)x>Nvw(T0`rq_-J>g@6w&LR{fYy>tk2=71Yyz1?R|l738Q}fevp#&+P8EV$U`D ze&&1TYwhYeuZB;pLy)6p>vBKGeTvDiK%?H~JlfUjnb_a`cYckyT71X4^v6%je1Gm} zGpKym!S9BD8)|`leDsxhr$4pm%xwCVNdg9b&C>?ziH%?ziiVo{<_Ay4E4K z|FoWi`|bWtW7E=iYY%;$JHLN=)S?agac+g<-wa>ZV9)xj{SG=#n_6mgujiA`4CpfN S?vX}RpsQzVKU>Vc?(;8IM7Ed! literal 0 HcmV?d00001 diff --git a/Tests/Rules/about_utf8.help.txt b/Tests/Rules/about_utf8.help.txt new file mode 100644 index 000000000..a06b616af --- /dev/null +++ b/Tests/Rules/about_utf8.help.txt @@ -0,0 +1,17 @@ +TOPIC + about_ + +SHORT DESCRIPTION + A short, one-line description of the topic contents. + +LONG DESCRIPTION + A detailed, full description of the subject or purpose of the module. + +EXAMPLES + Examples of how to use the module or how the subject feature works in practice. + +KEYWORDS + Terms or titles on which you might expect your users to search for the information in this topic. + +SEE ALSO + Text-only references for further reading. Hyperlinks cannot work in the Windows PowerShell console. \ No newline at end of file diff --git a/Tests/Rules/utf16.txt b/Tests/Rules/utf16.txt new file mode 100644 index 0000000000000000000000000000000000000000..23be5e142492d3fa0c2e92a5a20747c27e5ca3aa GIT binary patch literal 2120 zcmb`{+lo^`5QX79NMwz#wHuV5|LUi;Lm@$DLNqo#(yD z{zqXQ9)$DY4)}MW77+Ux%+MKh^&DxdFzf2~`D4WToygPM+>hA&rY`HjUT2sKd+Ch1 zjx+u>b2BtkJ<-rJ=c_<_pZ6|y_vsCJ%N5Ll9yR)Zq<;tM%yYk&wK+Zx55qQG1aEy8 zYJq)x>Nvw(T0`rq_-J>g@6w&LR{fYy>tk2=71Yyz1?R|l738Q}fevp#&+P8EV$U`D ze&&1TYwhYeuZB;pLy)6p>vBKGeTvDiK%?H~JlfUjnb_a`cYckyT71X4^v6%je1Gm} zGpKym!S9BD8)|`leDsxhr$4pm%xwCVNdg9b&C>?ziH%?ziiVo{<_Ay4E4K z|FoWi`|bWtW7E=iYY%;$JHLN=)S?agac+g<-wa>ZV9)xj{SG=#n_6mgujiA`4CpfN S?vX}RpsQzVKU>Vc?(;8IM7Ed! literal 0 HcmV?d00001