(mesos) branch master updated: Fixed inf scalar value parsing.

[bmahler]

This is an automated email from the ASF dual-hosted git repository.

bmahler pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git


The following commit(s) were added to refs/heads/master by this push:
     new 9c0787682 Fixed inf scalar value parsing.
9c0787682 is described below

commit 9c0787682c5f4c169b42146f376b8552fa740559
Author: Ilya Pronin <ipro...@twitter.com>
AuthorDate: Wed Dec 4 16:59:36 2019 -0800

    Fixed inf scalar value parsing.
    
    Infinite and NaN values are not usable with SCALAR values. However,
    there are use-cases where they are valid TEXT values, e.g. in the agent
    attribute specifying its rack name.
    
    Differential Revision: https://phabricator.twitter.biz/D409445
---
 src/common/values.cpp      | 20 ++++++++++++++++----
 src/tests/values_tests.cpp | 42 ++++++++++++++++++++++++++++++++++--------
 src/v1/values.cpp          | 21 ++++++++++++++++-----
 3 files changed, 66 insertions(+), 17 deletions(-)

diff --git a/src/common/values.cpp b/src/common/values.cpp
index d7bc91bcf..4dfb4cbbe 100644
--- a/src/common/values.cpp
+++ b/src/common/values.cpp
@@ -732,12 +732,24 @@ Try<Value> parse(const string& text)
     } else if (index == string::npos) {
       Try<double> value_ = numify<double>(temp);
       if (!value_.isError()) {
-        Option<Error> error =
-          common::validation::validateInputScalarValue(*value_);
-        if (error.isSome()) {
-          return Error("Invalid scalar value '" + temp + "':" + 
error->message);
+        // Check if this is a floating point value that we can work
+        // with. If not, we will handle it as text.
+        switch (std::fpclassify(value_.get())) {
+          case FP_INFINITE:
+          case FP_NAN:
+            value_ = Error("Unsupported floating point value");
+            break;
+          case FP_NORMAL:
+            if (value_.get() < 0) {
+              value_ = Error("Negative values not supported");
+            }
+            break;
+          case FP_SUBNORMAL:
+            return Error("Subnormal values not supported");
         }
+      }
 
+      if (!value_.isError()) {
         // This is a scalar.
         Value::Scalar* scalar = value.mutable_scalar();
         value.set_type(Value::SCALAR);
diff --git a/src/tests/values_tests.cpp b/src/tests/values_tests.cpp
index d67e4eb86..27b41e512 100644
--- a/src/tests/values_tests.cpp
+++ b/src/tests/values_tests.cpp
@@ -73,6 +73,37 @@ TEST(ValuesTest, ValidInput)
   ASSERT_SOME(result4);
   ASSERT_EQ(Value::TEXT, result4->type());
   ASSERT_EQ("123abc,s", result4->text().value());
+
+  // Test parsing unsupported scalar values.
+  Try<Value> result5 = parse("inf");
+  ASSERT_SOME(result5);
+  ASSERT_EQ(Value::TEXT, result5->type());
+  ASSERT_EQ("inf", result5->text().value());
+
+  Try<Value> result6 = parse("-inf");
+  ASSERT_SOME(result6);
+  ASSERT_EQ(Value::TEXT, result6->type());
+  ASSERT_EQ("-inf", result6->text().value());
+
+  Try<Value> result7 = parse("infinity");
+  ASSERT_SOME(result7);
+  ASSERT_EQ(Value::TEXT, result7->type());
+  ASSERT_EQ("infinity", result7->text().value());
+
+  Try<Value> result8 = parse("-infinity");
+  ASSERT_SOME(result8);
+  ASSERT_EQ(Value::TEXT, result8->type());
+  ASSERT_EQ("-infinity", result8->text().value());
+
+  Try<Value> result9 = parse("nan");
+  ASSERT_SOME(result9);
+  ASSERT_EQ(Value::TEXT, result9->type());
+  ASSERT_EQ("nan", result9->text().value());
+
+  Try<Value> result10 = parse("-nan");
+  ASSERT_SOME(result10);
+  ASSERT_EQ(Value::TEXT, result10->type());
+  ASSERT_EQ("-nan", result10->text().value());
 }
 
 
@@ -90,14 +121,9 @@ TEST(ValuesTest, InvalidInput)
   // Test when giving empty string.
   EXPECT_ERROR(parse("  "));
 
-  EXPECT_ERROR(parse("nan"));
-  EXPECT_ERROR(parse("-nan"));
-
-  EXPECT_ERROR(parse("inf"));
-  EXPECT_ERROR(parse("-inf"));
-
-  EXPECT_ERROR(parse("infinity"));
-  EXPECT_ERROR(parse("-infinity"));
+  // Test subnormal scalar value.
+  EXPECT_ERROR(parse("7.63918e-313"));
+  EXPECT_ERROR(parse("-7.63918e-313"));
 }
 
 
diff --git a/src/v1/values.cpp b/src/v1/values.cpp
index 6193dbd8b..917302431 100644
--- a/src/v1/values.cpp
+++ b/src/v1/values.cpp
@@ -755,13 +755,24 @@ Try<Value> parse(const string& text)
     } else if (index == string::npos) {
       Try<double> value_ = numify<double>(temp);
       if (!value_.isError()) {
-        Option<Error> error =
-          mesos::internal::common::validation::validateInputScalarValue(
-              *value_);
-        if (error.isSome()) {
-          return Error("Invalid scalar value '" + temp + "':" + 
error->message);
+        // Check if this is a floating point value that we can work
+        // with. If not, we will handle it as text.
+        switch (std::fpclassify(value_.get())) {
+          case FP_INFINITE:
+          case FP_NAN:
+            value_ = Error("Unsupported floating point value");
+            break;
+          case FP_NORMAL:
+            if (value_.get() < 0) {
+              value_ = Error("Negative values not supported");
+            }
+            break;
+          case FP_SUBNORMAL:
+            return Error("Subnormal values not supported");
         }
+      }
 
+      if (!value_.isError()) {
         // This is a scalar.
         Value::Scalar* scalar = value.mutable_scalar();
         value.set_type(Value::SCALAR);