[Scummvm-git-logs] scummvm master -> ddee0e76f92a5696d76c8ca826a5f5e6ad43b25f
bluegr
noreply at scummvm.org
Sun Apr 10 16:41:09 UTC 2022
This automated email contains information about 1 new commit which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .
Summary:
ddee0e76f9 COMMON: INIFile bugfixes and unit tests
Commit: ddee0e76f92a5696d76c8ca826a5f5e6ad43b25f
https://github.com/scummvm/scummvm/commit/ddee0e76f92a5696d76c8ca826a5f5e6ad43b25f
Author: Matthew Duggan (mgithub at guarana.org)
Date: 2022-04-10T19:41:06+03:00
Commit Message:
COMMON: INIFile bugfixes and unit tests
This adds a lot more unit tests for Common::INIFile and fixes some small
problems identified during their creation. Specifically:
* addSection() did not check the validity of the section name, which meant it
was possible to create a section that could not then be operated on by the
other functions (which all check validity)
* If allowNonEnglishCharacters was set, it was possible to create a key or
section name starting with '[' or '#', or including a carriage return. This
would not read back in correctly.
* Blank section and key names were considered valid, but neither would be
read back in correctly.
* The struct documentation did not mention whitespcae handling or that comments
include the '#' and carriage return.
Changed paths:
common/ini-file.cpp
common/ini-file.h
test/common/ini-file.h
diff --git a/common/ini-file.cpp b/common/ini-file.cpp
index 820ddc9a359..95d6efc5bfb 100644
--- a/common/ini-file.cpp
+++ b/common/ini-file.cpp
@@ -27,12 +27,25 @@
namespace Common {
+bool INIFile::isValidChar(char c) const {
+ if (_allowNonEnglishCharacters) {
+ // Chars that can break parsing are never allowed
+ return !(c == '[' || c == ']' || c == '=' || c == '#' || c == '\r' || c == '\n');
+ } else {
+ // Only some chars are allowed
+ return isAlnum(c) || c == '-' || c == '_' || c == '.' || c == ' ' || c == ':';
+ }
+}
+
bool INIFile::isValidName(const String &name) const {
- if (_allowNonEnglishCharacters)
- return true;
+ if (name.empty())
+ return false;
+
const char *p = name.c_str();
- while (*p && (isAlnum(*p) || *p == '-' || *p == '_' || *p == '.' || *p == ' ' || *p == ':'))
+ while (*p && isValidChar(*p)) {
p++;
+ }
+
return *p == 0;
}
@@ -237,6 +250,11 @@ bool INIFile::saveToStream(WriteStream &stream) {
}
void INIFile::addSection(const String §ion) {
+ if (!isValidName(section)) {
+ warning("Invalid section name: %s", section.c_str());
+ return;
+ }
+
Section *s = getSection(section);
if (s)
return;
diff --git a/common/ini-file.h b/common/ini-file.h
index 9c7618c5de6..51fad418b2f 100644
--- a/common/ini-file.h
+++ b/common/ini-file.h
@@ -57,9 +57,9 @@ class WriteStream;
class INIFile {
public:
struct KeyValue {
- String key; /*!< Key of the configuration entry. */
- String value; /*!< Value of the configuration entry. */
- String comment; /*!< Comment within an INI file. */
+ String key; /*!< Key of the configuration entry, whitespace trimmed. */
+ String value; /*!< Value of the configuration entry, whitespace trimmed. */
+ String comment; /*!< Comment within an INI file, including #s and newlines. */
};
typedef List<KeyValue> SectionKeyList; /*!< A list of all key/value pairs in this section. */
@@ -75,9 +75,9 @@ public:
* INI files manually.
*/
struct Section {
- String name; /*!< Name of the section. */
+ String name; /*!< Name of the section, whitespace trimmed. */
List<KeyValue> keys; /*!< List of all keys in this section. */
- String comment; /*!< Comment within the section. */
+ String comment; /*!< Comment within the section, including #s and newlines. */
bool hasKey(const String &key) const; /*!< Check whether the section has a @p key. */
const KeyValue* getKey(const String &key) const; /*!< Get the value assigned to a @p key. */
@@ -138,6 +138,7 @@ private:
Section *getSection(const String §ion);
const Section *getSection(const String §ion) const;
+ bool isValidChar(char c) const; /*!< Is given char allowed in section or key names*/
};
/** @} */
diff --git a/test/common/ini-file.h b/test/common/ini-file.h
index 5cf1ed92f04..c6a56d6e3ed 100644
--- a/test/common/ini-file.h
+++ b/test/common/ini-file.h
@@ -3,6 +3,7 @@
#include "common/ini-file.h"
#include "common/memstream.h"
+
class IniFileTestSuite : public CxxTest::TestSuite {
public:
void test_blank_ini_file() {
@@ -14,7 +15,6 @@ class IniFileTestSuite : public CxxTest::TestSuite {
TS_ASSERT_EQUALS(sections.size(), 0);
}
-
void test_simple_ini_file() {
static const unsigned char inistr[] = "#comment\n[s]\nabc=1\ndef=xyz";
Common::MemoryReadStream ms(inistr, sizeof(inistr));
@@ -39,7 +39,7 @@ class IniFileTestSuite : public CxxTest::TestSuite {
}
void test_multisection_ini_file() {
- static const unsigned char inistr[] = "[s]\nabc=1\ndef=xyz\n#comment=no\n[empty]\n\n[s2]\nabc=2";
+ static const unsigned char inistr[] = "[s]\nabc=1\ndef=xyz\n#comment=no\n[empty]\n\n[s2]\n abc = 2 ";
Common::MemoryReadStream ms(inistr, sizeof(inistr));
Common::INIFile inifile;
bool result = inifile.loadFromStream(ms);
@@ -59,6 +59,11 @@ class IniFileTestSuite : public CxxTest::TestSuite {
TS_ASSERT_EQUALS(val, "1");
TS_ASSERT(inifile.getKey("abc", "s2", val));
TS_ASSERT_EQUALS(val, "2");
+
+ inifile.clear();
+ sections = inifile.getSections();
+ TS_ASSERT_EQUALS(sections.size(), 0);
+ TS_ASSERT(!inifile.hasSection("s"));
}
void test_modify_ini_file() {
@@ -84,4 +89,84 @@ class IniFileTestSuite : public CxxTest::TestSuite {
inifile.removeSection("t");
TS_ASSERT(!inifile.hasSection("t"));
}
+
+ void test_name_validity() {
+ Common::INIFile inifile;
+
+ inifile.addSection("s*");
+ TS_ASSERT(!inifile.hasSection("s*"));
+
+ inifile.addSection("");
+ TS_ASSERT(!inifile.hasSection(""));
+
+ // Valid is alphanum plus [-_:. ]
+ inifile.addSection("sEcT10N -_..Name:");
+ TS_ASSERT(inifile.hasSection("sEcT10N -_..Name:"));
+
+ const char invalids[] = "!\"#$%&'()=~[]()+?<>\r\t\n";
+ for (int i = 0; i < sizeof(invalids) - 1; i++) {
+ char c = invalids[i];
+ const Common::String s(c);
+ inifile.addSection(s);
+ TS_ASSERT(!inifile.hasSection(s));
+ }
+
+ inifile.clear();
+ inifile.allowNonEnglishCharacters();
+ for (int i = 0; i < sizeof(invalids) - 1; i++) {
+ char c = invalids[i];
+ if (c == '[' || c == ']' || c == '#' || c == '=' || c == '\r' || c == '\n')
+ continue;
+ const Common::String s(c);
+ inifile.addSection(s);
+ TS_ASSERT(inifile.hasSection(s));
+ }
+ }
+
+ void test_write_simple_ini_file() {
+ byte buf[1024];
+ Common::INIFile inifile;
+ {
+ static const unsigned char inistr[] = "#comment\n[s]\nabc=1\ndef=xyz";
+ Common::MemoryReadStream mrs(inistr, sizeof(inistr));
+ TS_ASSERT(inifile.loadFromStream(mrs));
+ }
+
+ // A too-small write buffer (should fail)
+ {
+ Common::MemoryWriteStream mws(buf, 10);
+ TS_ASSERT(!inifile.saveToStream(mws));
+ }
+
+ // A good sized write buffer (should work)
+ int len;
+ {
+ Common::MemoryWriteStream mws(buf, 1024);
+ TS_ASSERT(inifile.saveToStream(mws));
+ len = mws.pos();
+ }
+
+ {
+ Common::MemoryReadStream mrs(buf, len - 1);
+ Common::INIFile checkinifile;
+ TS_ASSERT(checkinifile.loadFromStream(mrs));
+ TS_ASSERT(checkinifile.hasSection("s"));
+
+ const Common::INIFile::SectionList §ions = checkinifile.getSections();
+ const Common::INIFile::Section §ion = sections.front();
+ TS_ASSERT_EQUALS(section.comment, "#comment\n");
+ TS_ASSERT_EQUALS(section.name, "s");
+
+ TS_ASSERT(checkinifile.hasKey("abc", "s"));
+ TS_ASSERT(checkinifile.hasKey("def", "s"));
+
+ Common::String val;
+ TS_ASSERT(inifile.getKey("abc", "s", val));
+ TS_ASSERT_EQUALS(val, "1");
+ TS_ASSERT(inifile.getKey("def", "s", val));
+ TS_ASSERT_EQUALS(val, "xyz");
+ }
+
+ }
+
};
More information about the Scummvm-git-logs
mailing list