From 81c5943453fa7039912edadf51e68901a013b6a5 Mon Sep 17 00:00:00 2001 From: Lee Clagett Date: Sun, 17 Nov 2019 04:26:32 +0000 Subject: [PATCH 1/3] Remove temporary std::string creation in some hex->bin calls --- contrib/epee/include/string_tools.h | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/contrib/epee/include/string_tools.h b/contrib/epee/include/string_tools.h index 319c0121b..f9581a4e4 100644 --- a/contrib/epee/include/string_tools.h +++ b/contrib/epee/include/string_tools.h @@ -42,6 +42,7 @@ #include #include #include +#include #include "misc_log_ex.h" #include "storages/parserse_base_utils.h" #include "hex.h" @@ -69,7 +70,7 @@ namespace string_tools return to_hex::string(to_byte_span(to_span(src))); } //---------------------------------------------------------------------------- - inline bool parse_hexstr_to_binbuff(const epee::span s, epee::span& res) + inline bool parse_hexstr_to_binbuff(const boost::string_ref s, epee::span res) { if (s.size() != res.size() * 2) return false; @@ -90,7 +91,7 @@ namespace string_tools return true; } //---------------------------------------------------------------------------- - inline bool parse_hexstr_to_binbuff(const std::string& s, std::string& res) + inline bool parse_hexstr_to_binbuff(const boost::string_ref s, std::string& res) { if (s.size() & 1) return false; @@ -303,7 +304,7 @@ POP_WARNINGS } //---------------------------------------------------------------------------- template - bool hex_to_pod(const std::string& hex_str, t_pod_type& s) + bool hex_to_pod(const boost::string_ref hex_str, t_pod_type& s) { static_assert(std::is_pod::value, "expected pod type"); if(sizeof(s)*2 != hex_str.size()) @@ -313,13 +314,13 @@ POP_WARNINGS } //---------------------------------------------------------------------------- template - bool hex_to_pod(const std::string& hex_str, tools::scrubbed& s) + bool hex_to_pod(const boost::string_ref hex_str, tools::scrubbed& s) { return hex_to_pod(hex_str, unwrap(s)); } //---------------------------------------------------------------------------- template - bool hex_to_pod(const std::string& hex_str, epee::mlocked& s) + bool hex_to_pod(const boost::string_ref hex_str, epee::mlocked& s) { return hex_to_pod(hex_str, unwrap(s)); } From 5fcc23ae0a08e6846a4d0fbfdb51627559929068 Mon Sep 17 00:00:00 2001 From: Lee Clagett Date: Sun, 17 Nov 2019 04:29:18 +0000 Subject: [PATCH 2/3] Move hex->bin conversion to monero copyright files and with less includes --- contrib/epee/include/hex.h | 17 +++++-- .../include/storages/parserse_base_utils.h | 2 + contrib/epee/include/string_tools.h | 34 ++------------ contrib/epee/src/hex.cpp | 41 ++++++++++++++++- src/rpc/rpc_args.cpp | 4 +- src/wallet/wallet2.cpp | 2 +- tests/unit_tests/epee_utils.cpp | 46 +++++++++++++++++-- 7 files changed, 104 insertions(+), 42 deletions(-) diff --git a/contrib/epee/include/hex.h b/contrib/epee/include/hex.h index 4e8f90786..8443cb92f 100644 --- a/contrib/epee/include/hex.h +++ b/contrib/epee/include/hex.h @@ -1,4 +1,4 @@ -// Copyright (c) 2017-2019, The Monero Project +// Copyright (c) 2017-2020, The Monero Project // // All rights reserved. // @@ -80,9 +80,20 @@ namespace epee static void buffer_unchecked(char* out, const span src) noexcept; }; + //! Convert hex in UTF8 encoding to binary struct from_hex { - //! \return An std::vector of unsigned integers from the `src` - static std::vector vector(boost::string_ref src); + static bool to_string(std::string& out, boost::string_ref src); + + static bool to_buffer(span out, boost::string_ref src) noexcept; + + private: + static bool to_buffer_unchecked(std::uint8_t* out, boost::string_ref src) noexcept; + }; + + //! Convert hex in current C locale encoding to binary + struct from_hex_locale + { + static std::vector to_vector(boost::string_ref src); }; } diff --git a/contrib/epee/include/storages/parserse_base_utils.h b/contrib/epee/include/storages/parserse_base_utils.h index 8a498130c..2256f6b83 100644 --- a/contrib/epee/include/storages/parserse_base_utils.h +++ b/contrib/epee/include/storages/parserse_base_utils.h @@ -31,6 +31,8 @@ #include #include +#include "misc_log_ex.h" + #undef MONERO_DEFAULT_LOG_CATEGORY #define MONERO_DEFAULT_LOG_CATEGORY "serialization" diff --git a/contrib/epee/include/string_tools.h b/contrib/epee/include/string_tools.h index f9581a4e4..2d9317d60 100644 --- a/contrib/epee/include/string_tools.h +++ b/contrib/epee/include/string_tools.h @@ -70,34 +70,9 @@ namespace string_tools return to_hex::string(to_byte_span(to_span(src))); } //---------------------------------------------------------------------------- - inline bool parse_hexstr_to_binbuff(const boost::string_ref s, epee::span res) - { - if (s.size() != res.size() * 2) - return false; - - unsigned char *dst = (unsigned char *)&res[0]; - const unsigned char *src = (const unsigned char *)s.data(); - for(size_t i = 0; i < s.size(); i += 2) - { - int tmp = *src++; - tmp = epee::misc_utils::parse::isx[tmp]; - if (tmp == 0xff) return false; - int t2 = *src++; - t2 = epee::misc_utils::parse::isx[t2]; - if (t2 == 0xff) return false; - *dst++ = (tmp << 4) | t2; - } - - return true; - } - //---------------------------------------------------------------------------- inline bool parse_hexstr_to_binbuff(const boost::string_ref s, std::string& res) { - if (s.size() & 1) - return false; - res.resize(s.size() / 2); - epee::span rspan((char*)&res[0], res.size()); - return parse_hexstr_to_binbuff(epee::to_span(s), rspan); + return from_hex::to_string(res, s); } //---------------------------------------------------------------------------- PUSH_WARNINGS @@ -306,11 +281,8 @@ POP_WARNINGS template bool hex_to_pod(const boost::string_ref hex_str, t_pod_type& s) { - static_assert(std::is_pod::value, "expected pod type"); - if(sizeof(s)*2 != hex_str.size()) - return false; - epee::span rspan((char*)&s, sizeof(s)); - return parse_hexstr_to_binbuff(epee::to_span(hex_str), rspan); + static_assert(std::is_standard_layout(), "expected standard layout type"); + return from_hex::to_buffer(as_mut_byte_span(s), hex_str); } //---------------------------------------------------------------------------- template diff --git a/contrib/epee/src/hex.cpp b/contrib/epee/src/hex.cpp index 558983f7e..b25ce5421 100644 --- a/contrib/epee/src/hex.cpp +++ b/contrib/epee/src/hex.cpp @@ -1,4 +1,4 @@ -// Copyright (c) 2017-2019, The Monero Project +// Copyright (c) 2017-2020, The Monero Project // // All rights reserved. // @@ -33,6 +33,8 @@ #include #include +#include "storages/parserse_base_utils.h" + namespace epee { namespace @@ -84,7 +86,42 @@ namespace epee return write_hex(out, src); } - std::vector from_hex::vector(boost::string_ref src) + + bool from_hex::to_string(std::string& out, const boost::string_ref src) + { + out.resize(src.size() / 2); + return to_buffer_unchecked(reinterpret_cast(&out[0]), src); + } + + bool from_hex::to_buffer(span out, const boost::string_ref src) noexcept + { + if (src.size() / 2 != out.size()) + return false; + return to_buffer_unchecked(out.data(), src); + } + + bool from_hex::to_buffer_unchecked(std::uint8_t* dst, const boost::string_ref s) noexcept + { + if (s.size() % 2 != 0) + return false; + + const unsigned char *src = (const unsigned char *)s.data(); + for(size_t i = 0; i < s.size(); i += 2) + { + int tmp = *src++; + tmp = epee::misc_utils::parse::isx[tmp]; + if (tmp == 0xff) return false; + int t2 = *src++; + t2 = epee::misc_utils::parse::isx[t2]; + if (t2 == 0xff) return false; + *dst++ = (tmp << 4) | t2; + } + + return true; + } + + + std::vector from_hex_locale::to_vector(const boost::string_ref src) { // should we include a specific character auto include = [](char input) { diff --git a/src/rpc/rpc_args.cpp b/src/rpc/rpc_args.cpp index dcb804d3e..9153e76ea 100644 --- a/src/rpc/rpc_args.cpp +++ b/src/rpc/rpc_args.cpp @@ -1,4 +1,4 @@ -// Copyright (c) 2014-2019, The Monero Project +// Copyright (c) 2014-2020, The Monero Project // // All rights reserved. // @@ -51,7 +51,7 @@ namespace cryptonote const std::vector ssl_allowed_fingerprints = command_line::get_arg(vm, arg.rpc_ssl_allowed_fingerprints); std::vector> allowed_fingerprints{ ssl_allowed_fingerprints.size() }; - std::transform(ssl_allowed_fingerprints.begin(), ssl_allowed_fingerprints.end(), allowed_fingerprints.begin(), epee::from_hex::vector); + std::transform(ssl_allowed_fingerprints.begin(), ssl_allowed_fingerprints.end(), allowed_fingerprints.begin(), epee::from_hex_locale::to_vector); for (const auto &fpr: allowed_fingerprints) { if (fpr.size() != SSL_FINGERPRINT_SIZE) diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index 7ee32a436..6008dc210 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -357,7 +357,7 @@ std::unique_ptr make_basic(const boost::program_options::variabl else if (!daemon_ssl_ca_file.empty() || !daemon_ssl_allowed_fingerprints.empty()) { std::vector> ssl_allowed_fingerprints{ daemon_ssl_allowed_fingerprints.size() }; - std::transform(daemon_ssl_allowed_fingerprints.begin(), daemon_ssl_allowed_fingerprints.end(), ssl_allowed_fingerprints.begin(), epee::from_hex::vector); + std::transform(daemon_ssl_allowed_fingerprints.begin(), daemon_ssl_allowed_fingerprints.end(), ssl_allowed_fingerprints.begin(), epee::from_hex_locale::to_vector); for (const auto &fpr: ssl_allowed_fingerprints) { THROW_WALLET_EXCEPTION_IF(fpr.size() != SSL_FINGERPRINT_SIZE, tools::error::wallet_internal_error, diff --git a/tests/unit_tests/epee_utils.cpp b/tests/unit_tests/epee_utils.cpp index ee44ea2d5..055fc8d46 100644 --- a/tests/unit_tests/epee_utils.cpp +++ b/tests/unit_tests/epee_utils.cpp @@ -823,14 +823,14 @@ TEST(ToHex, String) } -TEST(FromHex, String) +TEST(HexLocale, String) { // the source data to encode and decode std::vector source{{ 0x00, 0xFF, 0x0F, 0xF0 }}; // encode and decode the data auto hex = epee::to_hex::string({ source.data(), source.size() }); - auto decoded = epee::from_hex::vector(hex); + auto decoded = epee::from_hex_locale::to_vector(hex); // encoded should be twice the size and should decode to the exact same data EXPECT_EQ(source.size() * 2, hex.size()); @@ -839,7 +839,7 @@ TEST(FromHex, String) // we will now create a padded hex string, we want to explicitly allow // decoding it this way also, ignoring spaces and colons between the numbers hex.assign("00:ff 0f:f0"); - EXPECT_EQ(source, epee::from_hex::vector(hex)); + EXPECT_EQ(source, epee::from_hex_locale::to_vector(hex)); } TEST(ToHex, Array) @@ -901,6 +901,46 @@ TEST(ToHex, Formatted) EXPECT_EQ(expected, out.str()); } +TEST(FromHex, ToString) +{ + static constexpr const char hex[] = u8"deadbeeffY"; + static constexpr const char binary[] = { + char(0xde), char(0xad), char(0xbe), char(0xef), 0x00 + }; + + std::string out{}; + EXPECT_FALSE(epee::from_hex::to_string(out, hex)); + + boost::string_ref portion{hex}; + portion.remove_suffix(1); + EXPECT_FALSE(epee::from_hex::to_string(out, portion)); + + portion.remove_suffix(1); + EXPECT_TRUE(epee::from_hex::to_string(out, portion)); + EXPECT_EQ(std::string{binary}, out); +} + +TEST(FromHex, ToBuffer) +{ + static constexpr const char hex[] = u8"deadbeeffY"; + static constexpr const std::uint8_t binary[] = {0xde, 0xad, 0xbe, 0xef}; + + std::vector out{}; + out.resize(sizeof(binary)); + EXPECT_FALSE(epee::from_hex::to_buffer(epee::to_mut_span(out), hex)); + + boost::string_ref portion{hex}; + portion.remove_suffix(1); + EXPECT_FALSE(epee::from_hex::to_buffer(epee::to_mut_span(out), portion)); + + portion.remove_suffix(1); + EXPECT_FALSE(epee::from_hex::to_buffer({out.data(), out.size() - 1}, portion)); + + EXPECT_TRUE(epee::from_hex::to_buffer(epee::to_mut_span(out), portion)); + const std::vector expected{std::begin(binary), std::end(binary)}; + EXPECT_EQ(expected, out); +} + TEST(StringTools, BuffToHex) { const std::vector all_bytes = get_all_bytes(); From 3387f0e327cb817c5f7f7f4c3d5dcc4990747cd1 Mon Sep 17 00:00:00 2001 From: Lee Clagett Date: Sun, 17 Nov 2019 04:30:19 +0000 Subject: [PATCH 3/3] Reduce template bloat in hex->bin for ZMQ json --- src/serialization/json_object.cpp | 21 +++++++++++++++++++-- src/serialization/json_object.h | 21 +++++++-------------- 2 files changed, 26 insertions(+), 16 deletions(-) diff --git a/src/serialization/json_object.cpp b/src/serialization/json_object.cpp index e98ba0483..926eb18c0 100644 --- a/src/serialization/json_object.cpp +++ b/src/serialization/json_object.cpp @@ -1,4 +1,4 @@ -// Copyright (c) 2016-2019, The Monero Project +// Copyright (c) 2016-2020, The Monero Project // // All rights reserved. // @@ -32,7 +32,11 @@ #include #include #include -#include "string_tools.h" + +// drop macro from windows.h +#ifdef GetObject + #undef GetObject +#endif namespace cryptonote { @@ -109,6 +113,19 @@ namespace } } +void read_hex(const rapidjson::Value& val, epee::span dest) +{ + if (!val.IsString()) + { + throw WRONG_TYPE("string"); + } + + if (!epee::from_hex::to_buffer(dest, {val.GetString(), val.Size()})) + { + throw BAD_INPUT(); + } +} + void toJsonValue(rapidjson::Writer& dest, const rapidjson::Value& src) { src.Accept(dest); diff --git a/src/serialization/json_object.h b/src/serialization/json_object.h index a1a5105d5..664b539b5 100644 --- a/src/serialization/json_object.h +++ b/src/serialization/json_object.h @@ -1,4 +1,4 @@ -// Copyright (c) 2016-2019, The Monero Project +// Copyright (c) 2016-2020, The Monero Project // // All rights reserved. // @@ -30,7 +30,6 @@ #include #include -#include "string_tools.h" // out of order because windows.h GetObject macro conflicts with GenericValue<..>::GetObject() #include #include #include @@ -39,6 +38,8 @@ #include "rpc/message_data_structs.h" #include "cryptonote_protocol/cryptonote_protocol_defs.h" #include "common/sfinae_helpers.h" +#include "hex.h" +#include "span.h" #define OBJECT_HAS_MEMBER_OR_THROW(val, key) \ do \ @@ -118,6 +119,8 @@ inline constexpr bool is_to_hex() return std::is_pod() && !std::is_integral(); } +void read_hex(const rapidjson::Value& val, epee::span dest); + // POD to json key template inline typename std::enable_if()>::type toJsonKey(rapidjson::Writer& dest, const Type& pod) @@ -137,18 +140,8 @@ inline typename std::enable_if()>::type toJsonValue(rapidjson::W template inline typename std::enable_if()>::type fromJsonValue(const rapidjson::Value& val, Type& t) { - if (!val.IsString()) - { - throw WRONG_TYPE("string"); - } - - //TODO: handle failure to convert hex string to POD type - bool success = epee::string_tools::hex_to_pod(val.GetString(), t); - - if (!success) - { - throw BAD_INPUT(); - } + static_assert(std::is_standard_layout(), "expected standard layout type"); + json::read_hex(val, epee::as_mut_byte_span(t)); } void toJsonValue(rapidjson::Writer& dest, const rapidjson::Value& src);