diff --git a/configure.ac b/configure.ac index 49fe3a22b0c..c7d124a1f17 100644 --- a/configure.ac +++ b/configure.ac @@ -1707,7 +1707,6 @@ AM_CONDITIONAL([ENABLE_QT_TESTS], [test "$BUILD_TEST_QT" = "yes"]) AM_CONDITIONAL([ENABLE_BENCH], [test "$use_bench" = "yes"]) AM_CONDITIONAL([USE_QRCODE], [test "$use_qr" = "yes"]) AM_CONDITIONAL([USE_LCOV], [test "$use_lcov" = "yes"]) -AM_CONDITIONAL([USE_LIBEVENT], [test "$use_libevent" = "yes"]) AM_CONDITIONAL([HARDEN], [test "$use_hardening" = "yes"]) AM_CONDITIONAL([ENABLE_SSE42], [test "$enable_sse42" = "yes"]) AM_CONDITIONAL([ENABLE_SSE41], [test "$enable_sse41" = "yes"]) diff --git a/src/Makefile.am b/src/Makefile.am index f48ed4d4425..c2e0c7b5b87 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -679,6 +679,7 @@ libbitcoin_common_a_SOURCES = \ common/run_command.cpp \ common/settings.cpp \ common/system.cpp \ + common/url.cpp \ compressor.cpp \ core_read.cpp \ core_write.cpp \ @@ -711,11 +712,6 @@ libbitcoin_common_a_SOURCES = \ script/solver.cpp \ warnings.cpp \ $(BITCOIN_CORE_H) - -if USE_LIBEVENT -libbitcoin_common_a_CPPFLAGS += $(EVENT_CFLAGS) -libbitcoin_common_a_SOURCES += common/url.cpp -endif # # util # diff --git a/src/Makefile.test.include b/src/Makefile.test.include index cf88a02b95f..942e0bf69b3 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -85,6 +85,7 @@ BITCOIN_TESTS =\ test/checkqueue_tests.cpp \ test/coins_tests.cpp \ test/coinstatsindex_tests.cpp \ + test/common_url_tests.cpp \ test/compilerbug_tests.cpp \ test/compress_tests.cpp \ test/crypto_tests.cpp \ diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index 161d703282e..8901d10ef63 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -11,7 +11,6 @@ #include #include #include -#include #include #include #include @@ -51,7 +50,6 @@ using CliClock = std::chrono::system_clock; const std::function G_TRANSLATION_FUN = nullptr; -UrlDecodeFn* const URL_DECODE = urlDecode; static const char DEFAULT_RPCCONNECT[] = "127.0.0.1"; static const int DEFAULT_HTTP_CLIENT_TIMEOUT=900; diff --git a/src/bitcoin-wallet.cpp b/src/bitcoin-wallet.cpp index d5dfbbec271..fe90958a5f5 100644 --- a/src/bitcoin-wallet.cpp +++ b/src/bitcoin-wallet.cpp @@ -11,7 +11,6 @@ #include #include #include -#include #include #include #include @@ -28,7 +27,6 @@ #include const std::function G_TRANSLATION_FUN = nullptr; -UrlDecodeFn* const URL_DECODE = nullptr; static void SetupWalletToolArgs(ArgsManager& argsman) { diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp index 4f0a816388e..54796c5abbf 100644 --- a/src/bitcoind.cpp +++ b/src/bitcoind.cpp @@ -12,7 +12,6 @@ #include #include #include -#include #include #include #include @@ -35,7 +34,6 @@ using node::NodeContext; const std::function G_TRANSLATION_FUN = nullptr; -UrlDecodeFn* const URL_DECODE = urlDecode; #if HAVE_DECL_FORK diff --git a/src/common/url.cpp b/src/common/url.cpp index 053e1a825c5..ecf88d07ea5 100644 --- a/src/common/url.cpp +++ b/src/common/url.cpp @@ -4,19 +4,36 @@ #include -#include - -#include +#include #include +#include +#include -std::string urlDecode(const std::string &urlEncoded) { +std::string UrlDecode(std::string_view url_encoded) +{ std::string res; - if (!urlEncoded.empty()) { - char *decoded = evhttp_uridecode(urlEncoded.c_str(), false, nullptr); - if (decoded) { - res = std::string(decoded); - free(decoded); + res.reserve(url_encoded.size()); + + for (size_t i = 0; i < url_encoded.size(); ++i) { + char c = url_encoded[i]; + // Special handling for percent which should be followed by two hex digits + // representing an octet values, see RFC 3986, Section 2.1 Percent-Encoding + if (c == '%' && i + 2 < url_encoded.size()) { + unsigned int decoded_value{0}; + auto [p, ec] = std::from_chars(url_encoded.data() + i + 1, url_encoded.data() + i + 3, decoded_value, 16); + + // Only if there is no error and the pointer is set to the end of + // the string, we can be sure both characters were valid hex + if (ec == std::errc{} && p == url_encoded.data() + i + 3) { + res += static_cast(decoded_value); + // Next two characters are part of the percent encoding + i += 2; + continue; + } + // In case of invalid percent encoding, add the '%' and continue } + res += c; } + return res; } diff --git a/src/common/url.h b/src/common/url.h index b16b8241af5..203f41c70f2 100644 --- a/src/common/url.h +++ b/src/common/url.h @@ -6,9 +6,12 @@ #define BITCOIN_COMMON_URL_H #include +#include -using UrlDecodeFn = std::string(const std::string& url_encoded); -UrlDecodeFn urlDecode; -extern UrlDecodeFn* const URL_DECODE; +/* Decode a URL. + * + * Notably this implementation does not decode a '+' to a ' '. + */ +std::string UrlDecode(std::string_view url_encoded); #endif // BITCOIN_COMMON_URL_H diff --git a/src/qt/main.cpp b/src/qt/main.cpp index ded057dbfaf..16befd99e89 100644 --- a/src/qt/main.cpp +++ b/src/qt/main.cpp @@ -4,7 +4,6 @@ #include -#include #include #include @@ -17,7 +16,6 @@ extern const std::function G_TRANSLATION_FUN = [](const char* psz) { return QCoreApplication::translate("bitcoin-core", psz).toStdString(); }; -UrlDecodeFn* const URL_DECODE = urlDecode; const std::function G_TEST_GET_FULL_NAME{}; diff --git a/src/test/common_url_tests.cpp b/src/test/common_url_tests.cpp new file mode 100644 index 00000000000..cc893cbed72 --- /dev/null +++ b/src/test/common_url_tests.cpp @@ -0,0 +1,72 @@ +// Copyright (c) 2024-present The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or https://opensource.org/license/mit/. + +#include + +#include + +#include + +BOOST_AUTO_TEST_SUITE(common_url_tests) + +// These test vectors were ported from test/regress.c in the libevent library +// which used to be a dependency of the UrlDecode function. + +BOOST_AUTO_TEST_CASE(encode_decode_test) { + BOOST_CHECK_EQUAL(UrlDecode("Hello"), "Hello"); + BOOST_CHECK_EQUAL(UrlDecode("99"), "99"); + BOOST_CHECK_EQUAL(UrlDecode(""), ""); + BOOST_CHECK_EQUAL(UrlDecode("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ123456789-.~_"), + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ123456789-.~_"); + BOOST_CHECK_EQUAL(UrlDecode("%20"), " "); + BOOST_CHECK_EQUAL(UrlDecode("%FF%F0%E0"), "\xff\xf0\xe0"); + BOOST_CHECK_EQUAL(UrlDecode("%01%19"), "\x01\x19"); + BOOST_CHECK_EQUAL(UrlDecode("http%3A%2F%2Fwww.ietf.org%2Frfc%2Frfc3986.txt"), + "http://www.ietf.org/rfc/rfc3986.txt"); + BOOST_CHECK_EQUAL(UrlDecode("1%2B2%3D3"), "1+2=3"); +} + +BOOST_AUTO_TEST_CASE(decode_malformed_test) { + BOOST_CHECK_EQUAL(UrlDecode("%%xhello th+ere \xff"), "%%xhello th+ere \xff"); + + BOOST_CHECK_EQUAL(UrlDecode("%"), "%"); + BOOST_CHECK_EQUAL(UrlDecode("%%"), "%%"); + BOOST_CHECK_EQUAL(UrlDecode("%%%"), "%%%"); + BOOST_CHECK_EQUAL(UrlDecode("%%%%"), "%%%%"); + + BOOST_CHECK_EQUAL(UrlDecode("+"), "+"); + BOOST_CHECK_EQUAL(UrlDecode("++"), "++"); + + BOOST_CHECK_EQUAL(UrlDecode("?"), "?"); + BOOST_CHECK_EQUAL(UrlDecode("??"), "??"); + + BOOST_CHECK_EQUAL(UrlDecode("%G1"), "%G1"); + BOOST_CHECK_EQUAL(UrlDecode("%2"), "%2"); + BOOST_CHECK_EQUAL(UrlDecode("%ZX"), "%ZX"); + + BOOST_CHECK_EQUAL(UrlDecode("valid%20string%G1"), "valid string%G1"); + BOOST_CHECK_EQUAL(UrlDecode("%20invalid%ZX"), " invalid%ZX"); + BOOST_CHECK_EQUAL(UrlDecode("%20%G1%ZX"), " %G1%ZX"); + + BOOST_CHECK_EQUAL(UrlDecode("%1 "), "%1 "); + BOOST_CHECK_EQUAL(UrlDecode("% 9"), "% 9"); + BOOST_CHECK_EQUAL(UrlDecode(" %Z "), " %Z "); + BOOST_CHECK_EQUAL(UrlDecode(" % X"), " % X"); + + BOOST_CHECK_EQUAL(UrlDecode("%-1"), "%-1"); + BOOST_CHECK_EQUAL(UrlDecode("%1-"), "%1-"); +} + +BOOST_AUTO_TEST_CASE(decode_lowercase_hex_test) { + BOOST_CHECK_EQUAL(UrlDecode("%f0%a0%b0"), "\xf0\xa0\xb0"); +} + +BOOST_AUTO_TEST_CASE(decode_internal_nulls_test) { + std::string result1{"\0\0x\0\0", 5}; + BOOST_CHECK_EQUAL(UrlDecode("%00%00x%00%00"), result1); + std::string result2{"abc\0\0", 5}; + BOOST_CHECK_EQUAL(UrlDecode("abc%00%00"), result2); +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/fuzz/string.cpp b/src/test/fuzz/string.cpp index e81efac6e0d..631da13803a 100644 --- a/src/test/fuzz/string.cpp +++ b/src/test/fuzz/string.cpp @@ -90,7 +90,7 @@ FUZZ_TARGET(string) (void)ToUpper(random_string_1); (void)TrimString(random_string_1); (void)TrimString(random_string_1, random_string_2); - (void)urlDecode(random_string_1); + (void)UrlDecode(random_string_1); (void)ContainsNoNUL(random_string_1); (void)_(random_string_1.c_str()); try { diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 2c18184261e..38350b33cc5 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -14,7 +14,6 @@ #include #include #include -#include #include #include #include @@ -81,7 +80,6 @@ using node::RegenerateCommitments; using node::VerifyLoadedChainstate; const std::function G_TRANSLATION_FUN = nullptr; -UrlDecodeFn* const URL_DECODE = nullptr; /** Random context to get unique temp data dirs. Separate from g_insecure_rand_ctx, which can be seeded from a const env var */ static FastRandomContext g_insecure_rand_ctx_temp_path; diff --git a/src/wallet/rpc/util.cpp b/src/wallet/rpc/util.cpp index 06ec7db1bc2..1252843e9d0 100644 --- a/src/wallet/rpc/util.cpp +++ b/src/wallet/rpc/util.cpp @@ -11,6 +11,7 @@ #include #include +#include #include #include @@ -61,9 +62,9 @@ bool ParseIncludeWatchonly(const UniValue& include_watchonly, const CWallet& wal bool GetWalletNameFromJSONRPCRequest(const JSONRPCRequest& request, std::string& wallet_name) { - if (URL_DECODE && request.URI.substr(0, WALLET_ENDPOINT_BASE.size()) == WALLET_ENDPOINT_BASE) { + if (request.URI.starts_with(WALLET_ENDPOINT_BASE)) { // wallet endpoint was used - wallet_name = URL_DECODE(request.URI.substr(WALLET_ENDPOINT_BASE.size())); + wallet_name = UrlDecode(std::string_view{request.URI}.substr(WALLET_ENDPOINT_BASE.size())); return true; } return false;