-
Notifications
You must be signed in to change notification settings - Fork 865
[1.5] Filter Chat #7968
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.
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
base: 1.5
Are you sure you want to change the base?
[1.5] Filter Chat #7968
Conversation
if (!FileExists(path.c_str())) { | ||
if (FILE *file = OpenFile(path.c_str(), "wb")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FileExists()
does not provide a strong enough guarantee to avoid truncating an existing file with wb
.
DevilutionX/Source/mpq/mpq_writer.cpp
Lines 97 to 99 in ecb3e46
// FileExists() may return false in the case of an error | |
// so we use "ab" instead of "wb" to avoid accidentally | |
// truncating an existing file |
if (GetFileSize(path.c_str(), &size) && size > 0) { | ||
buffer.resize(size); | ||
if (FILE *file = OpenFile(path.c_str(), "rb")) { | ||
if (std::fread(buffer.data(), size, 1, file) == 1) { | ||
std::string_view content(buffer.data(), size); | ||
std::string line; | ||
for (size_t i = 0, start = 0; i <= content.size(); ++i) { | ||
if (i == content.size() || content[i] == '\n' || content[i] == '\r') { | ||
size_t end = i; | ||
if (start < end) { | ||
line.assign(content.substr(start, end - start)); | ||
if (!line.empty() && line[0] != '#') { | ||
std::transform(line.begin(), line.end(), line.begin(), [](unsigned char c) { return std::tolower(c); }); | ||
BannedWords.push_back(line); | ||
} | ||
} | ||
if (i + 1 < content.size() && content[i] == '\r' && content[i + 1] == '\n') | ||
i++; | ||
start = i + 1; | ||
} | ||
} | ||
} else { | ||
LogError("Failed to read bannedwords.txt at {}", path); | ||
} | ||
std::fclose(file); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably use one or more techniques to reduce the nesting in this section. We can flip some of the conditions in the if statements and use return
or continue
. We can also separate reading and parsing like we do in options.cpp
.
DevilutionX/Source/options.cpp
Lines 137 to 151 in 707ceb1
FILE *file = OpenFile(path.c_str(), "rb"); | |
if (file != nullptr) { | |
uintmax_t size; | |
if (GetFileSize(path.c_str(), &size)) { | |
buffer.resize(static_cast<size_t>(size)); | |
if (std::fread(buffer.data(), static_cast<size_t>(size), 1, file) != 1) { | |
const char *errorMessage = std::strerror(errno); | |
if (errorMessage == nullptr) errorMessage = ""; | |
LogError(LogCategory::System, "std::fread: failed with \"{}\"", errorMessage); | |
buffer.clear(); | |
} | |
} | |
std::fclose(file); | |
} | |
tl::expected<Ini, std::string> result = Ini::parse(std::string_view(buffer.data(), buffer.size())); |
if (start < end) { | ||
line.assign(content.substr(start, end - start)); | ||
if (!line.empty() && line[0] != '#') { | ||
std::transform(line.begin(), line.end(), line.begin(), [](unsigned char c) { return std::tolower(c); }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
std::tolower
relies on the C locale, which we try to avoid.
{ | ||
std::string result = original; | ||
std::string lowered = original; | ||
std::transform(lowered.begin(), lowered.end(), lowered.begin(), [](unsigned char c) { return std::tolower(c); }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto
|
||
for (const std::string &word : BannedWords) { | ||
size_t pos = 0; | ||
while ((pos = lowered.find(word, pos)) != std::string::npos) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should be checking for word boundaries so we can avoid what might be considered a cl***ic mistake.
Adds a new file in the config directory
bannedwords.txt
. This file accepts a word on each line to be filtered in Multiplayer chat.Fixes: #7964