Skip to content

Commit

Permalink
Don't overmatch globstars into filenames (#291)
Browse files Browse the repository at this point in the history 
Fixes #290
  • Loading branch information
@heaths
heaths authored Jun 27, 2023
1 parent c7a417b commit 4af6aa0
Show file tree
Hide file tree
Showing 2 changed files with 119 additions and 27 deletions.
42 changes: 38 additions & 4 deletions src/vswhere.lib/Glob.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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".")
{
Expand Down Expand Up @@ -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))
{
Expand Down Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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;
}
104 changes: 81 additions & 23 deletions test/vswhere.test/GlobTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@
using namespace std;
using namespace Microsoft::VisualStudio::CppUnitTestFramework;

#ifdef _DEBUG
#define ASSERT_PATTERN
#endif

TEST_CLASS(GlobTests)
{
public:
Expand All @@ -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

Expand All @@ -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"));
Expand All @@ -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"));
Expand All @@ -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"));
Expand All @@ -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

Expand All @@ -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"));
Expand All @@ -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"));
Expand Down Expand Up @@ -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

Expand All @@ -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"));
Expand All @@ -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"));
Expand All @@ -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"));
Expand Down Expand Up @@ -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"));
Expand All @@ -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"));
Expand All @@ -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"));
}
};

0 comments on commit 4af6aa0

Please sign in to comment.