Skip to content

Commit

Permalink
Include validation analyzer (#7071)
Browse files Browse the repository at this point in the history 
* Update Errors.cs

* include analyzer package

* update version

* upgrade version

* Minor clean up

* update version

* merge

* update version

* restore analyzer severity level

* fix analyzer error of Errors.cs

* fix test case

* fix test case

* change "disallowed-html" to "disallowed-html-tag"

* change version

* update version

* test warn as error

* fix Errors.cs

* update version

Co-authored-by: Yufei Huang <[email protected]>
  • Loading branch information
@fkpwolf @yufeih
fkpwolf and yufeih authored Mar 4, 2021
1 parent 1af6c2e commit 9c36a70
Show file tree
Hide file tree
Showing 10 changed files with 60 additions and 32 deletions.
4 changes: 2 additions & 2 deletions docs/specs/error.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,8 @@ outputs:
pig4.jpg:
a.json:
.errors.log: |
{"message_severity":"info","code":"disallowed-html","message":"HTML tag 'br2' isn't allowed. Disallowed HTML poses a security risk and must be replaced with approved Docs Markdown syntax.","file":"a.md","line":1,"end_line":1,"column":2,"end_column":5}
{"message_severity":"info","code":"disallowed-html","message":"HTML tag 'br2' isn't allowed. Disallowed HTML poses a security risk and must be replaced with approved Docs Markdown syntax.","file":"a.md","line":2,"end_line":2,"column":2,"end_column":5}
{"message_severity":"info","code":"disallowed-html-tag","message":"HTML tag 'br2' isn't allowed. Disallowed HTML poses a security risk and must be replaced with approved Docs Markdown syntax.","file":"a.md","line":1,"end_line":1,"column":2,"end_column":5}
{"message_severity":"info","code":"disallowed-html-tag","message":"HTML tag 'br2' isn't allowed. Disallowed HTML poses a security risk and must be replaced with approved Docs Markdown syntax.","file":"a.md","line":2,"end_line":2,"column":2,"end_column":5}
{"message_severity":"info","code":"exceed-max-file-errors","message":"Info count exceed '2'. Build will continue but newer logs in 'a.md' will be ignored.","file":"a.md","line":0,"end_line":0,"column":0,"end_column":0}
---
Expand Down
18 changes: 9 additions & 9 deletions docs/specs/markdown.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -311,10 +311,10 @@ outputs:
docs/a.json: |
{ "conceptual": "<div></div><p>body</p>" }
.errors.log: |
{"message_severity":"info","code":"disallowed-html","message":"HTML tag 'script' isn't allowed. Disallowed HTML poses a security risk and must be replaced with approved Docs Markdown syntax.","file":"docs/a.md","line":1,"column":2}
{"message_severity":"info","code":"disallowed-html","message":"HTML tag 'link' isn't allowed. Disallowed HTML poses a security risk and must be replaced with approved Docs Markdown syntax.","file":"docs/a.md","line":2,"column":2}
{"message_severity":"info","code":"disallowed-html","message":"HTML attribute 'style' on tag 'div' isn't allowed. Disallowed HTML poses a security risk and must be replaced with approved Docs Markdown syntax.","file":"docs/a.md","line":3,"column":6}
{"message_severity":"info","code":"disallowed-html","message":"HTML tag 'style' isn't allowed. Disallowed HTML poses a security risk and must be replaced with approved Docs Markdown syntax.","file":"docs/a.md","line":4,"column":2}
{"message_severity":"info","code":"disallowed-html-tag","message":"HTML tag 'script' isn't allowed. Disallowed HTML poses a security risk and must be replaced with approved Docs Markdown syntax.","file":"docs/a.md","line":1,"column":2}
{"message_severity":"info","code":"disallowed-html-tag","message":"HTML tag 'link' isn't allowed. Disallowed HTML poses a security risk and must be replaced with approved Docs Markdown syntax.","file":"docs/a.md","line":2,"column":2}
{"message_severity":"info","code":"disallowed-html-attribute","message":"HTML attribute 'style' on tag 'div' isn't allowed. Disallowed HTML poses a security risk and must be replaced with approved Docs Markdown syntax.","file":"docs/a.md","line":3,"column":6}
{"message_severity":"info","code":"disallowed-html-tag","message":"HTML tag 'style' isn't allowed. Disallowed HTML poses a security risk and must be replaced with approved Docs Markdown syntax.","file":"docs/a.md","line":4,"column":2}
---
# markdown table with styles is allowed
inputs:
Expand Down Expand Up @@ -389,7 +389,7 @@ outputs:
{"message_severity":"warning","code":"bookmark-not-found","message":"Cannot find bookmark '#title-3' in 'docs/a.md', did you mean '#title-1'?","file":"docs/b.md","line":2,"column":1}
{"message_severity":"warning","code":"bookmark-not-found","message":"Cannot find bookmark '#title-3' in 'docs/a.md', did you mean '#title-1'?","file":"docs/a.md","line":4,"column":1}
{"message_severity":"warning","code":"bookmark-not-found","message":"Cannot find bookmark '#title-3' in 'docs/a.md', did you mean '#title-1'?","file":"docs/d.md","line":1,"column":1}
{"message_severity":"info","code":"disallowed-html","message":"HTML tag 'h2' isn't allowed. Disallowed HTML poses a security risk and must be replaced with approved Docs Markdown syntax.","file":"docs/a.md","line":8,"column":2}
{"message_severity":"info","code":"disallowed-html-tag","message":"HTML tag 'h2' isn't allowed. Disallowed HTML poses a security risk and must be replaced with approved Docs Markdown syntax.","file":"docs/a.md","line":8,"column":2}
---
# only validate bookmarks when the referenced file existed
inputs:
Expand Down Expand Up @@ -520,10 +520,10 @@ outputs:
docs/c.json: |
{ "conceptual": "<p><a><image src=\"https://shell.azure.com/images/launchcloudshell.png\"></image></a></p>\n" }
.errors.log: |
{"message_severity":"info","code":"disallowed-html","message":"HTML attribute 'style' on tag 'a' isn't allowed. Disallowed HTML poses a security risk and must be replaced with approved Docs Markdown syntax.","file":"docs/c.md","line":1,"column":4}
{"message_severity":"info","code":"disallowed-html","message":"HTML attribute 'style' on tag 'a' isn't allowed. Disallowed HTML poses a security risk and must be replaced with approved Docs Markdown syntax.","file":"docs/b.md","line":1,"column":4}
{"message_severity":"info","code":"disallowed-html","message":"HTML attribute 'onclick' on tag 'a' isn't allowed. Disallowed HTML poses a security risk and must be replaced with approved Docs Markdown syntax.","file":"docs/c.md","line":1,"column":27}
{"message_severity":"info","code":"disallowed-html","message":"HTML attribute 'onclick' on tag 'a' isn't allowed. Disallowed HTML poses a security risk and must be replaced with approved Docs Markdown syntax.","file":"docs/b.md","line":1,"column":27}
{"message_severity":"info","code":"disallowed-html-attribute","message":"HTML attribute 'style' on tag 'a' isn't allowed. Disallowed HTML poses a security risk and must be replaced with approved Docs Markdown syntax.","file":"docs/c.md","line":1,"column":4}
{"message_severity":"info","code":"disallowed-html-attribute","message":"HTML attribute 'style' on tag 'a' isn't allowed. Disallowed HTML poses a security risk and must be replaced with approved Docs Markdown syntax.","file":"docs/b.md","line":1,"column":4}
{"message_severity":"info","code":"disallowed-html-attribute","message":"HTML attribute 'onclick' on tag 'a' isn't allowed. Disallowed HTML poses a security risk and must be replaced with approved Docs Markdown syntax.","file":"docs/c.md","line":1,"column":27}
{"message_severity":"info","code":"disallowed-html-attribute","message":"HTML attribute 'onclick' on tag 'a' isn't allowed. Disallowed HTML poses a security risk and must be replaced with approved Docs Markdown syntax.","file":"docs/b.md","line":1,"column":27}
---
# yaml header shouldn't be array
inputs:
Expand Down
8 changes: 4 additions & 4 deletions docs/specs/moniker.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1407,7 +1407,7 @@ inputs:
outputs:
docs/v1/a.json:
.errors.log: |
{"message_severity":"error","code":"moniker-range-invalid","message":"Invalid moniker range '>= netcore-2.0': Moniker 'netcore-2.0' is not defined.","file":"docfx.yml","line":2,"column":17}
{"message_severity":"error","code":"moniker-range-missing","message":"Invalid moniker range '>= netcore-2.0': Moniker 'netcore-2.0' is not defined.","file":"docfx.yml","line":2,"column":17}
---
# Invalid moniker range in file
inputs:
Expand All @@ -1432,8 +1432,8 @@ inputs:
}
outputs:
.errors.log: |
{"message_severity":"error","code":"moniker-range-invalid","message":"Invalid moniker range 'netcore-1.2': Moniker 'netcore-1.2' is not defined.","file":"docs/v1/a.md","line":2,"column":15}
{"message_severity":"error","code":"moniker-range-invalid","message":"Invalid moniker range '<= netcore-1.2': Moniker 'netcore-1.2' is not defined.","file":"docs/v1/b.md","line":2,"column":1}
{"message_severity":"error","code":"moniker-range-missing","message":"Invalid moniker range 'netcore-1.2': Moniker 'netcore-1.2' is not defined.","file":"docs/v1/a.md","line":2,"column":15}
{"message_severity":"error","code":"moniker-range-missing","message":"Invalid moniker range '<= netcore-1.2': Moniker 'netcore-1.2' is not defined.","file":"docs/v1/b.md","line":2,"column":1}
{"message_severity":"error","code":"moniker-range-out-of-scope","message":"No moniker intersection between docfx.yml/docfx.json and file metadata. Config moniker range '>= netcore-1.0' is 'netcore-1.0', while file monikers is ''.","file":"docs/v1/a.md","line":2,"column":15}
{"message_severity":"warning","code":"moniker-zone-empty","message":"No intersection between zone and file level monikers. The result of zone level range string '<= netcore-1.2' is '', while file level monikers is 'netcore-1.0'.","file":"docs/v1/b.md","line":2,"column":1}
---
Expand Down Expand Up @@ -1699,7 +1699,7 @@ inputs:
}
outputs:
.errors.log: |
{"message_severity":"error","code":"moniker-range-invalid","message":"Invalid monikers: Moniker 'netcore-1.1' is not defined.","file":"docs/v1/TOC.md","line":2,"column":11}
{"message_severity":"error","code":"moniker-range-key-undefined","message":"Invalid monikers: Moniker 'netcore-1.1' is not defined.","file":"docs/v1/TOC.md","line":2,"column":11}
{"message_severity":"error","code":"moniker-range-out-of-scope","message":"No moniker intersection between docfx.yml/docfx.json and file metadata. Config moniker range '>= netcore-2.0' is 'netcore-2.0', while file monikers is ''.","file":"docs/v1/TOC.md","line":2,"column":11}
---
# Should take monikerRange as final monikers even it outputs empty
Expand Down
8 changes: 4 additions & 4 deletions docs/specs/validation/content-validation.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,10 @@ inputs:
outputs:
a.json:
.errors.log: |
{"message_severity":"info","code":"disallowed-html","message":"HTML tag 'H2' isn't allowed. Disallowed HTML poses a security risk and must be replaced with approved Docs Markdown syntax.","file":"a.md","line":1,"column":2}
{"message_severity":"info","code":"disallowed-html","message":"HTML tag 'BUTTON' isn't allowed. Disallowed HTML poses a security risk and must be replaced with approved Docs Markdown syntax.","file":"a.md","line":2,"column":2}
{"message_severity":"info","code":"disallowed-html","message":"HTML tag 'h1' isn't allowed. Disallowed HTML poses a security risk and must be replaced with approved Docs Markdown syntax.","file":"a.md","line":4,"column":2}
{"message_severity":"info","code":"disallowed-html","message":"HTML attribute 'onclick' on tag 'div' isn't allowed. Disallowed HTML poses a security risk and must be replaced with approved Docs Markdown syntax.","file":"a.md","line":5,"column":6}
{"message_severity":"info","code":"disallowed-html-tag","message":"HTML tag 'H2' isn't allowed. Disallowed HTML poses a security risk and must be replaced with approved Docs Markdown syntax.","file":"a.md","line":1,"column":2}
{"message_severity":"info","code":"disallowed-html-tag","message":"HTML tag 'BUTTON' isn't allowed. Disallowed HTML poses a security risk and must be replaced with approved Docs Markdown syntax.","file":"a.md","line":2,"column":2}
{"message_severity":"info","code":"disallowed-html-tag","message":"HTML tag 'h1' isn't allowed. Disallowed HTML poses a security risk and must be replaced with approved Docs Markdown syntax.","file":"a.md","line":4,"column":2}
{"message_severity":"info","code":"disallowed-html-attribute","message":"HTML attribute 'onclick' on tag 'div' isn't allowed. Disallowed HTML poses a security risk and must be replaced with approved Docs Markdown syntax.","file":"a.md","line":5,"column":6}
---
# Suppress disallowed HTML on archive content
inputs:
Expand Down
38 changes: 33 additions & 5 deletions src/docfx/Errors.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -457,11 +457,39 @@ public static Error MonikerOverlapping(string uid, List<FilePath> files, IEnumer
=> new Error(ErrorLevel.Error, "moniker-overlapping", $"Two or more documents with the same uid `{uid}`({StringUtility.Join(files)}) have defined overlapping moniker: {StringUtility.Join(overlappingMonikers)}.");

/// <summary>
/// Failed to parse moniker string.
/// Failed to parse moniker string: moniker is not defined.
/// </summary>
/// Behavior: ✔️ Message: ❌
public static Error MonikerRangeInvalid(SourceInfo? operand, FormattableString message)
=> new Error(ErrorLevel.Error, "moniker-range-invalid", message, operand);
public static Error MonikerRangeMissing(SourceInfo<string?> operand, string moniker)
=> new Error(ErrorLevel.Error, "moniker-range-missing", $"Invalid moniker range '{operand}': Moniker '{moniker}' is not defined.", operand);

/// <summary>
/// Failed to parse moniker string: parse ends before reaching end of string
/// </summary>
/// Behavior: ✔️ Message: ❌
public static Error MonikerRangeUnrecognized(SourceInfo? operand, string rangeString)
=> new Error(ErrorLevel.Error, "moniker-range-unrecognized", $"Parse ends before reaching end of string, unrecognized string: '{rangeString}'.", operand);

/// <summary>
/// Failed to parse moniker string: expect a comparator set
/// </summary>
/// Behavior: ✔️ Message: ❌
public static Error MonikerRangeMissComparator(SourceInfo? operand, string rangeString)
=> new Error(ErrorLevel.Error, "moniker-range-miss-comparator", $"Expect a comparator set, but got '{rangeString}'.", operand);

/// <summary>
/// Failed to parse moniker string: expect a moniker string
/// </summary>
/// Behavior: ✔️ Message: ❌
public static Error MonikerRangeMissMoniker(SourceInfo? operand, string rangeString)
=> new Error(ErrorLevel.Error, "moniker-range-miss-moniker", $"Expect a moniker string, but got '{rangeString}'.", operand);

/// <summary>
/// Failed to parse moniker string: Moniker key is not defined.
/// </summary>
/// Behavior: ✔️ Message: ❌
public static Error MonikerRangeKeyUndefined(SourceInfo? operand, string key)
=> new Error(ErrorLevel.Error, "moniker-range-key-undefined", $"Invalid monikers: Moniker '{key}' is not defined.", operand);

/// <summary>
/// MonikerRange is not defined in docfx.yml or doesn't match an article.md,
Expand Down Expand Up @@ -755,13 +783,13 @@ public static Error Custom404Page(FilePath file)
/// Html Tag value must be in allowed list
/// </summary>
public static Error DisallowedHtml(SourceInfo? source, string tag)
=> new Error(ErrorLevel.Info, "disallowed-html", $"HTML tag '{tag}' isn't allowed. Disallowed HTML poses a security risk and must be replaced with approved Docs Markdown syntax.", source, propertyPath: tag);
=> new Error(ErrorLevel.Info, "disallowed-html-tag", $"HTML tag '{tag}' isn't allowed. Disallowed HTML poses a security risk and must be replaced with approved Docs Markdown syntax.", source, propertyPath: tag);

/// <summary>
/// Html Attribute value must be in allowed list
/// </summary>
public static Error DisallowedHtml(SourceInfo? source, string tag, string attribute)
=> new Error(ErrorLevel.Info, "disallowed-html", $"HTML attribute '{attribute}' on tag '{tag}' isn't allowed. Disallowed HTML poses a security risk and must be replaced with approved Docs Markdown syntax.", source, propertyPath: $"{tag}_{attribute}");
=> new Error(ErrorLevel.Info, "disallowed-html-attribute", $"HTML attribute '{attribute}' on tag '{tag}' isn't allowed. Disallowed HTML poses a security risk and must be replaced with approved Docs Markdown syntax.", source, propertyPath: $"{tag}_{attribute}");
}

public static class DependencyRepository
Expand Down
3 changes: 2 additions & 1 deletion src/docfx/docfx.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
<ItemGroup>
<PackageReference Include="Menees.Analyzers.2017" Version="2.0.3" PrivateAssets="all" />
<PackageReference Include="Nerdbank.GitVersioning" Version="3.3.37" PrivateAssets="all" />
<PackageReference Include="docs.validation.analyzer" Version="1.0.0-beta-00529-24ad8c5" PrivateAssets="all" />
</ItemGroup>

<ItemGroup>
Expand All @@ -49,4 +50,4 @@
<ProjectReference Include="../Microsoft.Docs.LearnValidation/Microsoft.Docs.LearnValidation.csproj" />
</ItemGroup>

</Project>
</Project>
3 changes: 1 addition & 2 deletions src/docfx/lib/moniker/EvaluatorWithMonikersVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@ public EvaluatorWithMonikersVisitor(MonikerDefinitionModel monikerDefinition)
{
if (!MonikerOrder.TryGetValue(expression.Operand, out var moniker))
{
return (Errors.Versioning.MonikerRangeInvalid(
monikerRange, $"Invalid moniker range '{monikerRange}': Moniker '{expression.Operand}' is not defined."), Array.Empty<Moniker>());
return (Errors.Versioning.MonikerRangeMissing(monikerRange, expression.Operand), Array.Empty<Moniker>());
}

return expression.Operator switch
Expand Down
6 changes: 3 additions & 3 deletions src/docfx/lib/moniker/ExpressionCreator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public static (List<Error>, IExpression?) Create(string rangeString, SourceInfo?
errors.AddRange(rangeErrors);
if (!string.IsNullOrWhiteSpace(rangeString))
{
errors.Add(Errors.Versioning.MonikerRangeInvalid(source, $"Parse ends before reaching end of string, unrecognized string: '{rangeString}'."));
errors.Add(Errors.Versioning.MonikerRangeUnrecognized(source, rangeString));
}

return (errors, expression);
Expand Down Expand Up @@ -75,7 +75,7 @@ private static (List<Error>, IExpression?) GetComparatorSet(ref string rangeStri

if (result is null)
{
errors.Add(Errors.Versioning.MonikerRangeInvalid(source, $"Expect a comparator set, but got '{rangeString}'."));
errors.Add(Errors.Versioning.MonikerRangeMissComparator(source, rangeString));
}
return (errors, result);
}
Expand All @@ -101,7 +101,7 @@ private static bool TryGetComparator(ref string rangeString, SourceInfo? source,
}
else
{
error = Errors.Versioning.MonikerRangeInvalid(source, $"Expect a moniker string, but got '{rangeString}'.");
error = Errors.Versioning.MonikerRangeMissMoniker(source, rangeString);
return false;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/docfx/lib/moniker/MonikerRangeParser.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public MonikerList Validate(ErrorBuilder errors, SourceInfo<string>[] monikers)
{
if (!_monikersEvaluator.MonikerMap.ContainsKey(key))
{
errors.Add(Errors.Versioning.MonikerRangeInvalid(moniker, $"Invalid monikers: Moniker '{key}' is not defined."));
errors.Add(Errors.Versioning.MonikerRangeKeyUndefined(moniker, key));
}
else
{
Expand Down
2 changes: 1 addition & 1 deletion test/docfx.Test/lib/MonikerRangeParserTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ public void TestNullDefinitionShouldFail()
monikerRangeParser.Parse(errors, new SourceInfo<string>("netcore-1.0"));
Assert.Collection(errors, error =>
{
Assert.Equal("moniker-range-invalid", error.Code);
Assert.Equal("moniker-range-missing", error.Code);
Assert.Equal("Invalid moniker range 'netcore-1.0': Moniker 'netcore-1.0' is not defined.", error.Message);
});
}
Expand Down

0 comments on commit 9c36a70

Please sign in to comment.