fix: optional serializer edge cases (#8044)

* fix: should reject when deserializing wrong optional type

* fix: reject nested optionals in serializer

libtransmission/serializer.h

@@ -433,19 +433,27 @@ bool to_array(tr_variant const& src, C* const ptgt)
 template<typename T>
 tr_variant from_optional(std::optional<T> const& src)
 {
+    static_assert(!is_optional_v<T>);
     return src ? Converters::serialize(*src) : nullptr;
 }
 
 template<typename T>
 bool to_optional(tr_variant const& src, std::optional<T>* ptgt)
 {
+    static_assert(!is_optional_v<T>);
     if (src.index() == tr_variant::NullIndex)
     {
         ptgt->reset();
         return true;
     }
-    *ptgt = T{};
-    return Converters::deserialize(src, &**ptgt);
+
+    if (auto const val = to_value<T>(src))
+    {
+        *ptgt = std::move(val);
+        return true;
+    }
+
+    return false;
 }
 
 } // namespace detail

tests/libtransmission/serializer-tests.cc

@@ -247,6 +247,52 @@ TEST_F(SerializerTest, vectorIsNondestructiveOnPartialFailure)
     EXPECT_EQ(out, (std::vector<std::string>{ "keep" }));
 }
 
+TEST_F(SerializerTest, usesOptional)
+{
+    auto const expected = std::optional{ "apple"s };
+    auto const var = Converters::serialize(expected);
+
+    auto const sv = var.value_if<std::string_view>();
+    ASSERT_EQ(sv, "apple"sv);
+
+    auto actual = decltype(expected){};
+    EXPECT_TRUE(Converters::deserialize(var, &actual));
+    EXPECT_EQ(actual, expected);
+}
+
+TEST_F(SerializerTest, usesNullOptional)
+{
+    auto const expected = std::optional<std::string>{};
+    auto const var = Converters::serialize(expected);
+
+    auto const sv = var.value_if<std::string_view>();
+    ASSERT_FALSE(sv);
+
+    auto actual = decltype(expected){ "discard"s };
+    EXPECT_TRUE(Converters::deserialize(var, &actual));
+    EXPECT_EQ(actual, expected);
+}
+
+TEST_F(SerializerTest, usesOptionalOfCustom)
+{
+    registerRectConverter();
+
+    constexpr auto Expected = std::optional{ Rect{ 1, 2, 3, 4 } };
+    auto const var = Converters::serialize(Expected);
+
+    auto actual = decltype(Expected){};
+    EXPECT_TRUE(Converters::deserialize(var, &actual));
+    EXPECT_EQ(actual, Expected);
+}
+
+TEST_F(SerializerTest, optionalRejectsWrongType)
+{
+    auto const var = tr_variant{ true };
+    auto out = std::optional{ "keep"s };
+    EXPECT_FALSE(Converters::deserialize(var, &out));
+    EXPECT_EQ(out, "keep"s);
+}
+
 // ---
 
 using libtransmission::serializer::Field;