From 4b22dd49991b376157f3c9b84850b826bb3df6c5 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Fri, 22 Jan 2016 12:05:28 -0800 Subject: [PATCH] (FACT-1317) Fix YAML output of non-numeric types Several formats intended to be non-numeric are interpreted as numbers in (Ruby) YAML. Mac and IPv6 addresses as sexagesimal types: 10:10:10:10:10:10:10:10 01:23:45:54:32:21 Comma-delimited lists as comma-delimited integers: 1,2,3,4,5 123,456 Modify YAML quoting to ensure these patterns are quoted. --- acceptance/tests/verify_facts.rb | 4 +- lib/src/util/string.cc | 23 +++++++---- lib/tests/facts/string_value.cc | 68 ++++++++++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 11 deletions(-) diff --git a/acceptance/tests/verify_facts.rb b/acceptance/tests/verify_facts.rb index 4eb148aabf..797955c8f4 100644 --- a/acceptance/tests/verify_facts.rb +++ b/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' diff --git a/lib/src/util/string.cc b/lib/src/util/string.cc index 4e5c5a9e38..c05385fe90 100644 --- a/lib/src/util/string.cc +++ b/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)) { diff --git a/lib/tests/facts/string_value.cc b/lib/tests/facts/string_value.cc index 677e336165..0ce4ef396a 100644 --- a/lib/tests/facts/string_value.cc +++ b/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"); + } + } + } }