Fix filename comparison on Windows

boost::locale::to_lower uses locale-dependent lowercasing, which doesn't
match filesystem case insensitivity when running in the Turkish locale,
as then lowercase "I" is not "i", and uppercase "i" is not "I". To avoid
this issue on Windows, use CompareStringOrdinal to use the same rules
as the filesystem when comparing strings, and CharUpperBuffW to
uppercase strings before comparing hashes. It's less clear that
CharUpperBuffW uses the same rules as the filesystem, but that's what I
understand from reading around.

I haven't figured out how to get similar behaviour on Linux, though it
probably involves using ICU. For now, boost::locale::to_upper is used
before comparison. This is inaccurate, but at least avoids the problem
of some characters not round-tripping when lowercased.
This commit is contained in:
Oliver Hamlet
2019-05-25 10:45:59 +01:00
parent eeefb61c5b
commit a301369868
12 changed files with 188 additions and 31 deletions
+8 -1
View File
@@ -95,6 +95,13 @@ public:
*/
LOOT_API std::string GetLowercasedName() const;
/**
* Get the plugin name, normalized to be suitable for case-insensitive
* filename comparison.
* @return The normalized plugin name.
*/
LOOT_API std::string GetNormalizedName() const;
/**
* Check if the plugin metadata is enabled for use during sorting.
* @return True if the metadata will be used during sorting, false otherwise.
@@ -312,7 +319,7 @@ struct hash<loot::PluginMetadata> {
* @return The hash generated from the plugin's lowercased filename.
*/
size_t operator()(const loot::PluginMetadata& plugin) const {
return hash<string>()(plugin.GetLowercasedName());
return hash<string>()(plugin.GetNormalizedName());
}
};
}
+6 -5
View File
@@ -28,7 +28,8 @@
#include <boost/locale.hpp>
using boost::locale::to_lower;
#include "api/helpers/text.h"
using std::lock_guard;
using std::mutex;
using std::pair;
@@ -63,7 +64,7 @@ std::set<std::shared_ptr<const Plugin>> GameCache::GetPlugins() const {
std::shared_ptr<const Plugin> GameCache::GetPlugin(
const std::string& pluginName) const {
auto it = plugins_.find(to_lower(pluginName));
auto it = plugins_.find(NormalizeFilename(pluginName));
if (it != end(plugins_))
return it->second;
@@ -73,13 +74,13 @@ std::shared_ptr<const Plugin> GameCache::GetPlugin(
void GameCache::AddPlugin(const Plugin&& plugin) {
lock_guard<mutex> lock(mutex_);
auto lowercasedName = to_lower(plugin.GetName());
auto normalizedName = NormalizeFilename(plugin.GetName());
auto it = plugins_.find(lowercasedName);
auto it = plugins_.find(normalizedName);
if (it != end(plugins_))
plugins_.erase(it);
plugins_.emplace(lowercasedName,
plugins_.emplace(normalizedName,
std::make_shared<Plugin>(std::move(plugin)));
}
+57
View File
@@ -27,6 +27,12 @@
#include <boost/algorithm/string.hpp>
#ifdef _WIN32
#include "windows.h"
#else
#include <boost/locale.hpp>
#endif
using std::regex;
namespace loot {
@@ -107,4 +113,55 @@ std::optional<std::string> ExtractVersion(const std::string& text) {
return std::nullopt;
}
#ifdef _WIN32
std::wstring ToWinWide(const std::string& str) {
size_t len = MultiByteToWideChar(CP_UTF8, 0, str.c_str(), str.length(), 0, 0);
std::wstring wstr(len, 0);
MultiByteToWideChar(CP_UTF8, 0, str.c_str(), str.length(), &wstr[0], len);
return wstr;
}
std::string FromWinWide(const std::wstring& wstr) {
size_t len = WideCharToMultiByte(
CP_UTF8, 0, wstr.c_str(), wstr.length(), NULL, 0, NULL, NULL);
std::string str(len, 0);
WideCharToMultiByte(
CP_UTF8, 0, wstr.c_str(), wstr.length(), &str[0], len, NULL, NULL);
return str;
}
#endif
int CompareFilenames(const std::string& lhs, const std::string& rhs) {
#ifdef _WIN32
// On Windows, use CompareStringOrdinal as that will perform case conversion
// using the operating system uppercase table information, which (I think)
// will give results that match the filesystem, and is not locale-dependent.
int result = CompareStringOrdinal(ToWinWide(lhs).c_str(), -1, ToWinWide(rhs).c_str(), -1, true);
switch (result) {
case CSTR_LESS_THAN:
return -1;
case CSTR_EQUAL:
return 0;
case CSTR_GREATER_THAN:
return 1;
default:
throw std::invalid_argument("One of the filenames to compare was invalid.");
}
#else
using boost::locale::to_upper;
return to_upper(lhs).compare(to_upper(rhs));
#endif
}
std::string NormalizeFilename(const std::string& filename) {
#ifdef _WIN32
auto wideString = ToWinWide(filename);
CharUpperBuffW(&wideString[0], wideString.length());
return FromWinWide(wideString);
#else
return boost::locale::to_upper(filename);
#endif
}
}
+8
View File
@@ -35,6 +35,14 @@ namespace loot {
std::set<Tag> ExtractBashTags(const std::string& description);
std::optional<std::string> ExtractVersion(const std::string& text);
// Compare strings as if they're filenames, respecting filesystem case
// insensitivity on Windows. Returns -1 if lhs < rhs, 0 if lhs == rhs, and 1 if
// lhs > rhs.
int CompareFilenames(const std::string& lhs, const std::string& rhs);
// Uppercase the given filename using an invariant locale on Windows.
std::string NormalizeFilename(const std::string& filename);
}
#endif
+3 -2
View File
@@ -27,6 +27,7 @@
#include <boost/locale.hpp>
#include "api/metadata/yaml/file.h"
#include "api/helpers/text.h"
namespace loot {
File::File() {}
@@ -39,11 +40,11 @@ File::File(const std::string& name,
ConditionalMetadata(condition) {}
bool File::operator<(const File& rhs) const {
return boost::locale::to_lower(name_) < boost::locale::to_lower(rhs.name_);
return CompareFilenames(name_, rhs.name_) < 0;
}
bool File::operator==(const File& rhs) const {
return boost::locale::to_lower(name_) == boost::locale::to_lower(rhs.name_);
return CompareFilenames(name_, rhs.name_) == 0;
}
std::string File::GetName() const { return name_; }
+6 -1
View File
@@ -32,6 +32,7 @@
#include "api/game/game.h"
#include "api/helpers/logging.h"
#include "api/helpers/text.h"
using std::inserter;
using std::regex;
@@ -175,6 +176,10 @@ std::string PluginMetadata::GetLowercasedName() const {
return boost::locale::to_lower(name_);
}
std::string PluginMetadata::GetNormalizedName() const {
return NormalizeFilename(name_);
}
bool PluginMetadata::IsEnabled() const { return enabled_; }
std::optional<std::string> PluginMetadata::GetGroup() const { return group_; }
@@ -271,7 +276,7 @@ bool PluginMetadata::IsRegexPlugin() const {
bool PluginMetadata::operator==(const PluginMetadata& rhs) const {
if (IsRegexPlugin() == rhs.IsRegexPlugin()) {
return GetLowercasedName() == rhs.GetLowercasedName();
return CompareFilenames(name_, rhs.name_) == 0;
}
if (IsRegexPlugin())
+2 -1
View File
@@ -29,6 +29,7 @@
#include "api/game/game.h"
#include "api/helpers/logging.h"
#include "api/helpers/text.h"
#include "api/metadata/condition_evaluator.h"
#include "api/metadata/yaml/group.h"
#include "api/metadata/yaml/plugin_metadata.h"
@@ -100,7 +101,7 @@ void MetadataList::Save(const std::filesystem::path& filepath) const {
auto plugins = Plugins();
plugins.sort([](const PluginMetadata& p1, const PluginMetadata& p2) {
return p1.GetLowercasedName() < p2.GetLowercasedName();
return CompareFilenames(p1.GetName(), p2.GetName()) < 0;
});
if (!plugins.empty())
+1 -1
View File
@@ -226,7 +226,7 @@ uintmax_t Plugin::GetFileSize(std::filesystem::path pluginPath) {
}
bool Plugin::operator<(const Plugin& rhs) const {
return boost::locale::to_lower(name_) < boost::locale::to_lower(rhs.name_);
return CompareFilenames(name_, rhs.name_) < 0;
}
void Plugin::Load(const std::filesystem::path& path,
+10 -15
View File
@@ -35,6 +35,7 @@
#include "api/game/game.h"
#include "api/helpers/logging.h"
#include "api/helpers/text.h"
#include "api/metadata/condition_evaluator.h"
#include "api/sorting/group_sort.h"
#include "loot/exception/cyclic_interaction_error.h"
@@ -237,10 +238,9 @@ void PluginSorter::AddPluginVertices(Game& game) {
}
std::optional<vertex_t> PluginSorter::GetVertexByName(const std::string& name) const {
auto lowercasedName = boost::locale::to_lower(name);
for (const auto& vertex :
boost::make_iterator_range(boost::vertices(graph_))) {
if (graph_[vertex].GetLowercasedName() == lowercasedName) {
if (CompareFilenames(graph_[vertex].GetName(), name) == 0) {
return vertex;
}
}
@@ -678,22 +678,17 @@ int PluginSorter::ComparePlugins(const std::string& plugin1,
// comparison to get an ordering.
// Compare plugin basenames.
string name1 = boost::locale::to_lower(plugin1);
name1 = name1.substr(0, name1.length() - 4);
string name2 = boost::locale::to_lower(plugin2);
name2 = name2.substr(0, name2.length() - 4);
auto basename1 = plugin1.substr(0, plugin1.length() - 4);
auto basename2 = plugin2.substr(0, plugin2.length() - 4);
if (name1 < name2)
return -1;
else if (name2 < name1)
return 1;
else {
int result = CompareFilenames(basename1, basename2);
if (result != 0) {
return result;
} else {
// Could be a .esp and .esm plugin with the same basename,
// compare whole filenames.
if (plugin1 < plugin2)
return -1;
else
return 1;
return CompareFilenames(plugin1, plugin2);
}
}
return 0;
-4
View File
@@ -49,10 +49,6 @@ PluginSortingData::PluginSortingData(const Plugin& plugin,
std::string PluginSortingData::GetName() const { return plugin_.GetName(); }
std::string PluginSortingData::GetLowercasedName() const {
return boost::locale::to_lower(plugin_.GetName());
}
bool PluginSortingData::IsMaster() const {
return plugin_.IsMaster() || (plugin_.IsLightMaster() &&
!boost::iends_with(plugin_.GetName(), ".esp"));
-1
View File
@@ -36,7 +36,6 @@ public:
const PluginMetadata& userMetadata);
std::string GetName() const;
std::string GetLowercasedName() const;
bool IsMaster() const;
bool LoadsArchive() const;
std::vector<std::string> GetMasters() const;
@@ -176,6 +176,93 @@ TEST(ExtractVersion, shouldPreferVersionPrefixedNumbersOverVPrefixedNumber) {
"later),\nPatch Version: 1.0");
EXPECT_EQ("1.0", text.value());
}
#ifdef _WIN32
// MSVC interprets source files in the default code page, so
// for me u8"\xC3\x9C" != u8"\u00DC", which is a lot of fun.
// To avoid insanity, write non-ASCII characters as \uXXXX escapes.
// \u03a1 is greek rho uppercase 'Ρ'
// \u03c1 is greek rho lowercase 'ρ'
// \u03f1 is greek rho 'ϱ'
// \u0130 is turkish 'İ'
// \u0131 is turkish 'ı'
TEST(CompareFilenames, shouldBeCaseInsensitiveAndLocaleInvariant) {
EXPECT_EQ(0, CompareFilenames("i", "I"));
EXPECT_EQ(-1, CompareFilenames("i", u8"\u0130"));
EXPECT_EQ(-1, CompareFilenames("i", u8"\u0131"));
EXPECT_EQ(-1, CompareFilenames("I", u8"\u0130"));
EXPECT_EQ(-1, CompareFilenames("I", u8"\u0131"));
EXPECT_EQ(-1, CompareFilenames(u8"\u0130", u8"\u0131"));
EXPECT_EQ(1, CompareFilenames(u8"\u03f1", u8"\u03a1"));
EXPECT_EQ(1, CompareFilenames(u8"\u03f1", u8"\u03c1"));
EXPECT_EQ(0, CompareFilenames(u8"\u03a1", u8"\u03c1"));
// Set locale to Turkish.
std::locale::global(boost::locale::generator().generate("tr_TR.UTF-8"));
EXPECT_EQ(0, CompareFilenames("i", "I"));
EXPECT_EQ(-1, CompareFilenames("i", u8"\u0130"));
EXPECT_EQ(-1, CompareFilenames("i", u8"\u0131"));
EXPECT_EQ(-1, CompareFilenames("I", u8"\u0130"));
EXPECT_EQ(-1, CompareFilenames("I", u8"\u0131"));
EXPECT_EQ(-1, CompareFilenames(u8"\u0130", u8"\u0131"));
EXPECT_EQ(1, CompareFilenames(u8"\u03f1", u8"\u03a1"));
EXPECT_EQ(1, CompareFilenames(u8"\u03f1", u8"\u03c1"));
EXPECT_EQ(0, CompareFilenames(u8"\u03a1", u8"\u03c1"));
// Set locale to Greek.
std::locale::global(boost::locale::generator().generate("el_GR.UTF-8"));
EXPECT_EQ(0, CompareFilenames("i", "I"));
EXPECT_EQ(-1, CompareFilenames("i", u8"\u0130"));
EXPECT_EQ(-1, CompareFilenames("i", u8"\u0131"));
EXPECT_EQ(-1, CompareFilenames("I", u8"\u0130"));
EXPECT_EQ(-1, CompareFilenames("I", u8"\u0131"));
EXPECT_EQ(-1, CompareFilenames(u8"\u0130", u8"\u0131"));
EXPECT_EQ(1, CompareFilenames(u8"\u03f1", u8"\u03a1"));
EXPECT_EQ(1, CompareFilenames(u8"\u03f1", u8"\u03c1"));
EXPECT_EQ(0, CompareFilenames(u8"\u03a1", u8"\u03c1"));
// Reset locale.
std::locale::global(boost::locale::generator().generate(""));
}
TEST(NormalizeFilename, shouldUppercaseStringsAndBeLocaleInvariant) {
EXPECT_EQ("I", NormalizeFilename("i"));
EXPECT_EQ("I", NormalizeFilename("I"));
EXPECT_EQ(u8"\u0130", NormalizeFilename(u8"\u0130"));
EXPECT_EQ(u8"\u0131", NormalizeFilename(u8"\u0131"));
EXPECT_EQ(u8"\u03f1", NormalizeFilename(u8"\u03f1"));
EXPECT_EQ(u8"\u03a1", NormalizeFilename(u8"\u03a1"));
EXPECT_EQ(u8"\u03a1", NormalizeFilename(u8"\u03c1"));
// Set locale to Turkish.
std::locale::global(boost::locale::generator().generate("tr_TR.UTF-8"));
EXPECT_EQ("I", NormalizeFilename("i"));
EXPECT_EQ("I", NormalizeFilename("I"));
EXPECT_EQ(u8"\u0130", NormalizeFilename(u8"\u0130"));
EXPECT_EQ(u8"\u0131", NormalizeFilename(u8"\u0131"));
EXPECT_EQ(u8"\u03f1", NormalizeFilename(u8"\u03f1"));
EXPECT_EQ(u8"\u03a1", NormalizeFilename(u8"\u03a1"));
EXPECT_EQ(u8"\u03a1", NormalizeFilename(u8"\u03c1"));
// Set locale to Greek.
std::locale::global(boost::locale::generator().generate("el_GR.UTF-8"));
EXPECT_EQ("I", NormalizeFilename("i"));
EXPECT_EQ("I", NormalizeFilename("I"));
EXPECT_EQ(u8"\u0130", NormalizeFilename(u8"\u0130"));
EXPECT_EQ(u8"\u0131", NormalizeFilename(u8"\u0131"));
EXPECT_EQ(u8"\u03f1", NormalizeFilename(u8"\u03f1"));
EXPECT_EQ(u8"\u03a1", NormalizeFilename(u8"\u03a1"));
EXPECT_EQ(u8"\u03a1", NormalizeFilename(u8"\u03c1"));
// Reset locale.
std::locale::global(boost::locale::generator().generate(""));
}
#endif
}
}