From 4af6aa058af09bc9aa7f4b3cd6ab97d37372dfe0 Mon Sep 17 00:00:00 2001 From: Heath Stewart Date: Tue, 27 Jun 2023 14:57:04 -0700 Subject: [PATCH] Don't overmatch globstars into filenames (#291) Fixes #290 --- src/vswhere.lib/Glob.cpp | 42 +++++++++++-- test/vswhere.test/GlobTests.cpp | 104 +++++++++++++++++++++++++------- 2 files changed, 119 insertions(+), 27 deletions(-) diff --git a/src/vswhere.lib/Glob.cpp b/src/vswhere.lib/Glob.cpp index 03324f5..c35e233 100644 --- a/src/vswhere.lib/Glob.cpp +++ b/src/vswhere.lib/Glob.cpp @@ -8,9 +8,10 @@ using namespace std; using namespace std::experimental::filesystem; +std::wstring rtrim(const std::wstring& value); Glob::Glob(_In_ const wstring& root, _In_ const wstring& pattern) : - m_root(root) + m_root(rtrim(root)) { wstring value; wstring value_raw; @@ -36,8 +37,8 @@ Glob::Glob(_In_ const wstring& root, _In_ const wstring& pattern) : { if (found_globstar) { - // Replase globstar with any characer match plus optional directory separator. - accumulator += L".*\\\\?"; + // Replace globstar with any character match plus directory separator. + accumulator += L"(.*\\\\)?"; } else if (value_raw.length() == 1 && value_raw == L".") { @@ -134,6 +135,20 @@ Glob::Glob(_In_ const wstring& root, _In_ const wstring& pattern) : // Invalid leading globstar before non-separator or invalid character. ThrowError(pattern); } + else if (found_wildcard) + { + // A single star (asterisk) searches the any subdirectory not including the current directory. + if (value.length() == 1 && value[0] == L'*') + { + accumulator += L"[^\\]*"; + } + else + { + accumulator += value; + } + + value.clear(); + } if (RequiresEscape(ch)) { @@ -175,8 +190,9 @@ Glob::Glob(_In_ const wstring& root, _In_ const wstring& pattern) : { m_re = wregex(accumulator, wregex::extended | wregex::icase | wregex::nosubs | wregex::optimize); } - catch (const regex_error&) + catch (const regex_error& ex) { + _RPTN(_CRT_ERROR, "regex parse error: %s", ex.what()); ThrowError(pattern); } } @@ -251,3 +267,21 @@ void Glob::ThrowError(_In_ const wstring& pattern) auto message = ResourceManager::FormatString(IDS_E_INVALIDPATTERN, pattern.c_str()); throw win32_error(ERROR_INVALID_PARAMETER, message); } + +std::wstring rtrim(const std::wstring& value) +{ + if (value.back() != L'\\' && value.back() != L'/') + { + return value; + } + + wstring copy(value); + wstring::reverse_iterator it = copy.rbegin(); + while (it != copy.rend() && (*it == L'\\' || *it == L'/')) + { + it++; + } + + copy.erase(it.base(), copy.end()); + return copy; +} \ No newline at end of file diff --git a/test/vswhere.test/GlobTests.cpp b/test/vswhere.test/GlobTests.cpp index 2fadce7..a583ba3 100644 --- a/test/vswhere.test/GlobTests.cpp +++ b/test/vswhere.test/GlobTests.cpp @@ -8,6 +8,10 @@ using namespace std; using namespace Microsoft::VisualStudio::CppUnitTestFramework; +#ifdef _DEBUG +#define ASSERT_PATTERN +#endif + TEST_CLASS(GlobTests) { public: @@ -19,7 +23,7 @@ TEST_CLASS(GlobTests) Glob sut(L"C:\\ShouldNotExist", L"MSBuild\\15.0\\Bin\\MSBuild.exe"); Assert::AreEqual(L"C:\\ShouldNotExist\\MSBuild\\15.0\\Bin", sut.Root().c_str()); -#ifdef _DEBUG +#ifdef ASSERT_PATTERN Assert::AreEqual(L"^\\\\MSBuild\\.exe$", sut.Pattern().c_str()); #endif @@ -35,8 +39,8 @@ TEST_CLASS(GlobTests) Glob sut(L"C:\\ShouldNotExist", L"MSBuild\\**\\MSBuild.exe"); Assert::AreEqual(L"C:\\ShouldNotExist\\MSBuild", sut.Root().c_str()); -#ifdef _DEBUG - Assert::AreEqual(L"^\\\\.*\\\\?MSBuild\\.exe$", sut.Pattern().c_str()); +#ifdef ASSERT_PATTERN + Assert::AreEqual(L"^\\\\(.*\\\\)?MSBuild\\.exe$", sut.Pattern().c_str()); #endif Assert::IsTrue(sut.Match(L"C:\\ShouldNotExist\\MSBuild\\MSBuild.exe")); @@ -54,8 +58,8 @@ TEST_CLASS(GlobTests) Glob sut(L"C:\\ShouldNotExist", L"MSBuild\\**\\Bin\\**\\MSBuild.exe"); Assert::AreEqual(L"C:\\ShouldNotExist\\MSBuild", sut.Root().c_str()); -#ifdef _DEBUG - Assert::AreEqual(L"^\\\\.*\\\\?Bin\\\\.*\\\\?MSBuild\\.exe$", sut.Pattern().c_str()); +#ifdef ASSERT_PATTERN + Assert::AreEqual(L"^\\\\(.*\\\\)?Bin\\\\(.*\\\\)?MSBuild\\.exe$", sut.Pattern().c_str()); #endif Assert::IsFalse(sut.Match(L"C:\\ShouldNotExist\\MSBuild\\MSBuild.exe")); @@ -73,8 +77,8 @@ TEST_CLASS(GlobTests) Glob sut(L"C:\\ShouldNotExist", L"MSBuild\\**\\Bin\\*\\MSBuild.exe"); Assert::AreEqual(L"C:\\ShouldNotExist\\MSBuild", sut.Root().c_str()); -#ifdef _DEBUG - Assert::AreEqual(L"^\\\\.*\\\\?Bin\\\\[^\\]*\\\\MSBuild\\.exe$", sut.Pattern().c_str()); +#ifdef ASSERT_PATTERN + Assert::AreEqual(L"^\\\\(.*\\\\)?Bin\\\\[^\\]*\\\\MSBuild\\.exe$", sut.Pattern().c_str()); #endif Assert::IsFalse(sut.Match(L"C:\\ShouldNotExist\\MSBuild\\MSBuild.exe")); @@ -92,7 +96,7 @@ TEST_CLASS(GlobTests) Glob sut(L"C:\\ShouldNotExist", L"MSBuild\\Current\\Bin\\MSBuild.ex?"); Assert::AreEqual(L"C:\\ShouldNotExist\\MSBuild\\Current\\Bin", sut.Root().c_str()); -#ifdef _DEBUG +#ifdef ASSERT_PATTERN Assert::AreEqual(L"^\\\\MSBuild\\.ex[^\\]$", sut.Pattern().c_str()); #endif @@ -110,8 +114,8 @@ TEST_CLASS(GlobTests) Glob sut(L"C:\\ShouldNotExist", L"VC\\Tools\\MSVC\\14.*\\bin\\**\\cl.exe"); Assert::AreEqual(L"C:\\ShouldNotExist\\VC\\Tools\\MSVC", sut.Root().c_str()); -#ifdef _DEBUG - Assert::AreEqual(L"^\\\\14\\.[^\\]*\\\\bin\\\\.*\\\\?cl\\.exe$", sut.Pattern().c_str()); +#ifdef ASSERT_PATTERN + Assert::AreEqual(L"^\\\\14\\.[^\\]*\\\\bin\\\\(.*\\\\)?cl\\.exe$", sut.Pattern().c_str()); #endif Assert::IsFalse(sut.Match(L"C:\\ShouldNotExist\\VC\\Tools\\MSVC\\15.0\\bin\\cl.exe")); @@ -128,8 +132,8 @@ TEST_CLASS(GlobTests) Glob sut(L"C:\\ShouldNotExist", L"msbuild\\**\\msbuild.exe"); Assert::AreEqual(L"C:\\ShouldNotExist\\msbuild", sut.Root().c_str()); -#ifdef _DEBUG - Assert::AreEqual(L"^\\\\.*\\\\?msbuild\\.exe$", sut.Pattern().c_str()); +#ifdef ASSERT_PATTERN + Assert::AreEqual(L"^\\\\(.*\\\\)?msbuild\\.exe$", sut.Pattern().c_str()); #endif Assert::IsTrue(sut.Match(L"C:\\ShouldNotExist\\MSBuild\\Current\\Bin\\MSBuild.exe")); @@ -180,7 +184,7 @@ TEST_CLASS(GlobTests) Glob sut(L"C:\\ShouldNotExist", L"MSBuild\\**"); Assert::AreEqual(L"C:\\ShouldNotExist\\MSBuild", sut.Root().c_str()); -#ifdef _DEBUG +#ifdef ASSERT_PATTERN Assert::AreEqual(L"^\\\\.*$", sut.Pattern().c_str()); #endif @@ -199,8 +203,8 @@ TEST_CLASS(GlobTests) Glob sut(L"C:\\ShouldNotExist", L"\\MSBuild\\**\\MSBuild.exe"); Assert::AreEqual(L"C:\\ShouldNotExist\\MSBuild", sut.Root().c_str()); -#ifdef _DEBUG - Assert::AreEqual(L"^\\\\.*\\\\?MSBuild\\.exe$", sut.Pattern().c_str()); +#ifdef ASSERT_PATTERN + Assert::AreEqual(L"^\\\\(.*\\\\)?MSBuild\\.exe$", sut.Pattern().c_str()); #endif Assert::IsTrue(sut.Match(L"C:\\ShouldNotExist\\MSBuild\\MSBuild.exe")); @@ -218,8 +222,8 @@ TEST_CLASS(GlobTests) Glob sut(L"C:\\ShouldNotExist", L"\\\\MSBuild\\**\\MSBuild.exe"); Assert::AreEqual(L"C:\\ShouldNotExist\\MSBuild", sut.Root().c_str()); -#ifdef _DEBUG - Assert::AreEqual(L"^\\\\.*\\\\?MSBuild\\.exe$", sut.Pattern().c_str()); +#ifdef ASSERT_PATTERN + Assert::AreEqual(L"^\\\\(.*\\\\)?MSBuild\\.exe$", sut.Pattern().c_str()); #endif Assert::IsTrue(sut.Match(L"C:\\ShouldNotExist\\MSBuild\\MSBuild.exe")); @@ -237,8 +241,8 @@ TEST_CLASS(GlobTests) Glob sut(L"C:\\ShouldNotExist", L"MSBuild\\**\\Bin\\\\MSBuild.exe"); Assert::AreEqual(L"C:\\ShouldNotExist\\MSBuild", sut.Root().c_str()); -#ifdef _DEBUG - Assert::AreEqual(L"^\\\\.*\\\\?Bin\\\\MSBuild\\.exe$", sut.Pattern().c_str()); +#ifdef ASSERT_PATTERN + Assert::AreEqual(L"^\\\\(.*\\\\)?Bin\\\\MSBuild\\.exe$", sut.Pattern().c_str()); #endif Assert::IsFalse(sut.Match(L"C:\\ShouldNotExist\\MSBuild\\MSBuild.exe")); @@ -292,8 +296,8 @@ TEST_CLASS(GlobTests) Glob sut(L"C:\\ShouldNotExist", L".\\**\\msbuild.exe"); Assert::AreEqual(L"C:\\ShouldNotExist", sut.Root().c_str()); -#ifdef _DEBUG - Assert::AreEqual(L"^\\\\.*\\\\?msbuild\\.exe$", sut.Pattern().c_str()); +#ifdef ASSERT_PATTERN + Assert::AreEqual(L"^\\\\(.*\\\\)?msbuild\\.exe$", sut.Pattern().c_str()); #endif Assert::IsTrue(sut.Match(L"C:\\ShouldNotExist\\MSBuild.exe")); @@ -311,8 +315,8 @@ TEST_CLASS(GlobTests) Glob sut(L"C:\\ShouldNotExist", L"**\\.\\msbuild.exe"); Assert::AreEqual(L"C:\\ShouldNotExist", sut.Root().c_str()); -#ifdef _DEBUG - Assert::AreEqual(L"^\\\\.*\\\\?msbuild\\.exe$", sut.Pattern().c_str()); +#ifdef ASSERT_PATTERN + Assert::AreEqual(L"^\\\\(.*\\\\)?msbuild\\.exe$", sut.Pattern().c_str()); #endif Assert::IsTrue(sut.Match(L"C:\\ShouldNotExist\\MSBuild.exe")); @@ -321,4 +325,58 @@ TEST_CLASS(GlobTests) Assert::IsTrue(sut.Match(L"C:\\ShouldNotExist\\MSBuild\\Current\\Bin\\MSBuild.exe")); Assert::IsTrue(sut.Match(L"C:\\ShouldNotExist\\MSBuild\\Current\\Bin\\amd64\\MSBuild.exe")); } + + BEGIN_TEST_METHOD_ATTRIBUTE(Ignores_Trailing_Separator) + TEST_WORKITEM(290) + END_TEST_METHOD_ATTRIBUTE() + TEST_METHOD(Ignores_Trailing_Separator) + { + Glob sut(L"C:\\ShouldNotExist\\", L"**\\cl.exe"); + + Assert::IsTrue(sut.Match(L"C:\\ShouldNotExist\\VC\\Tools\\MSVC\\14.36.32532\\bin\\Hostx64\\x64\\cl.exe")); + } + + BEGIN_TEST_METHOD_ATTRIBUTE(Issue290) + TEST_WORKITEM(290) + END_TEST_METHOD_ATTRIBUTE() + TEST_METHOD(Issue290) + { + Glob sut(L"C:\\ShouldNotExist", L"**\\cl.exe"); + +#ifdef ASSERT_PATTERN + Assert::AreEqual(L"^\\\\(.*\\\\)?cl\\.exe$", sut.Pattern().c_str()); +#endif + + + Assert::IsFalse(sut.Match(L"C:\\ShouldNotExist\\Common7\\IDE\\CommonExtensions\\Microsoft\\TeamFoundation\\Team Explorer\\Git\\usr\\bin\\getfacl.exe")); + Assert::IsTrue(sut.Match(L"C:\\ShouldNotExist\\VC\\Tools\\MSVC\\14.36.32532\\bin\\Hostx64\\arm64\\cl.exe")); + Assert::IsTrue(sut.Match(L"C:\\ShouldNotExist\\VC\\Tools\\MSVC\\14.36.32532\\bin\\Hostx64\\x64\\cl.exe")); + Assert::IsTrue(sut.Match(L"C:\\ShouldNotExist\\VC\\Tools\\MSVC\\14.36.32532\\bin\\Hostx86\\arm\\cl.exe")); + Assert::IsTrue(sut.Match(L"C:\\ShouldNotExist\\VC\\Tools\\MSVC\\14.36.32532\\bin\\Hostx86\\x86\\cl.exe")); + + // Assert the reason the last directory specifier is optional: to make sure we find cl.exe in the root directory. + Assert::IsTrue(sut.Match(L"C:\\ShouldNotExist\\cl.exe")); + } + + BEGIN_TEST_METHOD_ATTRIBUTE(Issue290_With_Wildcard_Filename) + TEST_WORKITEM(290) + END_TEST_METHOD_ATTRIBUTE() + TEST_METHOD(Issue290_With_Wildcard_Filename) + { + Glob sut(L"C:\\ShouldNotExist", L"**\\*cl.exe"); + +#ifdef ASSERT_PATTERN + Assert::AreEqual(L"^\\\\(.*\\\\)?[^\\]*cl\\.exe$", sut.Pattern().c_str()); +#endif + + + Assert::IsTrue(sut.Match(L"C:\\ShouldNotExist\\Common7\\IDE\\CommonExtensions\\Microsoft\\TeamFoundation\\Team Explorer\\Git\\usr\\bin\\getfacl.exe")); + Assert::IsTrue(sut.Match(L"C:\\ShouldNotExist\\VC\\Tools\\MSVC\\14.36.32532\\bin\\Hostx64\\arm64\\cl.exe")); + Assert::IsTrue(sut.Match(L"C:\\ShouldNotExist\\VC\\Tools\\MSVC\\14.36.32532\\bin\\Hostx64\\x64\\cl.exe")); + Assert::IsTrue(sut.Match(L"C:\\ShouldNotExist\\VC\\Tools\\MSVC\\14.36.32532\\bin\\Hostx86\\arm\\cl.exe")); + Assert::IsTrue(sut.Match(L"C:\\ShouldNotExist\\VC\\Tools\\MSVC\\14.36.32532\\bin\\Hostx86\\x86\\cl.exe")); + + // Assert the reason the last directory specifier is optional: to make sure we find cl.exe in the root directory. + Assert::IsTrue(sut.Match(L"C:\\ShouldNotExist\\cl.exe")); + } };