This is an automated email from the ASF dual-hosted git repository. mzhu pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/mesos.git
commit c8170fad123c22b531bfb90c8bac294848d36ea2 Author: Meng Zhu <[email protected]> AuthorDate: Mon Mar 11 13:32:56 2019 -0700 Added validation method for input scalar values. The method validates the input scalar values to be non-negative normal values ("normal" as defined by `std::fpclassify`). Also refactored some related validation logic and tests. Review: https://reviews.apache.org/r/70198 --- src/common/validation.cpp | 43 +++++++++++++++++++++++------------ src/common/validation.hpp | 2 ++ src/common/values.cpp | 14 +++--------- src/tests/common_validation_tests.cpp | 10 +++++--- src/tests/master_validation_tests.cpp | 3 ++- src/v1/values.cpp | 16 ++++--------- 6 files changed, 47 insertions(+), 41 deletions(-) diff --git a/src/common/validation.cpp b/src/common/validation.cpp index b622a32..b42053e 100644 --- a/src/common/validation.cpp +++ b/src/common/validation.cpp @@ -626,27 +626,40 @@ Option<Error> validateOfferFilters(const OfferFilters& offerFilters) return Error("Resource quantities must contain at least one quantity"); } - // Use `auto` instead of `protobuf::MapPair<strinf, Value::>` since + // Use `auto` instead of `protobuf::MapPair<string, Value::>` since // `foreach` is a macro and does not allow angle brackets. foreach (auto&& quantity, quantities.quantities()) { - const Value::Scalar& scalar = quantity.second; - - // We do not allow negative quantities in offer filters. - if (scalar.value() < 0) { - return Error("Negative resource quantities are not allowed"); + Option<Error> error = validateInputScalarValue(quantity.second.value()); + if (error.isSome()) { + return Error( + "Invalid resource quantity for '" + quantity.first + "': " + + error->message); } + } + } + } - // Resource quantities cannot have NaN values. - if (std::isnan(scalar.value())) { - return Error("Resource quantities cannot be NaN"); - } + return None(); +} - // We do not allow infinite quantities in offer filters. - if (std::isinf(scalar.value())) { - return Error("Infinite resource quantities are not allowed"); - } +Option<Error> validateInputScalarValue(double value) +{ + switch (std::fpclassify(value)) { + case FP_INFINITE: + return Error("Infinite values not supported"); + case FP_NAN: + return Error("NaN not supported"); + case FP_SUBNORMAL: + return Error("Subnormal values not supported"); + case FP_ZERO: + break; + case FP_NORMAL: + if (value < 0) { + return Error("Negative values not supported"); } - } + break; + default: + return Error("Unknown error"); } return None(); diff --git a/src/common/validation.hpp b/src/common/validation.hpp index d09528f..2a7e8a2 100644 --- a/src/common/validation.hpp +++ b/src/common/validation.hpp @@ -71,6 +71,8 @@ Option<Error> validateExecutorCall(const mesos::executor::Call& call); Option<Error> validateOfferFilters(const OfferFilters& offerFilters); +Option<Error> validateInputScalarValue(double value); + } // namespace validation { } // namespace common { } // namespace internal { diff --git a/src/common/values.cpp b/src/common/values.cpp index b41c64e..7520382 100644 --- a/src/common/values.cpp +++ b/src/common/values.cpp @@ -15,6 +15,7 @@ // limitations under the License. #include "common/values.hpp" +#include "common/validation.hpp" #include <algorithm> #include <cmath> @@ -725,17 +726,8 @@ Try<Value> parse(const string& text) } else if (index == string::npos) { Try<double> value_ = numify<double>(temp); if (!value_.isError()) { - Option<Error> error = [value_]() -> Option<Error> { - switch (std::fpclassify(value_.get())) { - case FP_NORMAL: return None(); - case FP_ZERO: return None(); - case FP_INFINITE: return Error("Infinite values not supported"); - case FP_NAN: return Error("NaN not supported"); - case FP_SUBNORMAL: return Error("Subnormal values not supported"); - default: return Error("Unknown error"); - } - }(); - + Option<Error> error = + common::validation::validateInputScalarValue(*value_); if (error.isSome()) { return Error("Invalid scalar value '" + temp + "':" + error->message); } diff --git a/src/tests/common_validation_tests.cpp b/src/tests/common_validation_tests.cpp index 214b993..577192e 100644 --- a/src/tests/common_validation_tests.cpp +++ b/src/tests/common_validation_tests.cpp @@ -117,7 +117,9 @@ TEST(CommonValidationTest, OfferFilters) ->mutable_quantities() ->insert({"cpus", scalar}); EXPECT_SOME_EQ( - Error("Negative resource quantities are not allowed"), + Error( + "Invalid resource quantity for 'cpus': " + "Negative values not supported"), common::validation::validateOfferFilters(offerFilters)); } @@ -136,7 +138,7 @@ TEST(CommonValidationTest, OfferFilters) ->mutable_quantities() ->insert({"cpus", scalar}); EXPECT_SOME_EQ( - Error("Resource quantities cannot be NaN"), + Error("Invalid resource quantity for 'cpus': NaN not supported"), common::validation::validateOfferFilters(offerFilters)); } @@ -155,7 +157,9 @@ TEST(CommonValidationTest, OfferFilters) ->mutable_quantities() ->insert({"cpus", scalar}); EXPECT_SOME_EQ( - Error("Infinite resource quantities are not allowed"), + Error( + "Invalid resource quantity for 'cpus': " + "Infinite values not supported"), common::validation::validateOfferFilters(offerFilters)); } diff --git a/src/tests/master_validation_tests.cpp b/src/tests/master_validation_tests.cpp index 9568d39..63418c7 100644 --- a/src/tests/master_validation_tests.cpp +++ b/src/tests/master_validation_tests.cpp @@ -4739,7 +4739,8 @@ TEST_F(FrameworkInfoValidationTest, ValidateOfferFilters) {frameworkInfo.roles(0), offerFilters}); EXPECT_SOME_EQ( - Error("Negative resource quantities are not allowed"), + Error("Invalid resource quantity for 'cpus': " + "Negative values not supported"), framework::validate(frameworkInfo)); } diff --git a/src/v1/values.cpp b/src/v1/values.cpp index 4b58919..6193dbd 100644 --- a/src/v1/values.cpp +++ b/src/v1/values.cpp @@ -35,6 +35,8 @@ #include <stout/interval.hpp> #include <stout/strings.hpp> +#include "common/validation.hpp" + using std::max; using std::min; using std::ostream; @@ -753,17 +755,9 @@ Try<Value> parse(const string& text) } else if (index == string::npos) { Try<double> value_ = numify<double>(temp); if (!value_.isError()) { - Option<Error> error = [value_]() -> Option<Error> { - switch (std::fpclassify(value_.get())) { - case FP_NORMAL: return None(); - case FP_ZERO: return None(); - case FP_INFINITE: return Error("Infinite values not supported"); - case FP_NAN: return Error("NaN not supported"); - case FP_SUBNORMAL: return Error("Subnormal values not supported"); - default: return Error("Unknown error"); - } - }(); - + Option<Error> error = + mesos::internal::common::validation::validateInputScalarValue( + *value_); if (error.isSome()) { return Error("Invalid scalar value '" + temp + "':" + error->message); }
