From dffcdea12b564734f789a637b70cd2e347862d67 Mon Sep 17 00:00:00 2001 From: Scott Schurr Date: Thu, 23 Mar 2023 17:32:17 -0700 Subject: [PATCH] fix: `Expected` to return a value: (#4401) Fix a case where `ripple::Expected` returned a json array, not a value. The problem was that `Expected` invoked the wrong constructor for the expected type, which resulted in a constructor that took multiple arguments being interpreted as an array. A proposed fix was provided by @godexsoft, which involved a minor adjustment to three constructors that replaces the use of curly braces with parentheses. This makes `Expected` usable for [Clio](https://github.com/XRPLF/clio). A unit test is also included to ensure that the issue doesn't occur again in the future. --- src/ripple/basics/Expected.h | 6 +++--- src/test/basics/Expected_test.cpp | 13 +++++++++++++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/ripple/basics/Expected.h b/src/ripple/basics/Expected.h index 8dc368eef..bb699579b 100644 --- a/src/ripple/basics/Expected.h +++ b/src/ripple/basics/Expected.h @@ -137,14 +137,14 @@ class [[nodiscard]] Expected public: template requires std::convertible_to constexpr Expected(U && r) - : Base(T{std::forward(r)}) + : Base(T(std::forward(r))) { } template requires std::convertible_to && (!std::is_reference_v)constexpr Expected(Unexpected e) - : Base(E{std::move(e.value())}) + : Base(E(std::move(e.value()))) { } @@ -220,7 +220,7 @@ public: template requires std::convertible_to && (!std::is_reference_v)constexpr Expected(Unexpected e) - : Base(E{std::move(e.value())}) + : Base(E(std::move(e.value()))) { } diff --git a/src/test/basics/Expected_test.cpp b/src/test/basics/Expected_test.cpp index 1f16e724d..b89b9f6d3 100644 --- a/src/test/basics/Expected_test.cpp +++ b/src/test/basics/Expected_test.cpp @@ -20,6 +20,9 @@ #include #include #include +#if BOOST_VERSION >= 107500 +#include // Not part of boost before version 1.75 +#endif // BOOST_VERSION #include #include @@ -203,6 +206,16 @@ struct Expected_test : beast::unit_test::suite std::string const s(std::move(expected.error())); BEAST_EXPECT(s == "Not what is expected!"); } + // Test a case that previously unintentionally returned an array. +#if BOOST_VERSION >= 107500 + { + auto expected = []() -> Expected { + return boost::json::object{{"oops", "me array now"}}; + }(); + BEAST_EXPECT(expected); + BEAST_EXPECT(!expected.value().is_array()); + } +#endif // BOOST_VERSION } };