From 862b9d02170084c1c764bee921ab05b7670d0339 Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Thu, 8 Apr 2021 15:27:02 -0700 Subject: [PATCH 1/5] Escape special characters in section name of editor config --- .../Analyzers/AnalyzerConfigTests.cs | 35 +++++++++++++++++++ .../GenerateMSBuildEditorConfig.cs | 31 +++++++++++++++- .../GenerateMSBuildEditorConfigTests.cs | 28 +++++++++++++++ 3 files changed, 93 insertions(+), 1 deletion(-) diff --git a/src/Compilers/Core/CodeAnalysisTest/Analyzers/AnalyzerConfigTests.cs b/src/Compilers/Core/CodeAnalysisTest/Analyzers/AnalyzerConfigTests.cs index 1624752b3330e..c287e3f2e1d9b 100644 --- a/src/Compilers/Core/CodeAnalysisTest/Analyzers/AnalyzerConfigTests.cs +++ b/src/Compilers/Core/CodeAnalysisTest/Analyzers/AnalyzerConfigTests.cs @@ -57,6 +57,41 @@ public void SimpleCase() Assert.Equal("/bogus", config.NormalizedDirectory); } + [Fact] + public void ConfigWithEscapedValues() + { + var config = ParseConfigFile(@"is_global = true + +[c:/\{file1\}.cs] +build_metadata.Compile.ToRetrieve = abc123 + +[c:/file\#2.cs] +build_metadata.Compile.ToRetrieve = def456 + +[c:/file\[3\].cs] +build_metadata.Compile.ToRetrieve = ghi789 +"); + + var namedSections = config.NamedSections; + Assert.Equal("c:/\\{file1\\}.cs", namedSections[0].Name); + AssertEx.Equal( + new[] { KeyValuePair.Create("build_metadata.compile.toretrieve", "abc123") }, + namedSections[0].Properties + ); + + Assert.Equal("c:/file\\#2.cs", namedSections[1].Name); + AssertEx.Equal( + new[] { KeyValuePair.Create("build_metadata.compile.toretrieve", "def456") }, + namedSections[1].Properties + ); + + Assert.Equal("c:/file\\[3\\].cs", namedSections[2].Name); + AssertEx.Equal( + new[] { KeyValuePair.Create("build_metadata.compile.toretrieve", "ghi789") }, + namedSections[2].Properties + ); + } + [ConditionalFact(typeof(WindowsOnly))] public void WindowsPath() { diff --git a/src/Compilers/Core/MSBuildTask/GenerateMSBuildEditorConfig.cs b/src/Compilers/Core/MSBuildTask/GenerateMSBuildEditorConfig.cs index 85c4f573c0f3c..b3d4cd580e6e0 100644 --- a/src/Compilers/Core/MSBuildTask/GenerateMSBuildEditorConfig.cs +++ b/src/Compilers/Core/MSBuildTask/GenerateMSBuildEditorConfig.cs @@ -78,7 +78,7 @@ public override bool Execute() // write the section for this item builder.AppendLine() .Append("[") - .Append(group.Key) + .Append(EncodeString(group.Key)) .AppendLine("]"); foreach (var item in group) @@ -101,6 +101,35 @@ public override bool Execute() return true; } + // Filenames with special characters like '#' and'{' get written + // into the section names in the resulting .editorconfig file. Later, + // when the file is parsed in configuration options these special + // characters are interpretted as invalid values and ignored by the + // processor. We encode the special characters in these strings + // before writing them here. + + private static string EncodeString(string p) + { + StringBuilder builder = new StringBuilder(); + for (var i = 0; i < p.Length; i++) + { + builder.Append(p[i] switch + { + '*' => "\\*", + '?' => "\\?", + '{' => "\\{", + ',' => "\\,", + ';' => "\\;", + '}' => "\\}", + '[' => "\\[", + ']' => "\\]", + '#' => "\\#", + var @default => @default + }); + } + return builder.ToString(); + } + /// /// Equivalent to Roslyn.Utilities.PathUtilities.NormalizeWithForwardSlash /// Both methods should be kept in sync. diff --git a/src/Compilers/Core/MSBuildTaskTests/GenerateMSBuildEditorConfigTests.cs b/src/Compilers/Core/MSBuildTaskTests/GenerateMSBuildEditorConfigTests.cs index 19a0af562c043..98475bd0166b4 100644 --- a/src/Compilers/Core/MSBuildTaskTests/GenerateMSBuildEditorConfigTests.cs +++ b/src/Compilers/Core/MSBuildTaskTests/GenerateMSBuildEditorConfigTests.cs @@ -97,6 +97,34 @@ public void MultipleItemMetaDataCreatesSections() ", result); } + [Fact] + public void MultipleSpecialCharacterItemMetaDataCreatesSections() + { + ITaskItem item1 = MSBuildUtil.CreateTaskItem("c:/{file1}.cs", new Dictionary { { "ItemType", "Compile" }, { "MetadataName", "ToRetrieve" }, { "ToRetrieve", "abc123" } }); + ITaskItem item2 = MSBuildUtil.CreateTaskItem("c:/file#2.cs", new Dictionary { { "ItemType", "Compile" }, { "MetadataName", "ToRetrieve" }, { "ToRetrieve", "def456" } }); + ITaskItem item3 = MSBuildUtil.CreateTaskItem("c:/file[3].cs", new Dictionary { { "ItemType", "Compile" }, { "MetadataName", "ToRetrieve" }, { "ToRetrieve", "ghi789" } }); + + GenerateMSBuildEditorConfig configTask = new GenerateMSBuildEditorConfig() + { + MetadataItems = new[] { item1, item2, item3 } + }; + configTask.Execute(); + + var result = configTask.ConfigFileContents; + + Assert.Equal(@"is_global = true + +[c:/\{file1\}.cs] +build_metadata.Compile.ToRetrieve = abc123 + +[c:/file\#2.cs] +build_metadata.Compile.ToRetrieve = def456 + +[c:/file\[3\].cs] +build_metadata.Compile.ToRetrieve = ghi789 +", result); + } + [Fact] public void DuplicateItemSpecsAreCombinedInSections() { From 84bb4ad3cc7dfc02915833f28af26730d6658f1d Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Mon, 12 Apr 2021 15:02:08 -0700 Subject: [PATCH 2/5] Address feedback from peer review --- .../Analyzers/AnalyzerConfigTests.cs | 1 + .../GenerateMSBuildEditorConfig.cs | 28 ++++++------------- .../GenerateMSBuildEditorConfigTests.cs | 1 + 3 files changed, 11 insertions(+), 19 deletions(-) diff --git a/src/Compilers/Core/CodeAnalysisTest/Analyzers/AnalyzerConfigTests.cs b/src/Compilers/Core/CodeAnalysisTest/Analyzers/AnalyzerConfigTests.cs index c287e3f2e1d9b..7512069660d23 100644 --- a/src/Compilers/Core/CodeAnalysisTest/Analyzers/AnalyzerConfigTests.cs +++ b/src/Compilers/Core/CodeAnalysisTest/Analyzers/AnalyzerConfigTests.cs @@ -58,6 +58,7 @@ public void SimpleCase() } [Fact] + [WorkItem(52469, "https://github.com/dotnet/roslyn/issues/52469")] public void ConfigWithEscapedValues() { var config = ParseConfigFile(@"is_global = true diff --git a/src/Compilers/Core/MSBuildTask/GenerateMSBuildEditorConfig.cs b/src/Compilers/Core/MSBuildTask/GenerateMSBuildEditorConfig.cs index b3d4cd580e6e0..f66bba425c30e 100644 --- a/src/Compilers/Core/MSBuildTask/GenerateMSBuildEditorConfig.cs +++ b/src/Compilers/Core/MSBuildTask/GenerateMSBuildEditorConfig.cs @@ -77,9 +77,9 @@ public override bool Execute() { // write the section for this item builder.AppendLine() - .Append("[") - .Append(EncodeString(group.Key)) - .AppendLine("]"); + .Append("["); + EncodeString(builder, group.Key); + builder.AppendLine("]"); foreach (var item in group) { @@ -108,26 +108,16 @@ public override bool Execute() // processor. We encode the special characters in these strings // before writing them here. - private static string EncodeString(string p) + private static void EncodeString(StringBuilder builder, string p) { - StringBuilder builder = new StringBuilder(); - for (var i = 0; i < p.Length; i++) + foreach (var c in p) { - builder.Append(p[i] switch + if (c is '*' or '?' or '{' or ',' or ';' or '}' or '[' or ']' or '#') { - '*' => "\\*", - '?' => "\\?", - '{' => "\\{", - ',' => "\\,", - ';' => "\\;", - '}' => "\\}", - '[' => "\\[", - ']' => "\\]", - '#' => "\\#", - var @default => @default - }); + builder.Append("\\"); + } + builder.Append(c); } - return builder.ToString(); } /// diff --git a/src/Compilers/Core/MSBuildTaskTests/GenerateMSBuildEditorConfigTests.cs b/src/Compilers/Core/MSBuildTaskTests/GenerateMSBuildEditorConfigTests.cs index 98475bd0166b4..de6aeccf85a24 100644 --- a/src/Compilers/Core/MSBuildTaskTests/GenerateMSBuildEditorConfigTests.cs +++ b/src/Compilers/Core/MSBuildTaskTests/GenerateMSBuildEditorConfigTests.cs @@ -98,6 +98,7 @@ public void MultipleItemMetaDataCreatesSections() } [Fact] + [WorkItem(52469, "https://github.com/dotnet/roslyn/issues/52469")] public void MultipleSpecialCharacterItemMetaDataCreatesSections() { ITaskItem item1 = MSBuildUtil.CreateTaskItem("c:/{file1}.cs", new Dictionary { { "ItemType", "Compile" }, { "MetadataName", "ToRetrieve" }, { "ToRetrieve", "abc123" } }); From e07e512a1ab5b1eba8cce8f4545c8484e923dc20 Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Mon, 12 Apr 2021 15:27:33 -0700 Subject: [PATCH 3/5] Clean up code comments and vars --- .../MSBuildTask/GenerateMSBuildEditorConfig.cs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/Compilers/Core/MSBuildTask/GenerateMSBuildEditorConfig.cs b/src/Compilers/Core/MSBuildTask/GenerateMSBuildEditorConfig.cs index f66bba425c30e..e13fc7699fc8f 100644 --- a/src/Compilers/Core/MSBuildTask/GenerateMSBuildEditorConfig.cs +++ b/src/Compilers/Core/MSBuildTask/GenerateMSBuildEditorConfig.cs @@ -101,16 +101,18 @@ public override bool Execute() return true; } - // Filenames with special characters like '#' and'{' get written - // into the section names in the resulting .editorconfig file. Later, - // when the file is parsed in configuration options these special - // characters are interpretted as invalid values and ignored by the - // processor. We encode the special characters in these strings - // before writing them here. + /// + /// Filenames with special characters like '#' and'{' get written + /// into the section names in the resulting .editorconfig file. Later, + /// when the file is parsed in configuration options these special + /// characters are interpretted as invalid values and ignored by the + /// processor. We encode the special characters in these strings + /// before writing them here. + /// - private static void EncodeString(StringBuilder builder, string p) + private static void EncodeString(StringBuilder builder, string value) { - foreach (var c in p) + foreach (var c in value) { if (c is '*' or '?' or '{' or ',' or ';' or '}' or '[' or ']' or '#') { From 66a1caaabd0622903b8a3f00920f0bc6687c0686 Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Tue, 13 Apr 2021 16:31:39 -0700 Subject: [PATCH 4/5] Escape '!' characters in section names --- src/Compilers/Core/MSBuildTask/GenerateMSBuildEditorConfig.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Compilers/Core/MSBuildTask/GenerateMSBuildEditorConfig.cs b/src/Compilers/Core/MSBuildTask/GenerateMSBuildEditorConfig.cs index e13fc7699fc8f..d0eead0082aa4 100644 --- a/src/Compilers/Core/MSBuildTask/GenerateMSBuildEditorConfig.cs +++ b/src/Compilers/Core/MSBuildTask/GenerateMSBuildEditorConfig.cs @@ -114,7 +114,7 @@ private static void EncodeString(StringBuilder builder, string value) { foreach (var c in value) { - if (c is '*' or '?' or '{' or ',' or ';' or '}' or '[' or ']' or '#') + if (c is '*' or '?' or '{' or ',' or ';' or '}' or '[' or ']' or '#' or '!') { builder.Append("\\"); } From 5d0c5b2199bbe175db8d5b7f08facde26546ac08 Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Tue, 13 Apr 2021 17:37:12 -0700 Subject: [PATCH 5/5] Add more test coverage for special characters --- .../Analyzers/AnalyzerConfigTests.cs | 12 ++++++------ .../GenerateMSBuildEditorConfigTests.cs | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/Compilers/Core/CodeAnalysisTest/Analyzers/AnalyzerConfigTests.cs b/src/Compilers/Core/CodeAnalysisTest/Analyzers/AnalyzerConfigTests.cs index 7512069660d23..b3b540ef660da 100644 --- a/src/Compilers/Core/CodeAnalysisTest/Analyzers/AnalyzerConfigTests.cs +++ b/src/Compilers/Core/CodeAnalysisTest/Analyzers/AnalyzerConfigTests.cs @@ -63,30 +63,30 @@ public void ConfigWithEscapedValues() { var config = ParseConfigFile(@"is_global = true -[c:/\{file1\}.cs] +[c:/\{f\*i\?le1\}.cs] build_metadata.Compile.ToRetrieve = abc123 -[c:/file\#2.cs] +[c:/f\,ile\#2.cs] build_metadata.Compile.ToRetrieve = def456 -[c:/file\[3\].cs] +[c:/f\;i\!le\[3\].cs] build_metadata.Compile.ToRetrieve = ghi789 "); var namedSections = config.NamedSections; - Assert.Equal("c:/\\{file1\\}.cs", namedSections[0].Name); + Assert.Equal("c:/\\{f\\*i\\?le1\\}.cs", namedSections[0].Name); AssertEx.Equal( new[] { KeyValuePair.Create("build_metadata.compile.toretrieve", "abc123") }, namedSections[0].Properties ); - Assert.Equal("c:/file\\#2.cs", namedSections[1].Name); + Assert.Equal("c:/f\\,ile\\#2.cs", namedSections[1].Name); AssertEx.Equal( new[] { KeyValuePair.Create("build_metadata.compile.toretrieve", "def456") }, namedSections[1].Properties ); - Assert.Equal("c:/file\\[3\\].cs", namedSections[2].Name); + Assert.Equal("c:/f\\;i\\!le\\[3\\].cs", namedSections[2].Name); AssertEx.Equal( new[] { KeyValuePair.Create("build_metadata.compile.toretrieve", "ghi789") }, namedSections[2].Properties diff --git a/src/Compilers/Core/MSBuildTaskTests/GenerateMSBuildEditorConfigTests.cs b/src/Compilers/Core/MSBuildTaskTests/GenerateMSBuildEditorConfigTests.cs index de6aeccf85a24..5ce1dc1604995 100644 --- a/src/Compilers/Core/MSBuildTaskTests/GenerateMSBuildEditorConfigTests.cs +++ b/src/Compilers/Core/MSBuildTaskTests/GenerateMSBuildEditorConfigTests.cs @@ -101,9 +101,9 @@ public void MultipleItemMetaDataCreatesSections() [WorkItem(52469, "https://github.com/dotnet/roslyn/issues/52469")] public void MultipleSpecialCharacterItemMetaDataCreatesSections() { - ITaskItem item1 = MSBuildUtil.CreateTaskItem("c:/{file1}.cs", new Dictionary { { "ItemType", "Compile" }, { "MetadataName", "ToRetrieve" }, { "ToRetrieve", "abc123" } }); - ITaskItem item2 = MSBuildUtil.CreateTaskItem("c:/file#2.cs", new Dictionary { { "ItemType", "Compile" }, { "MetadataName", "ToRetrieve" }, { "ToRetrieve", "def456" } }); - ITaskItem item3 = MSBuildUtil.CreateTaskItem("c:/file[3].cs", new Dictionary { { "ItemType", "Compile" }, { "MetadataName", "ToRetrieve" }, { "ToRetrieve", "ghi789" } }); + ITaskItem item1 = MSBuildUtil.CreateTaskItem("c:/{f*i?le1}.cs", new Dictionary { { "ItemType", "Compile" }, { "MetadataName", "ToRetrieve" }, { "ToRetrieve", "abc123" } }); + ITaskItem item2 = MSBuildUtil.CreateTaskItem("c:/f,ile#2.cs", new Dictionary { { "ItemType", "Compile" }, { "MetadataName", "ToRetrieve" }, { "ToRetrieve", "def456" } }); + ITaskItem item3 = MSBuildUtil.CreateTaskItem("c:/f;i!le[3].cs", new Dictionary { { "ItemType", "Compile" }, { "MetadataName", "ToRetrieve" }, { "ToRetrieve", "ghi789" } }); GenerateMSBuildEditorConfig configTask = new GenerateMSBuildEditorConfig() { @@ -115,13 +115,13 @@ public void MultipleSpecialCharacterItemMetaDataCreatesSections() Assert.Equal(@"is_global = true -[c:/\{file1\}.cs] +[c:/\{f\*i\?le1\}.cs] build_metadata.Compile.ToRetrieve = abc123 -[c:/file\#2.cs] +[c:/f\,ile\#2.cs] build_metadata.Compile.ToRetrieve = def456 -[c:/file\[3\].cs] +[c:/f\;i\!le\[3\].cs] build_metadata.Compile.ToRetrieve = ghi789 ", result); }