Skip to content

Fix WildcardPattern.Escape to escape lone backticks correctly #25211

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Mar 25, 2025
Merged

Fix WildcardPattern.Escape to escape lone backticks correctly #25211

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
8b98336
Escape backtick in WildcardPattern.Escape() method
ArmaanMcleod Mar 19, 2025
d3190e5
Add tests
ArmaanMcleod Mar 19, 2025
49af878
Add wildcard pattern comparison tests with -eq
ArmaanMcleod Mar 19, 2025
eb16878
Remove extra tests which are in XUnit tests
ArmaanMcleod Mar 22, 2025
d610779
Merge branch 'master' into fix-wildcard-escape-v2
ArmaanMcleod Mar 22, 2025
0970c8e
Add more tests
ArmaanMcleod Mar 22, 2025
ed7fc45
Simplify condition to just check if char exists in special chars
ArmaanMcleod Mar 22, 2025
e05d6dc
Add more tests to cover wildcard escaping
ArmaanMcleod Mar 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/System.Management.Automation/engine/regex.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,9 +238,9 @@ internal static string Escape(string pattern, char[] charsNotToEscape)
char ch = pattern[i];

//
// if it is a wildcard char, escape it
// if it is a special char, escape it
//
if (IsWildcardChar(ch) && !charsNotToEscape.Contains(ch))
if (SpecialChars.Contains(ch) && !charsNotToEscape.Contains(ch))
{
temp[tempIndex++] = escapeChar;
}
Expand Down
15 changes: 15 additions & 0 deletions test/powershell/Language/Operators/ComparisonOperator.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,21 @@ Describe "ComparisonOperator" -Tag "CI" {
param($lhs, $operator, $rhs)
Invoke-Expression "$lhs $operator $rhs" | Should -BeFalse
}

It "Should be <result> for backtick comparison <lhs> <operator> <rhs>" -TestCases @(
@{ lhs = 'abc`def'; operator = '-like'; rhs = 'abc`def'; result = $false }
@{ lhs = 'abc`def'; operator = '-like'; rhs = 'abc``def'; result = $true }
@{ lhs = 'abc`def'; operator = '-like'; rhs = 'abc````def'; result = $false }
@{ lhs = 'abc``def'; operator = '-like'; rhs = 'abc````def'; result = $true }
@{ lhs = 'abc`def'; operator = '-like'; rhs = [WildcardPattern]::Escape('abc`def'); result = $true }
@{ lhs = 'abc`def'; operator = '-like'; rhs = [WildcardPattern]::Escape('abc``def'); result = $false }
@{ lhs = 'abc``def'; operator = '-like'; rhs = [WildcardPattern]::Escape('abc``def'); result = $true }
@{ lhs = 'abc``def'; operator = '-like'; rhs = [WildcardPattern]::Escape('abc````def'); result = $false }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is "Win7 backcompatibility requires treating '`' pattern as '' pattern" comment in regex.cs file. Please add a test for the scenario.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iSazonov Do we need to worry about this? I am not sure why we are still trying to support Win 7 compatibility and why such code even exists 😄. Can you elaborate on what scenario this covers?

Copy link
Contributor Author

@ArmaanMcleod ArmaanMcleod Mar 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given also the code changes here are in Escape() I am not sure what I am testing. The Win7 compatibility is more with ensuring WildcardPattern in Parse() method treats lone backtick as empty string.

) {
param($lhs, $operator, $rhs, $result)
$expression = "'$lhs' $operator '$rhs'"
Invoke-Expression $expression | Should -Be $result
}
}

Describe "Bytewise Operator" -Tag "CI" {
Expand Down
16 changes: 16 additions & 0 deletions test/xUnit/csharp/test_WildcardPattern.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,22 @@ public void TestEscape_Empty()
[InlineData("a", "a")]
[InlineData("a*", "a`*")]
[InlineData("*?[]", "`*`?`[`]")]
[InlineData("`", "``")]
[InlineData("```", "``````")]
[InlineData("a`", "a``")]
[InlineData("abc`def", "abc``def")]
[InlineData("abc``def", "abc````def")]
[InlineData("text with `backticks", "text with ``backticks")]
[InlineData("ends with `", "ends with ``")]
[InlineData("`starts with", "``starts with")]
[InlineData("`in`between`", "``in``between``")]
[InlineData("no special characters", "no special characters")]
[InlineData("`*`", "```*``")]
[InlineData("*`?[]", "`*```?`[`]")]
[InlineData("`*`?`[", "```*```?```[")]
[InlineData("*?[]`", "`*`?`[`]``")]
[InlineData("nested `backticks `inside`", "nested ``backticks ``inside``")]
[InlineData("wildcard*with`backtick", "wildcard`*with``backtick")]
public void TestEscape_String(string source, string expected)
{
Assert.Equal(WildcardPattern.Escape(source), expected);
Expand Down
Loading