From 96a65f544209fab619ab26226aaf39abcc5d1cdf Mon Sep 17 00:00:00 2001 From: Tom Jackson Date: Wed, 12 Jul 2017 11:32:18 -0700 Subject: [PATCH] Don't use range construction for associative containers in convertTo() Summary: This avoids having exceptions occur while constructing hashtables. Reviewed By: ot, yfeldblum Differential Revision: D5384419 fbshipit-source-id: ba2de8cffa46df550bdc62abbe529801249e45cd --- folly/DynamicConverter.h | 106 ++++++++++++++++------------ folly/test/DynamicConverterTest.cpp | 13 +++- 2 files changed, 71 insertions(+), 48 deletions(-) diff --git a/folly/DynamicConverter.h b/folly/DynamicConverter.h index e569f641..5da62711 100644 --- a/folly/DynamicConverter.h +++ b/folly/DynamicConverter.h @@ -19,6 +19,8 @@ #pragma once #include +#include + namespace folly { template T convertTo(const dynamic&); template dynamic toDynamic(const T&); @@ -54,6 +56,7 @@ namespace dynamicconverter_detail { BOOST_MPL_HAS_XXX_TRAIT_DEF(value_type); BOOST_MPL_HAS_XXX_TRAIT_DEF(iterator); BOOST_MPL_HAS_XXX_TRAIT_DEF(mapped_type); +BOOST_MPL_HAS_XXX_TRAIT_DEF(key_type); template struct iterator_class_is_container { typedef std::reverse_iterator some_iterator; @@ -62,38 +65,20 @@ template struct iterator_class_is_container { }; template -using class_is_container = typename - std::conditional< - has_iterator::value, - iterator_class_is_container, - std::false_type - >::type; - -template struct class_is_range { - enum { value = has_value_type::value && - has_iterator::value }; -}; +using class_is_container = + Conjunction, iterator_class_is_container>; +template +using is_range = StrictConjunction, has_iterator>; -template struct is_container - : std::conditional< - std::is_class::value, - class_is_container, - std::false_type - >::type {}; +template +using is_container = StrictConjunction, class_is_container>; -template struct is_range - : std::conditional< - std::is_class::value, - class_is_range, - std::false_type - >::type {}; +template +using is_map = StrictConjunction, has_mapped_type>; -template struct is_map - : std::integral_constant< - bool, - is_range::value && has_mapped_type::value - > {}; +template +using is_associative = StrictConjunction, has_key_type>; } // namespace dynamicconverter_detail @@ -143,11 +128,9 @@ struct Dereferencer> { }; template -class Transformer : public boost::iterator_adaptor< - Transformer, - It, - typename T::value_type - > { +class Transformer + : public boost:: + iterator_adaptor, It, typename T::value_type> { friend class boost::iterator_core_access; typedef typename T::value_type ttype; @@ -169,15 +152,14 @@ class Transformer : public boost::iterator_adaptor< return cache_; } -public: + public: explicit Transformer(const It& it) - : Transformer::iterator_adaptor_(it), valid_(false) {} + : Transformer::iterator_adaptor_(it), valid_(false) {} }; // conversion factory template -inline std::move_iterator> -conversionIterator(const It& it) { +inline std::move_iterator> conversionIterator(const It& it) { return std::make_move_iterator(Transformer(it)); } @@ -204,9 +186,10 @@ struct DynamicConverter { // integrals template -struct DynamicConverter::value && - !std::is_same::value>::type> { +struct DynamicConverter< + T, + typename std::enable_if< + std::is_integral::value && !std::is_same::value>::type> { static T convert(const dynamic& d) { return folly::to(d.asInt()); } @@ -214,8 +197,9 @@ struct DynamicConverter -struct DynamicConverter::value>::type> { +struct DynamicConverter< + T, + typename std::enable_if::value>::type> { static T convert(const dynamic& d) { using type = typename std::underlying_type::type; return static_cast(DynamicConverter::convert(d)); @@ -224,7 +208,8 @@ struct DynamicConverter -struct DynamicConverter::value>::type> { static T convert(const dynamic& d) { return folly::to(d.asDouble()); @@ -249,7 +234,7 @@ struct DynamicConverter { // std::pair template -struct DynamicConverter> { +struct DynamicConverter> { static std::pair convert(const dynamic& d) { if (d.isArray() && d.size() == 2) { return std::make_pair(convertTo(d[0]), convertTo(d[1])); @@ -262,11 +247,13 @@ struct DynamicConverter> { } }; -// containers +// non-associative containers template -struct DynamicConverter::value>::type> { + dynamicconverter_detail::is_container::value && + !dynamicconverter_detail::is_associative::value>::type> { static C convert(const dynamic& d) { if (d.isArray()) { return C(dynamicconverter_detail::conversionIterator(d.begin()), @@ -282,6 +269,31 @@ struct DynamicConverter +struct DynamicConverter< + C, + typename std::enable_if< + dynamicconverter_detail::is_container::value && + dynamicconverter_detail::is_associative::value>::type> { + static C convert(const dynamic& d) { + C ret; // avoid direct initialization due to unordered_map's constructor + // causing memory corruption if the iterator throws an exception + if (d.isArray()) { + ret.insert( + dynamicconverter_detail::conversionIterator(d.begin()), + dynamicconverter_detail::conversionIterator(d.end())); + } else if (d.isObject()) { + ret.insert( + dynamicconverter_detail::conversionIterator(d.items().begin()), + dynamicconverter_detail::conversionIterator(d.items().end())); + } else { + throw TypeError("object or array", d.type()); + } + return ret; + } +}; + /////////////////////////////////////////////////////////////////////////////// // DynamicConstructor specializations diff --git a/folly/test/DynamicConverterTest.cpp b/folly/test/DynamicConverterTest.cpp index 042d1694..af5cbac8 100644 --- a/folly/test/DynamicConverterTest.cpp +++ b/folly/test/DynamicConverterTest.cpp @@ -411,7 +411,18 @@ TEST(DynamicConverter, partial_dynamics) { EXPECT_EQ(d, toDynamic(c)); std::unordered_map m{{"one", 1}, {"two", 2}}; - dynamic md = dynamic::object("one", 1)("two", 2); EXPECT_EQ(md, toDynamic(m)); } + +TEST(DynamicConverter, asan_exception_case_umap) { + EXPECT_THROW( + (convertTo>(dynamic::array(1))), TypeError); +} + +TEST(DynamicConverter, asan_exception_case_uset) { + EXPECT_THROW( + (convertTo>( + dynamic::array(1, dynamic::array(), 3))), + TypeError); +} -- 2.34.1