Merge 4b22dd4 into c08a5ed

acceptance/tests/verify_facts.rb

@@ -29,9 +29,7 @@ def validate_fact(name, node, fact_value, hidden)
               # YAML-CPP seems to drop the decimal whenever it feels like it, so just match Int too.
               fact_value.is_a? Float or fact_value.is_a? Integer or fact_value.to_s =~ /^(\.inf|\.nan|-\.inf)$/
             when 'string'
-              # Any string can be interpreted as a more specific type by ruby
-              # YAML.
-              true
+              fact_value.nil? or fact_value.is_a? String or fact_value.is_a? TrueValue or fact_value.is_a? FalseValue
             when 'ip'
               fact_value.is_a? String and fact_value =~ @ip_pattern
             when 'ip6'

lib/src/util/string.cc

@@ -114,26 +114,33 @@ namespace facter { namespace util {
             return true;
         }
 
-        // Check that it doesn't start with the ':' special character. This interferes with parsing.
-        if (str[0] == ':') {
+        // A string starting with the ':' special character interferes with parsing.
+        // Other string patterns containing ':' can be unexpectedly parsed as sexagesimal.
+        // To be safe quote all strings that contain ':' characters.
+        if (str.find(':') != string::npos) {
             return true;
         }
 
-        // Poor man's check for a numerical string
+        // Poor man's check for a numerical string or list of numbers
         // May start with - or +
-        // May contain at most one . or ,
+        // May contain one . and one or more ,
         // All other characters should be digits
-        bool has_separator = false;
+        // Note: Ruby YAML interprets 1,2,3 as a number, but not 1.2.3.
+        // It doesn't appear to honor locales when parsing YAML.
+        bool has_dot = false;
         for (size_t i = 0; i < str.size(); ++i) {
             char c = str[i];
             if (i == 0 && (c == '+' || c == '-')) {
                 continue;
             }
-            if (c == '.' || c == ',') {
-                if (has_separator) {
+            if (c == ',') {
+                continue;
+            }
+            if (c == '.') {
+                if (has_dot) {
                     return false;
                 }
-                has_separator = true;
+                has_dot = true;
                 continue;
             }
             if (!isdigit(c)) {

lib/tests/facts/string_value.cc

@@ -162,4 +162,72 @@ SCENARIO("using a string fact value") {
             }
         }
     }
+
+    GIVEN("a valid mac address") {
+        string_value value("00:50:56:55:42:45");
+        WHEN("serialized to JSON") {
+            THEN("it should have the same value") {
+                json_value json;
+                json_allocator allocator;
+                value.to_json(allocator, json);
+                REQUIRE(json.IsString());
+                REQUIRE(json.GetString() == string("00:50:56:55:42:45"));
+            }
+        }
+        WHEN("serialized to YAML") {
+            THEN("it should be quoted") {
+                Emitter emitter;
+                value.write(emitter);
+                REQUIRE(string(emitter.c_str()) == "\"00:50:56:55:42:45\"");
+            }
+        }
+        WHEN("serialized to text with quotes") {
+            THEN("it should be quoted") {
+                ostringstream stream;
+                value.write(stream);
+                REQUIRE(stream.str() == "\"00:50:56:55:42:45\"");
+                }
+            }
+        WHEN("serialized to text without quotes") {
+            THEN("it should not be quoted") {
+                ostringstream stream;
+                value.write(stream, false);
+                REQUIRE(stream.str() == "00:50:56:55:42:45");
+            }
+        }
+    }
+
+    GIVEN("a list of numbers") {
+        string_value value("1,2,3,4,5");
+        WHEN("serialized to JSON") {
+            THEN("it should have the same value") {
+                json_value json;
+                json_allocator allocator;
+                value.to_json(allocator, json);
+                REQUIRE(json.IsString());
+                REQUIRE(json.GetString() == string("1,2,3,4,5"));
+            }
+        }
+        WHEN("serialized to YAML") {
+            THEN("it should be quoted") {
+                Emitter emitter;
+                value.write(emitter);
+                REQUIRE(string(emitter.c_str()) == "\"1,2,3,4,5\"");
+            }
+        }
+        WHEN("serialized to text with quotes") {
+            THEN("it should be quoted") {
+                ostringstream stream;
+                value.write(stream);
+                REQUIRE(stream.str() == "\"1,2,3,4,5\"");
+                }
+            }
+        WHEN("serialized to text without quotes") {
+            THEN("it should not be quoted") {
+                ostringstream stream;
+                value.write(stream, false);
+                REQUIRE(stream.str() == "1,2,3,4,5");
+            }
+        }
+    }
 }