settings: Be more strict in converting settings to specific data types

As the behavior was inconsistent for empty strings or strings with characters
appended to a number, testing the code failed on some platforms. The new rules
are more strict, returning the default if additional characters or an empty
string was found for a setting.

diff --git a/src/libstrongswan/settings/settings.c b/src/libstrongswan/settings/settings.c
index 6c69782ec342c8417e2118c75702d5ddc122224d..4fa283a6ffc57464e424e4293a5ba8063520e9c8 100644 (file)
--- a/src/libstrongswan/settings/settings.c
+++ b/src/libstrongswan/settings/settings.c
@@ -509,11 +509,13 @@ METHOD(settings_t, get_bool, bool,
 inline int settings_value_as_int(char *value, int def)
 {
        int intval;
+       char *end;
+
        if (value)
        {
                errno = 0;
-               intval = strtol(value, NULL, 10);
-               if (errno == 0)
+               intval = strtol(value, &end, 10);
+               if (errno == 0 && *end == 0 && end != value)
                {
                        return intval;
                }
@@ -539,11 +541,13 @@ METHOD(settings_t, get_int, int,
 inline double settings_value_as_double(char *value, double def)
 {
        double dval;
+       char *end;
+
        if (value)
        {
                errno = 0;
-               dval = strtod(value, NULL);
-               if (errno == 0)
+               dval = strtod(value, &end);
+               if (errno == 0 && *end == 0 && end != value)
                {
                        return dval;
                }
@@ -574,6 +578,10 @@ inline u_int32_t settings_value_as_time(char *value, u_int32_t def)
        {
                errno = 0;
                timeval = strtoul(value, &endptr, 10);
+               if (endptr == value)
+               {
+                       return def;
+               }
                if (errno == 0)
                {
                        switch (*endptr)
@@ -588,8 +596,10 @@ inline u_int32_t settings_value_as_time(char *value, u_int32_t def)
                                        timeval *= 60;
                                        break;
                                case 's':               /* time in seconds */
-                               default:
+                               case '\0':
                                        break;
+                               default:
+                                       return def;
                        }
                        return timeval;
                }

diff --git a/src/libstrongswan/tests/suites/test_settings.c b/src/libstrongswan/tests/suites/test_settings.c
index 32676be26ae9a501c25d61b0664bd45c655a2f8f..3348d33140d76e627785ada55f2b32bc2ee02d5c 100644 (file)
--- a/src/libstrongswan/tests/suites/test_settings.c
+++ b/src/libstrongswan/tests/suites/test_settings.c
@@ -265,7 +265,6 @@ START_SETUP(setup_int_config)
        create_settings(chunk_from_str(
                "main {\n"
                "       key1 = 5\n"
-               "       # gets cut off\n"
                "       key2 = 5.5\n"
                "       key3 = -42\n"
                "       empty = \"\"\n"
@@ -283,16 +282,14 @@ END_SETUP
 START_TEST(test_get_int)
 {
        verify_int(5, 0, "main.key1");
-       verify_int(5, 0, "main.key2");
+       verify_int(0, 0, "main.key2");
        verify_int(-42, 0, "main.key3");
 
-       verify_int(0, 11, "main.empty");
+       verify_int(11, 11, "main.empty");
        verify_int(11, 11, "main.none");
-
-       /* FIXME: do we want this behavior? */
-       verify_int(0, 11, "main.foo1");
-       verify_int(0, 11, "main.foo2");
-       verify_int(13, 11, "main.foo3");
+       verify_int(11, 11, "main.foo1");
+       verify_int(11, 11, "main.foo2");
+       verify_int(11, 11, "main.foo3");
 
        verify_int(13, 13, "main.key4");
        verify_int(-13, -13, "main");
@@ -341,13 +338,11 @@ START_TEST(test_get_double)
        verify_double(-42, 0, "main.key3");
        verify_double(-42.5, 0, "main.key4");
 
-       verify_double(0, 11.5, "main.empty");
+       verify_double(11.5, 11.5, "main.empty");
        verify_double(11.5, 11.5, "main.none");
-
-       /* FIXME: do we want this behavior? */
-       verify_double(0, 11.5, "main.foo1");
-       verify_double(0, 11.5, "main.foo2");
-       verify_double(13.5, 11.5, "main.foo3");
+       verify_double(11.5, 11.5, "main.foo1");
+       verify_double(11.5, 11.5, "main.foo2");
+       verify_double(11.5, 11.5, "main.foo3");
 
        verify_double(11.5, 11.5, "main.key5");
        verify_double(-11.5, -11.5, "main");
@@ -375,6 +370,7 @@ START_SETUP(setup_time_config)
 {
        create_settings(chunk_from_str(
                "main {\n"
+               "       key0 = 5\n"
                "       key1 = 5s\n"
                "       key2 = 5m\n"
                "       key3 = 5h\n"
@@ -393,18 +389,17 @@ END_SETUP
 
 START_TEST(test_get_time)
 {
+       verify_time(5, 0, "main.key0");
        verify_time(5, 0, "main.key1");
        verify_time(300, 0, "main.key2");
        verify_time(18000, 0, "main.key3");
        verify_time(432000, 0, "main.key4");
 
-       verify_time(0, 11, "main.empty");
+       verify_time(11, 11, "main.empty");
        verify_time(11, 11, "main.none");
-
-       /* FIXME: do we want this behavior? */
-       verify_time(0, 11, "main.foo1");
-       verify_time(0, 11, "main.foo2");
-       verify_time(13, 11, "main.foo3");
+       verify_time(11, 11, "main.foo1");
+       verify_time(11, 11, "main.foo2");
+       verify_time(11, 11, "main.foo3");
 
        verify_time(11, 11, "main.key5");
        verify_time(11, 11, "main");