Fix File::GetDisplayName() not escaping special chars

When display_ is an empty string, GetDisplayName() returns name_, but
name_'s value might contain Markdown special characters, and
GetDisplayName() is supposed to return a Markdown string, though this
was poorly documented.
This commit is contained in:
Oliver Hamlet
2020-07-27 22:18:54 +01:00
parent b4dc6e2f3a
commit 36d90ee72b
6 changed files with 67 additions and 16 deletions
+8 -2
View File
@@ -47,7 +47,8 @@ public:
* @param name
* The filename of the file.
* @param display
* The name to be displayed for the file in messages.
* The name to be displayed for the file in messages, formatted using
* GitHub Flavored Markdown.
* @param condition
* The File's condition string.
* @return A File object.
@@ -77,7 +78,12 @@ public:
/**
* Get the display name of the file.
* @return The file's display name.
*
* If the File was constructed with an empty display string, the name field
* will be returned instead, with any `ASCII punctuation characters
* <https://github.github.com/gfm/#ascii-punctuation-character>`_ escaped.
* Escaping is not performed if returning the value of the display string.
* @return The file's display name or filename.
*/
LOOT_API std::string GetDisplayName() const;
+7
View File
@@ -71,6 +71,13 @@ const std::vector<regex> versionRegexes({
regex::ECMAScript | regex::icase),
});
// As defined by <https://github.github.com/gfm/#ascii-punctuation-character>.
const regex asciiPunctuationCharacters("([!\"#$%&'()*+,\\-./:;<=>?@\\[\\\\\\]^_`{|}~])");
std::string EscapeMarkdownASCIIPunctuation(const std::string& text) {
return std::regex_replace(text, asciiPunctuationCharacters, "\\$1");
}
std::vector<Tag> ExtractBashTags(const std::string& description) {
std::vector<Tag> tags;
+2
View File
@@ -32,6 +32,8 @@
#include "loot/metadata/tag.h"
namespace loot {
std::string EscapeMarkdownASCIIPunctuation(const std::string& text);
std::vector<Tag> ExtractBashTags(const std::string& description);
std::optional<std::string> ExtractVersion(const std::string& text);
+5 -4
View File
@@ -65,10 +65,11 @@ bool File::operator==(const File& rhs) const {
Filename File::GetName() const { return name_; }
std::string File::GetDisplayName() const {
if (display_.empty())
return std::string(name_);
else
return display_;
if (display_.empty()) {
return EscapeMarkdownASCIIPunctuation(std::string(name_));
}
return display_;
}
bool operator!=(const File& lhs, const File& rhs) { return !(lhs == rhs); }
+9 -3
View File
@@ -31,6 +31,7 @@
#include <yaml-cpp/yaml.h>
#include "loot/metadata/file.h"
#include "api/helpers/text.h"
namespace YAML {
template<>
@@ -42,8 +43,11 @@ struct convert<loot::File> {
if (rhs.IsConditional())
node["condition"] = rhs.GetCondition();
if (rhs.GetDisplayName() != std::string(rhs.GetName()))
auto escapedName =
loot::EscapeMarkdownASCIIPunctuation(std::string(rhs.GetName()));
if (rhs.GetDisplayName() != escapedName) {
node["display"] = rhs.GetDisplayName();
}
return node;
}
@@ -83,9 +87,11 @@ struct convert<loot::File> {
};
inline Emitter& operator<<(Emitter& out, const loot::File& rhs) {
auto escapedName = loot::EscapeMarkdownASCIIPunctuation(std::string(rhs.GetName()));
if (!rhs.IsConditional() &&
(rhs.GetDisplayName().empty() ||
rhs.GetDisplayName() == std::string(rhs.GetName())))
rhs.GetDisplayName() == escapedName))
out << YAML::SingleQuoted << std::string(rhs.GetName());
else {
out << BeginMap << Key << "name" << Value << YAML::SingleQuoted
@@ -95,7 +101,7 @@ inline Emitter& operator<<(Emitter& out, const loot::File& rhs) {
out << Key << "condition" << Value << YAML::SingleQuoted
<< rhs.GetCondition();
if (rhs.GetDisplayName() != std::string(rhs.GetName()))
if (rhs.GetDisplayName() != escapedName)
out << Key << "display" << Value << YAML::SingleQuoted
<< rhs.GetDisplayName();
+36 -7
View File
@@ -275,6 +275,31 @@ TEST(
EXPECT_TRUE(file2 >= file1);
}
TEST(File, getDisplayNameShouldReturnDisplayStringIfItIsNotEmpty) {
File file("name", "display");
EXPECT_EQ("display", file.GetDisplayName());
}
TEST(File, getDisplayNameShouldReturnNameStringIfDisplayStringIsEmpty) {
File file("name", "");
EXPECT_EQ("name", file.GetDisplayName());
}
TEST(File, getDisplayNameShouldNotEscapeASCIIPunctuationInDisplayString) {
auto display = "!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~";
File file("name", display);
EXPECT_EQ(display, file.GetDisplayName());
}
TEST(File, getDisplayNameShouldEscapeASCIIPunctuationInNameString) {
File file("!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~");
EXPECT_EQ(R"raw(\!\"\#\$\%\&\'\(\)\*\+\,\-\.\/\:\;\<\=\>\?\@\[\\\]\^\_\`\{\|\}\~)raw",
file.GetDisplayName());
}
TEST(File, emittingAsYamlShouldSingleQuoteValues) {
File file("name1", "display1", "condition1");
YAML::Emitter emitter;
@@ -287,19 +312,21 @@ TEST(File, emittingAsYamlShouldSingleQuoteValues) {
}
TEST(File, emittingAsYamlShouldOutputAsAScalarIfOnlyTheNameStringIsNotEmpty) {
File file("name1");
File file("file.esp");
YAML::Emitter emitter;
emitter << file;
EXPECT_EQ("'" + std::string(file.GetName()) + "'", emitter.c_str());
}
TEST(File, emittingAsYamlShouldOmitDisplayFieldIfItMatchesTheNameField) {
File file("name1", "name1");
TEST(
File,
emittingAsYamlShouldOmitDisplayFieldIfItMatchesTheNameFieldAfterEscapingASCIIPunctuation) {
File file("file.esp", "file\\.esp");
YAML::Emitter emitter;
emitter << file;
EXPECT_EQ("'" + std::string(file.GetName()) + "'", emitter.c_str());
EXPECT_STREQ("'file.esp'", emitter.c_str());
}
TEST(File, emittingAsYamlShouldOmitAnEmptyConditionString) {
@@ -323,7 +350,7 @@ TEST(File, encodingAsYamlShouldStoreDataCorrectly) {
}
TEST(File, encodingAsYamlShouldOmitEmptyFields) {
File file("name1");
File file("file.esp");
YAML::Node node;
node = file;
@@ -332,8 +359,10 @@ TEST(File, encodingAsYamlShouldOmitEmptyFields) {
EXPECT_FALSE(node["condition"]);
}
TEST(File, encodingAsYamlShouldOmitDisplayFieldIfItMatchesTheNameField) {
File file("name1", "name1");
TEST(
File,
encodingAsYamlShouldOmitDisplayFieldIfItMatchesTheNameFieldAfterEscapingASCIIPunctuation) {
File file("file.esp", "file\\.esp");
YAML::Node node;
node = file;