[FarGroup/FarManager] master: Fix string replace when pattern and/or replacement are views into the same string (e54e3d9f2)

[farg...@farmanager.com]

Repository : https://github.com/FarGroup/FarManager
On branch : master
Link : https://github.com/FarGroup/FarManager/commit/e54e3d9f25aff54a2bd860e049bec7d47ade5cb9

>---------------------------------------------------------------

commit e54e3d9f25aff54a2bd860e049bec7d47ade5cb9
Author: Alex Alabuzhev <alab...@gmail.com>
Date: Thu Oct 6 17:44:43 2022 +0100

Fix string replace when pattern and/or replacement are views into the same string


>---------------------------------------------------------------

e54e3d9f25aff54a2bd860e049bec7d47ade5cb9
far/changelog | 5 ++++
far/filemasks.cpp | 31 +++++++++++++++++---
far/strmix.cpp | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
far/vbuild.m4 | 2 +-
4 files changed, 116 insertions(+), 6 deletions(-)

diff --git a/far/changelog b/far/changelog
index f3cccd8b0..d01aa40c6 100644
--- a/far/changelog
+++ b/far/changelog
@@ -1,3 +1,8 @@
+--------------------------------------------------------------------------------
+drkns 06.10.2022 17:43:00 +0100 - build 6037
+
+1. Fix string replace when pattern and/or replacement are views into the same string.
+
--------------------------------------------------------------------------------
drkns 05.10.2022 23:47:54 +0100 - build 6036

diff --git a/far/filemasks.cpp b/far/filemasks.cpp
index 32a65f8d0..ad070f36a 100644
--- a/far/filemasks.cpp
+++ b/far/filemasks.cpp
@@ -120,6 +120,13 @@ filemasks::~filemasks() = default;
filemasks::filemasks(filemasks&&) noexcept = default;
filemasks& filemasks::operator=(filemasks&&) noexcept = default;

+static string get_mask_group(string_view const Group)
+{
+ return ConfigProvider().GeneralCfg()->GetValue<string>(L"Masks"sv, Group);
+}
+
+static auto mask_group_accessor = &get_mask_group;
+
bool filemasks::assign(string_view Str, DWORD const Flags)
{
if (Str.empty())
@@ -137,16 +144,16 @@ bool filemasks::assign(string_view Str, DWORD const Flags)
while ((LBPos = ExpandedGroups.find(L'<')) != string::npos && (RBPos = ExpandedGroups.find(L'>', LBPos)) != string::npos)
{
const auto MaskGroupNameWithBrackets = string_view(ExpandedGroups).substr(LBPos, RBPos - LBPos + 1);
- string MaskGroupName(MaskGroupNameWithBrackets.substr(1, MaskGroupNameWithBrackets.size() - 2));
+ const auto MaskGroupName = MaskGroupNameWithBrackets.substr(1, MaskGroupNameWithBrackets.size() - 2);

- if (contains(UsedGroups, MaskGroupName))
+ if (contains(UsedGroups, string_comparer::generic_key{ MaskGroupName }))
{
MaskGroupValue.clear();
}
else
{
- MaskGroupValue = ConfigProvider().GeneralCfg()->GetValue<string>(L"Masks"sv, MaskGroupName);
- UsedGroups.emplace(std::move(MaskGroupName));
+ MaskGroupValue = mask_group_accessor(MaskGroupName);
+ UsedGroups.emplace(MaskGroupName);
}
replace(ExpandedGroups, MaskGroupNameWithBrackets, MaskGroupValue);
}
@@ -432,14 +439,30 @@ TEST_CASE("masks")
{ L"file.*"sv, L"file.bin"sv, true },
{ L"file.*"sv, L"file..bin"sv, true },
{ L"file.*|*b*"sv, L"file.bin"sv, false },
+ { L"*.ext,<duh>"sv, L"meow.txt"sv, true },
+ { L"<duh>,*.ext"sv, L"boo.boo"sv, true },
+ { L"*.ext,<doh>"sv, L"meow.ext"sv, true },
+ { L"<doh>,*.ext"sv, L"boo.ext"sv, true },
};

filemasks Masks;

+ const auto test_mask_group_accessor = [](string_view const Mask)
+ {
+ if (Mask == L"duh"sv)
+ return L"*.txt;*.boo"s;
+
+ return L""s;
+ };
+
+ mask_group_accessor = test_mask_group_accessor;
+
for (const auto& i: Tests)
{
REQUIRE(Masks.assign(i.Mask, FMF_SILENT));
REQUIRE(i.Match == Masks.check(i.Test));
}
+
+ mask_group_accessor = &get_mask_group;
}
#endif
diff --git a/far/strmix.cpp b/far/strmix.cpp
index dba37553b..be92e4bbe 100644
--- a/far/strmix.cpp
+++ b/far/strmix.cpp
@@ -491,14 +491,52 @@ string FileSizeToStr(unsigned long long FileSize, int WidthWithSign, unsigned lo
return FormatSize(std::move(Str), UnitIndex);
}

+static bool within(string_view const Haystack, string_view const Needle)
+{
+ // Comparing potentially unrelated pointers is, technically, UB.
+ // Integers are always fine.
+
+ const auto HaystackBegin = reinterpret_cast<uintptr_t>(Haystack.data());
+ const auto HaystackEnd = reinterpret_cast<uintptr_t>(Haystack.data() + Haystack.size());
+
+ const auto NeedleBegin = reinterpret_cast<uintptr_t>(Needle.data());
+ const auto NeedleEnd = reinterpret_cast<uintptr_t>(Needle.data() + Needle.size());
+
+ /*
+ HHHHHH
+ NN
+ NN
+ NN
+ */
+ return
+ NeedleBegin >= HaystackBegin && NeedleBegin < HaystackEnd&&
+ NeedleEnd > HaystackBegin && NeedleEnd <= HaystackEnd &&
+ // An empty needle could be within the haystack, but who cares.
+ // You can't really break an empty view by invalidating its underlying storage.
+ NeedleEnd > NeedleBegin;
+}

// Заменить в строке Str Count вхождений подстроки FindStr на подстроку ReplStr
// Если Count == npos - заменять "до полной победы"
-bool ReplaceStrings(string& strStr, const string_view FindStr, const string_view ReplStr, const bool IgnoreCase, size_t Count)
+bool ReplaceStrings(string& strStr, string_view FindStr, string_view ReplStr, const bool IgnoreCase, size_t Count)
{
if (strStr.empty() || FindStr.empty() || !Count)
return false;

+ string FindCopy, ReplaceCopy;
+
+ if (Count != 1 && within(strStr, FindStr))
+ {
+ FindCopy = FindStr;
+ FindStr = FindCopy;
+ }
+
+ if (Count != 1 && within(strStr, ReplStr))
+ {
+ ReplaceCopy = ReplStr;
+ ReplStr = ReplaceCopy;
+ }
+
size_t replaced = 0;
size_t StartPos = 0;

@@ -1179,6 +1217,50 @@ TEST_CASE("ReplaceStrings")
}
}

+TEST_CASE("within")
+{
+ const auto Haystack = L"banana"sv;
+
+ REQUIRE(within(Haystack, Haystack.substr(0)));
+ REQUIRE(within(Haystack, Haystack.substr(0, 2)));
+ REQUIRE(within(Haystack, Haystack.substr(2, 2)));
+ REQUIRE(within(Haystack, Haystack.substr(4)));
+ REQUIRE(within(Haystack, Haystack.substr(Haystack.size() - 1)));
+
+ // Empty views are not within anything.
+ REQUIRE(!within(Haystack, Haystack.substr(0, 0)));
+ REQUIRE(!within(Haystack, Haystack.substr(1, 0)));
+ REQUIRE(!within(Haystack, Haystack.substr(Haystack.size())));
+}
+
+TEST_CASE("ReplaceStrings.within")
+{
+ {
+ auto Str = L"99 little bugs in the code. 99 little bugs in the code"s;
+ const auto Find = string_view(Str).substr(0, 2);
+ const auto Replace = string_view(Str).substr(3, 6);
+ ReplaceStrings(Str, Find, Replace);
+ REQUIRE(Str == L"little little bugs in the code. little little bugs in the code"sv);
+ }
+
+ {
+ auto Str = L"banana banana banana banana"s;
+ const auto Find = string_view(Str).substr(21, 6);
+ const auto Replace = string_view(Str).substr(2, 2);
+ ReplaceStrings(Str, Find, Replace);
+ REQUIRE(Str == L"na na na na"sv);
+ }
+
+ {
+ auto Str = L"Alegría Macarena"s;
+ const auto Find = string_view(Str).substr(0, 7);
+ const auto Replace = string_view(Str).substr(8, 8);
+ // A single replace should pick the fast path
+ ReplaceStrings(Str, Find, Replace, false, 1);
+ REQUIRE(Str == L"Macarena Macarena"sv);
+ }
+}
+
TEST_CASE("remove_duplicates")
{
static const struct
diff --git a/far/vbuild.m4 b/far/vbuild.m4
index d7a3cf5e5..4b6413877 100644
--- a/far/vbuild.m4
+++ b/far/vbuild.m4
@@ -1 +1 @@
-6036
+6037