Settings: Fail on invalid sequence and throw exception for LuaSettings

diff --git a/src/script/lua_api/l_settings.cpp b/src/script/lua_api/l_settings.cpp
index 13a88ee9598b4ffcdd37910e4fec3f2476f4a6c6..9c88a3e05e7c64dd1bdf94d5e0a9600e0e4d1b14 100644 (file)
--- a/src/script/lua_api/l_settings.cpp
+++ b/src/script/lua_api/l_settings.cpp
@@ -73,9 +73,10 @@ int LuaSettings::l_set(lua_State* L)
        std::string key = std::string(luaL_checkstring(L, 2));
        const char* value = luaL_checkstring(L, 3);
 
-       o->m_settings->set(key, value);
+       if (!o->m_settings->set(key, value))
+               throw LuaError("Invalid sequence found in setting parameters");
 
-       return 1;
+       return 0;
 }
 
 // remove(self, key) -> success

diff --git a/src/settings.cpp b/src/settings.cpp
index 8d2e9fa6c8a699c0240e19dabdacdc8977da277c..a8b15b6c2b47f87e2349efbb50993a1ddd403dbb 100644 (file)
--- a/src/settings.cpp
+++ b/src/settings.cpp
@@ -63,6 +63,30 @@ Settings & Settings::operator = (const Settings &other)
 }
 
 
+bool Settings::checkNameValid(const std::string &name)
+{
+       size_t pos = name.find_first_of("\t\n\v\f\r\b =\"{}#");
+       if (pos != std::string::npos) {
+               errorstream << "Invalid character '" << name[pos]
+                       << "' found in setting name" << std::endl;
+               return false;
+       }
+       return true;
+}
+
+
+bool Settings::checkValueValid(const std::string &value)
+{
+       if (value.substr(0, 3) == "\"\"\"" ||
+               value.find("\n\"\"\"") != std::string::npos) {
+               errorstream << "Invalid character sequence '\"\"\"' found in"
+                       " setting value" << std::endl;
+               return false;
+       }
+       return true;
+}
+
+
 std::string Settings::sanitizeName(const std::string &name)
 {
        std::string n(name);
@@ -704,112 +728,119 @@ bool Settings::getFlagStrNoEx(const std::string &name, u32 &val,
  * Setters *
  ***********/
 
-void Settings::setEntry(const std::string &name, const void *data,
+bool Settings::setEntry(const std::string &name, const void *data,
        bool set_group, bool set_default)
 {
        Settings *old_group = NULL;
 
-       std::string n = sanitizeName(name);
+       if (!checkNameValid(name))
+               return false;
+       if (!set_group && !checkValueValid(*(const std::string *)data))
+               return false;
 
        {
                JMutexAutoLock lock(m_mutex);
 
-               SettingsEntry &entry = set_default ? m_defaults[n] : m_settings[n];
+               SettingsEntry &entry = set_default ? m_defaults[name] : m_settings[name];
                old_group = entry.group;
 
-               entry.value    = set_group ? "" : sanitizeValue(*(const std::string *)data);
+               entry.value    = set_group ? "" : *(const std::string *)data;
                entry.group    = set_group ? *(Settings **)data : NULL;
                entry.is_group = set_group;
        }
 
        delete old_group;
+
+       return true;
 }
 
 
-void Settings::set(const std::string &name, const std::string &value)
+bool Settings::set(const std::string &name, const std::string &value)
 {
-       setEntry(name, &value, false, false);
+       if (!setEntry(name, &value, false, false))
+               return false;
 
        doCallbacks(name);
+       return true;
 }
 
 
-void Settings::setDefault(const std::string &name, const std::string &value)
+bool Settings::setDefault(const std::string &name, const std::string &value)
 {
-       setEntry(name, &value, false, true);
+       return setEntry(name, &value, false, true);
 }
 
 
-void Settings::setGroup(const std::string &name, Settings *group)
+bool Settings::setGroup(const std::string &name, Settings *group)
 {
-       setEntry(name, &group, true, false);
+       return setEntry(name, &group, true, false);
 }
 
 
-void Settings::setGroupDefault(const std::string &name, Settings *group)
+bool Settings::setGroupDefault(const std::string &name, Settings *group)
 {
-       setEntry(name, &group, true, true);
+       return setEntry(name, &group, true, true);
 }
 
 
-void Settings::setBool(const std::string &name, bool value)
+bool Settings::setBool(const std::string &name, bool value)
 {
-       set(name, value ? "true" : "false");
+       return set(name, value ? "true" : "false");
 }
 
 
-void Settings::setS16(const std::string &name, s16 value)
+bool Settings::setS16(const std::string &name, s16 value)
 {
-       set(name, itos(value));
+       return set(name, itos(value));
 }
 
 
-void Settings::setU16(const std::string &name, u16 value)
+bool Settings::setU16(const std::string &name, u16 value)
 {
-       set(name, itos(value));
+       return set(name, itos(value));
 }
 
 
-void Settings::setS32(const std::string &name, s32 value)
+bool Settings::setS32(const std::string &name, s32 value)
 {
-       set(name, itos(value));
+       return set(name, itos(value));
 }
 
 
-void Settings::setU64(const std::string &name, u64 value)
+bool Settings::setU64(const std::string &name, u64 value)
 {
        std::ostringstream os;
        os << value;
-       set(name, os.str());
+       return set(name, os.str());
 }
 
 
-void Settings::setFloat(const std::string &name, float value)
+bool Settings::setFloat(const std::string &name, float value)
 {
-       set(name, ftos(value));
+       return set(name, ftos(value));
 }
 
 
-void Settings::setV2F(const std::string &name, v2f value)
+bool Settings::setV2F(const std::string &name, v2f value)
 {
        std::ostringstream os;
        os << "(" << value.X << "," << value.Y << ")";
-       set(name, os.str());
+       return set(name, os.str());
 }
 
 
-void Settings::setV3F(const std::string &name, v3f value)
+bool Settings::setV3F(const std::string &name, v3f value)
 {
        std::ostringstream os;
        os << "(" << value.X << "," << value.Y << "," << value.Z << ")";
-       set(name, os.str());
+       return set(name, os.str());
 }
 
 
-void Settings::setFlagStr(const std::string &name, u32 flags,
+bool Settings::setFlagStr(const std::string &name, u32 flags,
        const FlagDesc *flagdesc, u32 flagmask)
 {
-       set(name, writeFlagString(flags, flagdesc, flagmask));
+       return set(name, writeFlagString(flags, flagdesc, flagmask));
 }
 
 
@@ -820,12 +851,11 @@ bool Settings::setStruct(const std::string &name, const std::string &format,
        if (!serializeStructToString(&structstr, format, value))
                return false;
 
-       set(name, structstr);
-       return true;
+       return set(name, structstr);
 }
 
 
-void Settings::setNoiseParams(const std::string &name,
+bool Settings::setNoiseParams(const std::string &name,
        const NoiseParams &np, bool set_default)
 {
        Settings *group = new Settings;
@@ -839,7 +869,7 @@ void Settings::setNoiseParams(const std::string &name,
        group->setFloat("lacunarity",  np.lacunarity);
        group->setFlagStr("flags",     np.flags, flagdesc_noiseparams, np.flags);
 
-       setEntry(name, &group, true, set_default);
+       return setEntry(name, &group, true, set_default);
 }
 
 

diff --git a/src/settings.h b/src/settings.h
index ba7e9c1442d9b1f171af7f08f52acebde474bb96..89f7589dfddc1bd40932133009ba1c52d6be25e1 100644 (file)
--- a/src/settings.h
+++ b/src/settings.h
@@ -112,6 +112,8 @@ public:
        bool updateConfigObject(std::istream &is, std::ostream &os,
                const std::string &end, u32 tab_depth=0);
 
+       static bool checkNameValid(const std::string &name);
+       static bool checkValueValid(const std::string &value);
        static std::string sanitizeName(const std::string &name);
        static std::string sanitizeValue(const std::string &value);
        static std::string getMultiline(std::istream &is, size_t *num_lines=NULL);
@@ -175,23 +177,23 @@ public:
 
        // N.B. Groups not allocated with new must be set to NULL in the settings
        // tree before object destruction.
-       void setEntry(const std::string &name, const void *entry,
+       bool setEntry(const std::string &name, const void *entry,
                bool set_group, bool set_default);
-       void set(const std::string &name, const std::string &value);
-       void setDefault(const std::string &name, const std::string &value);
-       void setGroup(const std::string &name, Settings *group);
-       void setGroupDefault(const std::string &name, Settings *group);
-       void setBool(const std::string &name, bool value);
-       void setS16(const std::string &name, s16 value);
-       void setU16(const std::string &name, u16 value);
-       void setS32(const std::string &name, s32 value);
-       void setU64(const std::string &name, u64 value);
-       void setFloat(const std::string &name, float value);
-       void setV2F(const std::string &name, v2f value);
-       void setV3F(const std::string &name, v3f value);
-       void setFlagStr(const std::string &name, u32 flags,
+       bool set(const std::string &name, const std::string &value);
+       bool setDefault(const std::string &name, const std::string &value);
+       bool setGroup(const std::string &name, Settings *group);
+       bool setGroupDefault(const std::string &name, Settings *group);
+       bool setBool(const std::string &name, bool value);
+       bool setS16(const std::string &name, s16 value);
+       bool setU16(const std::string &name, u16 value);
+       bool setS32(const std::string &name, s32 value);
+       bool setU64(const std::string &name, u64 value);
+       bool setFloat(const std::string &name, float value);
+       bool setV2F(const std::string &name, v2f value);
+       bool setV3F(const std::string &name, v3f value);
+       bool setFlagStr(const std::string &name, u32 flags,
                const FlagDesc *flagdesc, u32 flagmask);
-       void setNoiseParams(const std::string &name, const NoiseParams &np,
+       bool setNoiseParams(const std::string &name, const NoiseParams &np,
                bool set_default=false);
        // N.B. if setStruct() is used to write a non-POD aggregate type,
        // the behavior is undefined.

diff --git a/src/test.cpp b/src/test.cpp
index e0e17bed4d05efd2842949d5750a6e63f62c4b8d..072bda8ef79ab63bd5abae70662328667918db68 100644 (file)
--- a/src/test.cpp
+++ b/src/test.cpp
@@ -531,9 +531,12 @@ struct TestSettings: public TestBase
                group2->setS16("num_oranges", 53);
                group2->setGroup("animals", group3);
                group2->set("animals", "cute"); //destroys group 3
+               s.setGroup("groupy_thing", group2);
 
-               // the bad chars in here should be stripped
-               s.setGroup("groupy  \"_\"  thing", group2);
+               // Test set failure conditions
+               UASSERT(s.set("Zoop = Poop\nsome_other_setting", "false") == false);
+               UASSERT(s.set("sneaky", "\"\"\"\njabberwocky = false") == false);
+               UASSERT(s.set("hehe", "asdfasdf\n\"\"\"\nsomething = false") == false);
 
                // Test multiline settings
                UASSERT(group->get("ccc") == "testy\n   testa   ");