From 765ec3fcf9634ea3bcf0574971877d7ba75c5324 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 4 Apr 2022 13:19:49 -0400 Subject: [PATCH 1/8] Make DecodeBase{32,64} always return vector, not string Base32/base64 are mechanisms for encoding binary data. That they'd decode to a string is just bizarre. The fact that they'd do that based on the type of input arguments even more so. --- src/httprpc.cpp | 3 ++- src/i2p.cpp | 2 +- src/netaddress.cpp | 2 +- src/psbt.cpp | 8 ++++---- src/psbt.h | 2 +- src/qt/walletframe.cpp | 6 +++--- src/test/base32_tests.cpp | 4 ++-- src/test/base64_tests.cpp | 4 ++-- src/test/fuzz/base_encode_decode.cpp | 8 ++++---- src/test/fuzz/psbt.cpp | 6 ++++-- src/util/message.cpp | 2 +- src/util/strencodings.cpp | 10 ++++------ src/util/strencodings.h | 4 ++-- 13 files changed, 31 insertions(+), 30 deletions(-) diff --git a/src/httprpc.cpp b/src/httprpc.cpp index f28105c9b11f3..a078a5cdf98ab 100644 --- a/src/httprpc.cpp +++ b/src/httprpc.cpp @@ -173,8 +173,9 @@ static bool RPCAuthorized(const std::string& strAuth, std::string& strAuthUserna return false; std::string strUserPass64 = TrimString(strAuth.substr(6)); bool invalid; - std::string strUserPass = DecodeBase64(strUserPass64, &invalid); + std::vector userpass_data = DecodeBase64(strUserPass64, &invalid); if (invalid) return false; + std::string strUserPass(userpass_data.begin(), userpass_data.end()); if (strUserPass.find(':') != std::string::npos) strAuthUsernameOut = strUserPass.substr(0, strUserPass.find(':')); diff --git a/src/i2p.cpp b/src/i2p.cpp index 8ecb50645fe2e..0ee4b90ed4388 100644 --- a/src/i2p.cpp +++ b/src/i2p.cpp @@ -70,7 +70,7 @@ static Binary DecodeI2PBase64(const std::string& i2p_b64) { const std::string& std_b64 = SwapBase64(i2p_b64); bool invalid; - Binary decoded = DecodeBase64(std_b64.c_str(), &invalid); + Binary decoded = DecodeBase64(std_b64, &invalid); if (invalid) { throw std::runtime_error(strprintf("Cannot decode Base64: \"%s\"", i2p_b64)); } diff --git a/src/netaddress.cpp b/src/netaddress.cpp index 6466aec6fa2c0..5d6568618fcd8 100644 --- a/src/netaddress.cpp +++ b/src/netaddress.cpp @@ -236,7 +236,7 @@ bool CNetAddr::SetTor(const std::string& addr) } bool invalid; - const auto& input = DecodeBase32(addr.substr(0, addr.size() - suffix_len).c_str(), &invalid); + const auto& input = DecodeBase32(addr.substr(0, addr.size() - suffix_len), &invalid); if (invalid) { return false; diff --git a/src/psbt.cpp b/src/psbt.cpp index bc63976f7c6a8..602e0504e8b56 100644 --- a/src/psbt.cpp +++ b/src/psbt.cpp @@ -344,17 +344,17 @@ std::string PSBTRoleName(PSBTRole role) { bool DecodeBase64PSBT(PartiallySignedTransaction& psbt, const std::string& base64_tx, std::string& error) { bool invalid; - std::string tx_data = DecodeBase64(base64_tx, &invalid); + auto tx_data = DecodeBase64(base64_tx, &invalid); if (invalid) { error = "invalid base64"; return false; } - return DecodeRawPSBT(psbt, tx_data, error); + return DecodeRawPSBT(psbt, MakeByteSpan(tx_data), error); } -bool DecodeRawPSBT(PartiallySignedTransaction& psbt, const std::string& tx_data, std::string& error) +bool DecodeRawPSBT(PartiallySignedTransaction& psbt, Span tx_data, std::string& error) { - CDataStream ss_data(MakeByteSpan(tx_data), SER_NETWORK, PROTOCOL_VERSION); + CDataStream ss_data(tx_data, SER_NETWORK, PROTOCOL_VERSION); try { ss_data >> psbt; if (!ss_data.empty()) { diff --git a/src/psbt.h b/src/psbt.h index 868a86620181e..6619dc842f7b9 100644 --- a/src/psbt.h +++ b/src/psbt.h @@ -914,6 +914,6 @@ bool FinalizeAndExtractPSBT(PartiallySignedTransaction& psbtx, CMutableTransacti //! Decode a base64ed PSBT into a PartiallySignedTransaction [[nodiscard]] bool DecodeBase64PSBT(PartiallySignedTransaction& decoded_psbt, const std::string& base64_psbt, std::string& error); //! Decode a raw (binary blob) PSBT into a PartiallySignedTransaction -[[nodiscard]] bool DecodeRawPSBT(PartiallySignedTransaction& decoded_psbt, const std::string& raw_psbt, std::string& error); +[[nodiscard]] bool DecodeRawPSBT(PartiallySignedTransaction& decoded_psbt, Span raw_psbt, std::string& error); #endif // BITCOIN_PSBT_H diff --git a/src/qt/walletframe.cpp b/src/qt/walletframe.cpp index 9e177c673e305..297ae114b3ed0 100644 --- a/src/qt/walletframe.cpp +++ b/src/qt/walletframe.cpp @@ -249,7 +249,7 @@ void WalletFrame::gotoVerifyMessageTab(QString addr) void WalletFrame::gotoLoadPSBT(bool from_clipboard) { - std::string data; + std::vector data; if (from_clipboard) { std::string raw = QApplication::clipboard()->text().toStdString(); @@ -269,12 +269,12 @@ void WalletFrame::gotoLoadPSBT(bool from_clipboard) return; } std::ifstream in{filename.toLocal8Bit().data(), std::ios::binary}; - data = std::string(std::istreambuf_iterator{in}, {}); + data.assign(std::istream_iterator{in}, {}); } std::string error; PartiallySignedTransaction psbtx; - if (!DecodeRawPSBT(psbtx, data, error)) { + if (!DecodeRawPSBT(psbtx, MakeByteSpan(data), error)) { Q_EMIT message(tr("Error"), tr("Unable to decode PSBT") + "\n" + QString::fromStdString(error), CClientUIInterface::MSG_ERROR); return; } diff --git a/src/test/base32_tests.cpp b/src/test/base32_tests.cpp index 61d2a64e73331..e8e8d030f5a8e 100644 --- a/src/test/base32_tests.cpp +++ b/src/test/base32_tests.cpp @@ -23,9 +23,9 @@ BOOST_AUTO_TEST_CASE(base32_testvectors) strEnc = EncodeBase32(vstrIn[i], false); BOOST_CHECK_EQUAL(strEnc, vstrOutNoPadding[i]); bool invalid; - std::string strDec = DecodeBase32(vstrOut[i], &invalid); + auto dec = DecodeBase32(vstrOut[i], &invalid); BOOST_CHECK(!invalid); - BOOST_CHECK_EQUAL(strDec, vstrIn[i]); + BOOST_CHECK_MESSAGE(MakeByteSpan(dec) == MakeByteSpan(vstrIn[i]), vstrOut[i]); } // Decoding strings with embedded NUL characters should fail diff --git a/src/test/base64_tests.cpp b/src/test/base64_tests.cpp index 07db62a29cc06..2c7a5b382ea74 100644 --- a/src/test/base64_tests.cpp +++ b/src/test/base64_tests.cpp @@ -20,9 +20,9 @@ BOOST_AUTO_TEST_CASE(base64_testvectors) std::string strEnc = EncodeBase64(vstrIn[i]); BOOST_CHECK_EQUAL(strEnc, vstrOut[i]); bool invalid; - std::string strDec = DecodeBase64(strEnc, &invalid); + auto dec = DecodeBase64(strEnc, &invalid); BOOST_CHECK(!invalid); - BOOST_CHECK_EQUAL(strDec, vstrIn[i]); + BOOST_CHECK_MESSAGE(MakeByteSpan(dec) == MakeByteSpan(vstrIn[i]), vstrOut[i]); } { diff --git a/src/test/fuzz/base_encode_decode.cpp b/src/test/fuzz/base_encode_decode.cpp index 4384fe6c9ca38..7c8d263eda4c4 100644 --- a/src/test/fuzz/base_encode_decode.cpp +++ b/src/test/fuzz/base_encode_decode.cpp @@ -33,16 +33,16 @@ FUZZ_TARGET(base_encode_decode) } bool pf_invalid; - std::string decoded_string = DecodeBase32(random_encoded_string, &pf_invalid); + decoded = DecodeBase32(random_encoded_string, &pf_invalid); if (!pf_invalid) { - const std::string encoded_string = EncodeBase32(decoded_string); + const std::string encoded_string = EncodeBase32(decoded); assert(encoded_string == TrimString(encoded_string)); assert(ToLower(encoded_string) == ToLower(TrimString(random_encoded_string))); } - decoded_string = DecodeBase64(random_encoded_string, &pf_invalid); + decoded = DecodeBase64(random_encoded_string, &pf_invalid); if (!pf_invalid) { - const std::string encoded_string = EncodeBase64(decoded_string); + const std::string encoded_string = EncodeBase64(decoded); assert(encoded_string == TrimString(encoded_string)); assert(ToLower(encoded_string) == ToLower(TrimString(random_encoded_string))); } diff --git a/src/test/fuzz/psbt.cpp b/src/test/fuzz/psbt.cpp index b932385e1cec9..dffc0af1276e8 100644 --- a/src/test/fuzz/psbt.cpp +++ b/src/test/fuzz/psbt.cpp @@ -23,7 +23,8 @@ FUZZ_TARGET(psbt) FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; PartiallySignedTransaction psbt_mut; std::string error; - if (!DecodeRawPSBT(psbt_mut, fuzzed_data_provider.ConsumeRandomLengthString(), error)) { + auto str = fuzzed_data_provider.ConsumeRandomLengthString(); + if (!DecodeRawPSBT(psbt_mut, MakeByteSpan(str), error)) { return; } const PartiallySignedTransaction psbt = psbt_mut; @@ -70,7 +71,8 @@ FUZZ_TARGET(psbt) } PartiallySignedTransaction psbt_merge; - if (!DecodeRawPSBT(psbt_merge, fuzzed_data_provider.ConsumeRandomLengthString(), error)) { + str = fuzzed_data_provider.ConsumeRandomLengthString(); + if (!DecodeRawPSBT(psbt_merge, MakeByteSpan(str), error)) { psbt_merge = psbt; } psbt_mut = psbt; diff --git a/src/util/message.cpp b/src/util/message.cpp index ac2a1f8306eaf..44a12b6bc37da 100644 --- a/src/util/message.cpp +++ b/src/util/message.cpp @@ -36,7 +36,7 @@ MessageVerificationResult MessageVerify( } bool invalid = false; - std::vector signature_bytes = DecodeBase64(signature.c_str(), &invalid); + std::vector signature_bytes = DecodeBase64(signature, &invalid); if (invalid) { return MessageVerificationResult::ERR_MALFORMED_SIGNATURE; } diff --git a/src/util/strencodings.cpp b/src/util/strencodings.cpp index 4fea3925fd0e2..80529bcd9f07c 100644 --- a/src/util/strencodings.cpp +++ b/src/util/strencodings.cpp @@ -175,14 +175,13 @@ std::vector DecodeBase64(const char* p, bool* pf_invalid) return ret; } -std::string DecodeBase64(const std::string& str, bool* pf_invalid) +std::vector DecodeBase64(const std::string& str, bool* pf_invalid) { if (!ValidAsCString(str)) { *pf_invalid = true; return {}; } - std::vector vchRet = DecodeBase64(str.c_str(), pf_invalid); - return std::string((const char*)vchRet.data(), vchRet.size()); + return DecodeBase64(str.c_str(), pf_invalid); } std::string EncodeBase32(Span input, bool pad) @@ -251,14 +250,13 @@ std::vector DecodeBase32(const char* p, bool* pf_invalid) return ret; } -std::string DecodeBase32(const std::string& str, bool* pf_invalid) +std::vector DecodeBase32(const std::string& str, bool* pf_invalid) { if (!ValidAsCString(str)) { *pf_invalid = true; return {}; } - std::vector vchRet = DecodeBase32(str.c_str(), pf_invalid); - return std::string((const char*)vchRet.data(), vchRet.size()); + return DecodeBase32(str.c_str(), pf_invalid); } namespace { diff --git a/src/util/strencodings.h b/src/util/strencodings.h index f9a33fb64f380..8da80e82f7f51 100644 --- a/src/util/strencodings.h +++ b/src/util/strencodings.h @@ -65,12 +65,12 @@ bool IsHex(std::string_view str); */ bool IsHexNumber(std::string_view str); std::vector DecodeBase64(const char* p, bool* pf_invalid); -std::string DecodeBase64(const std::string& str, bool* pf_invalid); +std::vector DecodeBase64(const std::string& str, bool* pf_invalid); std::string EncodeBase64(Span input); inline std::string EncodeBase64(Span input) { return EncodeBase64(MakeUCharSpan(input)); } inline std::string EncodeBase64(const std::string& str) { return EncodeBase64(MakeUCharSpan(str)); } std::vector DecodeBase32(const char* p, bool* pf_invalid); -std::string DecodeBase32(const std::string& str, bool* pf_invalid); +std::vector DecodeBase32(const std::string& str, bool* pf_invalid); /** * Base32 encode. From 9519ace8d3a8fcab7ddb17dd58d128beca757a96 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 4 Apr 2022 13:52:06 -0400 Subject: [PATCH 2/8] Make DecodeBase{32,64} return optional instead of taking bool* --- src/httprpc.cpp | 8 ++++---- src/i2p.cpp | 7 +++---- src/netaddress.cpp | 20 +++++++++----------- src/psbt.cpp | 7 +++---- src/qt/walletframe.cpp | 6 +++--- src/test/base32_tests.cpp | 20 +++++++------------- src/test/base64_tests.cpp | 20 +++++++------------- src/test/fuzz/base_encode_decode.cpp | 13 ++++++------- src/util/message.cpp | 7 +++---- src/util/strencodings.cpp | 18 ++++++++---------- src/util/strencodings.h | 8 ++++---- 11 files changed, 57 insertions(+), 77 deletions(-) diff --git a/src/httprpc.cpp b/src/httprpc.cpp index a078a5cdf98ab..b460281bea596 100644 --- a/src/httprpc.cpp +++ b/src/httprpc.cpp @@ -172,10 +172,10 @@ static bool RPCAuthorized(const std::string& strAuth, std::string& strAuthUserna if (strAuth.substr(0, 6) != "Basic ") return false; std::string strUserPass64 = TrimString(strAuth.substr(6)); - bool invalid; - std::vector userpass_data = DecodeBase64(strUserPass64, &invalid); - if (invalid) return false; - std::string strUserPass(userpass_data.begin(), userpass_data.end()); + auto userpass_data = DecodeBase64(strUserPass64); + std::string strUserPass; + if (!userpass_data) return false; + strUserPass.assign(userpass_data->begin(), userpass_data->end()); if (strUserPass.find(':') != std::string::npos) strAuthUsernameOut = strUserPass.substr(0, strUserPass.find(':')); diff --git a/src/i2p.cpp b/src/i2p.cpp index 0ee4b90ed4388..23f93a2ad7275 100644 --- a/src/i2p.cpp +++ b/src/i2p.cpp @@ -69,12 +69,11 @@ static std::string SwapBase64(const std::string& from) static Binary DecodeI2PBase64(const std::string& i2p_b64) { const std::string& std_b64 = SwapBase64(i2p_b64); - bool invalid; - Binary decoded = DecodeBase64(std_b64, &invalid); - if (invalid) { + auto decoded = DecodeBase64(std_b64); + if (!decoded) { throw std::runtime_error(strprintf("Cannot decode Base64: \"%s\"", i2p_b64)); } - return decoded; + return std::move(*decoded); } /** diff --git a/src/netaddress.cpp b/src/netaddress.cpp index 5d6568618fcd8..50f0b17d9d9ea 100644 --- a/src/netaddress.cpp +++ b/src/netaddress.cpp @@ -235,17 +235,16 @@ bool CNetAddr::SetTor(const std::string& addr) return false; } - bool invalid; - const auto& input = DecodeBase32(addr.substr(0, addr.size() - suffix_len), &invalid); + auto input = DecodeBase32(addr.substr(0, addr.size() - suffix_len)); - if (invalid) { + if (!input) { return false; } - if (input.size() == torv3::TOTAL_LEN) { - Span input_pubkey{input.data(), ADDR_TORV3_SIZE}; - Span input_checksum{input.data() + ADDR_TORV3_SIZE, torv3::CHECKSUM_LEN}; - Span input_version{input.data() + ADDR_TORV3_SIZE + torv3::CHECKSUM_LEN, sizeof(torv3::VERSION)}; + if (input->size() == torv3::TOTAL_LEN) { + Span input_pubkey{input->data(), ADDR_TORV3_SIZE}; + Span input_checksum{input->data() + ADDR_TORV3_SIZE, torv3::CHECKSUM_LEN}; + Span input_version{input->data() + ADDR_TORV3_SIZE + torv3::CHECKSUM_LEN, sizeof(torv3::VERSION)}; if (input_version != torv3::VERSION) { return false; @@ -281,15 +280,14 @@ bool CNetAddr::SetI2P(const std::string& addr) // can decode it. const std::string b32_padded = addr.substr(0, b32_len) + "===="; - bool invalid; - const auto& address_bytes = DecodeBase32(b32_padded.c_str(), &invalid); + auto address_bytes = DecodeBase32(b32_padded); - if (invalid || address_bytes.size() != ADDR_I2P_SIZE) { + if (!address_bytes || address_bytes->size() != ADDR_I2P_SIZE) { return false; } m_net = NET_I2P; - m_addr.assign(address_bytes.begin(), address_bytes.end()); + m_addr.assign(address_bytes->begin(), address_bytes->end()); return true; } diff --git a/src/psbt.cpp b/src/psbt.cpp index 602e0504e8b56..a08cef7f2a69f 100644 --- a/src/psbt.cpp +++ b/src/psbt.cpp @@ -343,13 +343,12 @@ std::string PSBTRoleName(PSBTRole role) { bool DecodeBase64PSBT(PartiallySignedTransaction& psbt, const std::string& base64_tx, std::string& error) { - bool invalid; - auto tx_data = DecodeBase64(base64_tx, &invalid); - if (invalid) { + auto tx_data = DecodeBase64(base64_tx); + if (!tx_data) { error = "invalid base64"; return false; } - return DecodeRawPSBT(psbt, MakeByteSpan(tx_data), error); + return DecodeRawPSBT(psbt, MakeByteSpan(*tx_data), error); } bool DecodeRawPSBT(PartiallySignedTransaction& psbt, Span tx_data, std::string& error) diff --git a/src/qt/walletframe.cpp b/src/qt/walletframe.cpp index 297ae114b3ed0..f3781d1621c00 100644 --- a/src/qt/walletframe.cpp +++ b/src/qt/walletframe.cpp @@ -253,12 +253,12 @@ void WalletFrame::gotoLoadPSBT(bool from_clipboard) if (from_clipboard) { std::string raw = QApplication::clipboard()->text().toStdString(); - bool invalid; - data = DecodeBase64(raw, &invalid); - if (invalid) { + auto result = DecodeBase64(raw); + if (!result) { Q_EMIT message(tr("Error"), tr("Unable to decode PSBT from clipboard (invalid base64)"), CClientUIInterface::MSG_ERROR); return; } + data = std::move(*result); } else { QString filename = GUIUtil::getOpenFileName(this, tr("Load Transaction Data"), QString(), diff --git a/src/test/base32_tests.cpp b/src/test/base32_tests.cpp index e8e8d030f5a8e..27d05854bfcd7 100644 --- a/src/test/base32_tests.cpp +++ b/src/test/base32_tests.cpp @@ -22,22 +22,16 @@ BOOST_AUTO_TEST_CASE(base32_testvectors) BOOST_CHECK_EQUAL(strEnc, vstrOut[i]); strEnc = EncodeBase32(vstrIn[i], false); BOOST_CHECK_EQUAL(strEnc, vstrOutNoPadding[i]); - bool invalid; - auto dec = DecodeBase32(vstrOut[i], &invalid); - BOOST_CHECK(!invalid); - BOOST_CHECK_MESSAGE(MakeByteSpan(dec) == MakeByteSpan(vstrIn[i]), vstrOut[i]); + auto dec = DecodeBase32(vstrOut[i]); + BOOST_REQUIRE(dec); + BOOST_CHECK_MESSAGE(MakeByteSpan(*dec) == MakeByteSpan(vstrIn[i]), vstrOut[i]); } // Decoding strings with embedded NUL characters should fail - bool failure; - (void)DecodeBase32("invalid\0"s, &failure); // correct size, invalid due to \0 - BOOST_CHECK(failure); - (void)DecodeBase32("AWSX3VPP"s, &failure); // valid - BOOST_CHECK(!failure); - (void)DecodeBase32("AWSX3VPP\0invalid"s, &failure); // correct size, invalid due to \0 - BOOST_CHECK(failure); - (void)DecodeBase32("AWSX3VPPinvalid"s, &failure); // invalid size - BOOST_CHECK(failure); + BOOST_CHECK(!DecodeBase32("invalid\0"s)); // correct size, invalid due to \0 + BOOST_CHECK(DecodeBase32("AWSX3VPP"s)); // valid + BOOST_CHECK(!DecodeBase32("AWSX3VPP\0invalid"s)); // correct size, invalid due to \0 + BOOST_CHECK(!DecodeBase32("AWSX3VPPinvalid"s)); // invalid size } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/base64_tests.cpp b/src/test/base64_tests.cpp index 2c7a5b382ea74..3367acee9ece1 100644 --- a/src/test/base64_tests.cpp +++ b/src/test/base64_tests.cpp @@ -19,10 +19,9 @@ BOOST_AUTO_TEST_CASE(base64_testvectors) { std::string strEnc = EncodeBase64(vstrIn[i]); BOOST_CHECK_EQUAL(strEnc, vstrOut[i]); - bool invalid; - auto dec = DecodeBase64(strEnc, &invalid); - BOOST_CHECK(!invalid); - BOOST_CHECK_MESSAGE(MakeByteSpan(dec) == MakeByteSpan(vstrIn[i]), vstrOut[i]); + auto dec = DecodeBase64(strEnc); + BOOST_REQUIRE(dec); + BOOST_CHECK_MESSAGE(MakeByteSpan(*dec) == MakeByteSpan(vstrIn[i]), vstrOut[i]); } { @@ -36,15 +35,10 @@ BOOST_AUTO_TEST_CASE(base64_testvectors) } // Decoding strings with embedded NUL characters should fail - bool failure; - (void)DecodeBase64("invalid\0"s, &failure); - BOOST_CHECK(failure); - (void)DecodeBase64("nQB/pZw="s, &failure); - BOOST_CHECK(!failure); - (void)DecodeBase64("nQB/pZw=\0invalid"s, &failure); - BOOST_CHECK(failure); - (void)DecodeBase64("nQB/pZw=invalid\0"s, &failure); - BOOST_CHECK(failure); + BOOST_CHECK(!DecodeBase64("invalid\0"s)); + BOOST_CHECK(DecodeBase64("nQB/pZw="s)); + BOOST_CHECK(!DecodeBase64("nQB/pZw=\0invalid"s)); + BOOST_CHECK(!DecodeBase64("nQB/pZw=invalid\0"s)); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/fuzz/base_encode_decode.cpp b/src/test/fuzz/base_encode_decode.cpp index 7c8d263eda4c4..5d7715203ead3 100644 --- a/src/test/fuzz/base_encode_decode.cpp +++ b/src/test/fuzz/base_encode_decode.cpp @@ -32,17 +32,16 @@ FUZZ_TARGET(base_encode_decode) assert(ToLower(encoded_string) == ToLower(TrimString(random_encoded_string))); } - bool pf_invalid; - decoded = DecodeBase32(random_encoded_string, &pf_invalid); - if (!pf_invalid) { - const std::string encoded_string = EncodeBase32(decoded); + auto result = DecodeBase32(random_encoded_string); + if (result) { + const std::string encoded_string = EncodeBase32(*result); assert(encoded_string == TrimString(encoded_string)); assert(ToLower(encoded_string) == ToLower(TrimString(random_encoded_string))); } - decoded = DecodeBase64(random_encoded_string, &pf_invalid); - if (!pf_invalid) { - const std::string encoded_string = EncodeBase64(decoded); + result = DecodeBase64(random_encoded_string); + if (result) { + const std::string encoded_string = EncodeBase64(*result); assert(encoded_string == TrimString(encoded_string)); assert(ToLower(encoded_string) == ToLower(TrimString(random_encoded_string))); } diff --git a/src/util/message.cpp b/src/util/message.cpp index 44a12b6bc37da..5c9607cfe7225 100644 --- a/src/util/message.cpp +++ b/src/util/message.cpp @@ -35,14 +35,13 @@ MessageVerificationResult MessageVerify( return MessageVerificationResult::ERR_ADDRESS_NO_KEY; } - bool invalid = false; - std::vector signature_bytes = DecodeBase64(signature, &invalid); - if (invalid) { + auto signature_bytes = DecodeBase64(signature); + if (!signature_bytes) { return MessageVerificationResult::ERR_MALFORMED_SIGNATURE; } CPubKey pubkey; - if (!pubkey.RecoverCompact(MessageHash(message), signature_bytes)) { + if (!pubkey.RecoverCompact(MessageHash(message), *signature_bytes)) { return MessageVerificationResult::ERR_PUBKEY_NOT_RECOVERED; } diff --git a/src/util/strencodings.cpp b/src/util/strencodings.cpp index 80529bcd9f07c..ea98ab7dde0e2 100644 --- a/src/util/strencodings.cpp +++ b/src/util/strencodings.cpp @@ -129,7 +129,7 @@ std::string EncodeBase64(Span input) return str; } -std::vector DecodeBase64(const char* p, bool* pf_invalid) +std::optional> DecodeBase64(const char* p) { static const int8_t decode64_table[256]{ -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, @@ -170,18 +170,17 @@ std::vector DecodeBase64(const char* p, bool* pf_invalid) ++p; } valid = valid && (p - e) % 4 == 0 && p - q < 4; - *pf_invalid = !valid; + if (!valid) return {}; return ret; } -std::vector DecodeBase64(const std::string& str, bool* pf_invalid) +std::optional> DecodeBase64(const std::string& str) { if (!ValidAsCString(str)) { - *pf_invalid = true; return {}; } - return DecodeBase64(str.c_str(), pf_invalid); + return DecodeBase64(str.c_str()); } std::string EncodeBase32(Span input, bool pad) @@ -204,7 +203,7 @@ std::string EncodeBase32(const std::string& str, bool pad) return EncodeBase32(MakeUCharSpan(str), pad); } -std::vector DecodeBase32(const char* p, bool* pf_invalid) +std::optional> DecodeBase32(const char* p) { static const int8_t decode32_table[256]{ -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, @@ -245,18 +244,17 @@ std::vector DecodeBase32(const char* p, bool* pf_invalid) ++p; } valid = valid && (p - e) % 8 == 0 && p - q < 8; - *pf_invalid = !valid; + if (!valid) return {}; return ret; } -std::vector DecodeBase32(const std::string& str, bool* pf_invalid) +std::optional> DecodeBase32(const std::string& str) { if (!ValidAsCString(str)) { - *pf_invalid = true; return {}; } - return DecodeBase32(str.c_str(), pf_invalid); + return DecodeBase32(str.c_str()); } namespace { diff --git a/src/util/strencodings.h b/src/util/strencodings.h index 8da80e82f7f51..7344fe39ef415 100644 --- a/src/util/strencodings.h +++ b/src/util/strencodings.h @@ -64,13 +64,13 @@ bool IsHex(std::string_view str); * Return true if the string is a hex number, optionally prefixed with "0x" */ bool IsHexNumber(std::string_view str); -std::vector DecodeBase64(const char* p, bool* pf_invalid); -std::vector DecodeBase64(const std::string& str, bool* pf_invalid); +std::optional> DecodeBase64(const char* p); +std::optional> DecodeBase64(const std::string& str); std::string EncodeBase64(Span input); inline std::string EncodeBase64(Span input) { return EncodeBase64(MakeUCharSpan(input)); } inline std::string EncodeBase64(const std::string& str) { return EncodeBase64(MakeUCharSpan(str)); } -std::vector DecodeBase32(const char* p, bool* pf_invalid); -std::vector DecodeBase32(const std::string& str, bool* pf_invalid); +std::optional> DecodeBase32(const char* p); +std::optional> DecodeBase32(const std::string& str); /** * Base32 encode. From 7e5c0b5adbe1df5c8d1e058959794de135b1d65a Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 4 Apr 2022 11:58:54 -0400 Subject: [PATCH 3/8] Generalize ConvertBits to permit transforming the input --- src/util/strencodings.h | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/util/strencodings.h b/src/util/strencodings.h index 7344fe39ef415..0ed061c1be97e 100644 --- a/src/util/strencodings.h +++ b/src/util/strencodings.h @@ -237,15 +237,26 @@ bool TimingResistantEqual(const T& a, const T& b) */ [[nodiscard]] bool ParseFixedPoint(const std::string &val, int decimals, int64_t *amount_out); +namespace { +/** Helper class for the default infn argument to ConvertBits (just returns the input). */ +struct IntIdentity +{ + [[maybe_unused]] int operator()(int x) const { return x; } +}; + +} // namespace + /** Convert from one power-of-2 number base to another. */ -template -bool ConvertBits(const O& outfn, I it, I end) { +template +bool ConvertBits(O outfn, It it, It end, I infn = {}) { size_t acc = 0; size_t bits = 0; constexpr size_t maxv = (1 << tobits) - 1; constexpr size_t max_acc = (1 << (frombits + tobits - 1)) - 1; while (it != end) { - acc = ((acc << frombits) | *it) & max_acc; + int v = infn(*it); + if (v < 0) return false; + acc = ((acc << frombits) | v) & max_acc; bits += frombits; while (bits >= tobits) { bits -= tobits; From b6122ea62996e393e9189df7bd32318ee5edbc53 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 4 Apr 2022 14:10:10 -0400 Subject: [PATCH 4/8] Make DecodeBase{32,64} take string_view arguments --- src/netaddress.cpp | 2 +- src/util/strencodings.cpp | 85 ++++++++++++--------------------------- src/util/strencodings.h | 6 +-- 3 files changed, 28 insertions(+), 65 deletions(-) diff --git a/src/netaddress.cpp b/src/netaddress.cpp index 50f0b17d9d9ea..843f30c7c2724 100644 --- a/src/netaddress.cpp +++ b/src/netaddress.cpp @@ -235,7 +235,7 @@ bool CNetAddr::SetTor(const std::string& addr) return false; } - auto input = DecodeBase32(addr.substr(0, addr.size() - suffix_len)); + auto input = DecodeBase32(std::string_view{addr}.substr(0, addr.size() - suffix_len)); if (!input) { return false; diff --git a/src/util/strencodings.cpp b/src/util/strencodings.cpp index ea98ab7dde0e2..efd66093f156d 100644 --- a/src/util/strencodings.cpp +++ b/src/util/strencodings.cpp @@ -129,7 +129,7 @@ std::string EncodeBase64(Span input) return str; } -std::optional> DecodeBase64(const char* p) +std::optional> DecodeBase64(std::string_view str) { static const int8_t decode64_table[256]{ -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, @@ -147,42 +147,23 @@ std::optional> DecodeBase64(const char* p) -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1 }; - const char* e = p; - std::vector val; - val.reserve(strlen(p)); - while (*p != 0) { - int x = decode64_table[(unsigned char)*p]; - if (x == -1) break; - val.push_back(uint8_t(x)); - ++p; - } + if (str.size() % 4 != 0) return {}; + /* One or two = characters at the end are permitted. */ + if (str.size() >= 1 && str.back() == '=') str.remove_suffix(1); + if (str.size() >= 1 && str.back() == '=') str.remove_suffix(1); std::vector ret; - ret.reserve((val.size() * 3) / 4); - bool valid = ConvertBits<6, 8, false>([&](unsigned char c) { ret.push_back(c); }, val.begin(), val.end()); - - const char* q = p; - while (valid && *p != 0) { - if (*p != '=') { - valid = false; - break; - } - ++p; - } - valid = valid && (p - e) % 4 == 0 && p - q < 4; + ret.reserve((str.size() * 3) / 4); + bool valid = ConvertBits<6, 8, false>( + [&](unsigned char c) { ret.push_back(c); }, + str.begin(), str.end(), + [](char c) { return decode64_table[uint8_t(c)]; } + ); if (!valid) return {}; return ret; } -std::optional> DecodeBase64(const std::string& str) -{ - if (!ValidAsCString(str)) { - return {}; - } - return DecodeBase64(str.c_str()); -} - std::string EncodeBase32(Span input, bool pad) { static const char *pbase32 = "abcdefghijklmnopqrstuvwxyz234567"; @@ -203,7 +184,7 @@ std::string EncodeBase32(const std::string& str, bool pad) return EncodeBase32(MakeUCharSpan(str), pad); } -std::optional> DecodeBase32(const char* p) +std::optional> DecodeBase32(std::string_view str) { static const int8_t decode32_table[256]{ -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, @@ -221,42 +202,26 @@ std::optional> DecodeBase32(const char* p) -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1 }; - const char* e = p; - std::vector val; - val.reserve(strlen(p)); - while (*p != 0) { - int x = decode32_table[(unsigned char)*p]; - if (x == -1) break; - val.push_back(uint8_t(x)); - ++p; - } + if (str.size() % 8 != 0) return {}; + /* 1, 3, 4, or 6 padding '=' suffix characters are permitted. */ + if (str.size() >= 1 && str.back() == '=') str.remove_suffix(1); + if (str.size() >= 2 && str.substr(str.size() - 2) == "==") str.remove_suffix(2); + if (str.size() >= 1 && str.back() == '=') str.remove_suffix(1); + if (str.size() >= 2 && str.substr(str.size() - 2) == "==") str.remove_suffix(2); std::vector ret; - ret.reserve((val.size() * 5) / 8); - bool valid = ConvertBits<5, 8, false>([&](unsigned char c) { ret.push_back(c); }, val.begin(), val.end()); - - const char* q = p; - while (valid && *p != 0) { - if (*p != '=') { - valid = false; - break; - } - ++p; - } - valid = valid && (p - e) % 8 == 0 && p - q < 8; + ret.reserve((str.size() * 5) / 8); + bool valid = ConvertBits<5, 8, false>( + [&](unsigned char c) { ret.push_back(c); }, + str.begin(), str.end(), + [](char c) { return decode32_table[uint8_t(c)]; } + ); + if (!valid) return {}; return ret; } -std::optional> DecodeBase32(const std::string& str) -{ - if (!ValidAsCString(str)) { - return {}; - } - return DecodeBase32(str.c_str()); -} - namespace { template bool ParseIntegral(const std::string& str, T* out) diff --git a/src/util/strencodings.h b/src/util/strencodings.h index 0ed061c1be97e..ab7ec8391cfeb 100644 --- a/src/util/strencodings.h +++ b/src/util/strencodings.h @@ -64,13 +64,11 @@ bool IsHex(std::string_view str); * Return true if the string is a hex number, optionally prefixed with "0x" */ bool IsHexNumber(std::string_view str); -std::optional> DecodeBase64(const char* p); -std::optional> DecodeBase64(const std::string& str); +std::optional> DecodeBase64(std::string_view str); std::string EncodeBase64(Span input); inline std::string EncodeBase64(Span input) { return EncodeBase64(MakeUCharSpan(input)); } inline std::string EncodeBase64(const std::string& str) { return EncodeBase64(MakeUCharSpan(str)); } -std::optional> DecodeBase32(const char* p); -std::optional> DecodeBase32(const std::string& str); +std::optional> DecodeBase32(std::string_view str); /** * Base32 encode. From 5271cfacee493b6d5eb77e671c20b532a221a3b5 Mon Sep 17 00:00:00 2001 From: pasta Date: Sat, 11 Jan 2025 13:16:46 -0600 Subject: [PATCH 5/8] fix: DecodeBase64 changes in dash code --- src/httpserver.cpp | 11 +++++------ src/rpc/evo.cpp | 12 ++++++------ src/rpc/governance.cpp | 14 ++++++-------- src/stacktraces.cpp | 7 +++---- 4 files changed, 20 insertions(+), 24 deletions(-) diff --git a/src/httpserver.cpp b/src/httpserver.cpp index b4b263eb50488..cfa4b17f967c1 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -272,12 +272,11 @@ static void http_request_cb(struct evhttp_request* req, void* arg) return false; std::string strUserPass64 = TrimString(strAuth.substr(6)); - bool invalid; - std::string strUserPass = DecodeBase64(strUserPass64, &invalid); - if (invalid) return false; - - if (strUserPass.find(':') == std::string::npos) return false; - const std::string username{strUserPass.substr(0, strUserPass.find(':'))}; + auto opt_strUserPass = DecodeBase64(strUserPass64); + if (!opt_strUserPass.has_value()) return false; + auto it = std::find(opt_strUserPass->begin(), opt_strUserPass->end(), ':'); + if (it == opt_strUserPass->end()) return false; + const std::string username = std::string(opt_strUserPass->begin(), it); return find(g_external_usernames.begin(), g_external_usernames.end(), username) != g_external_usernames.end(); }(); diff --git a/src/rpc/evo.cpp b/src/rpc/evo.cpp index cd18a04976c55..b20ca26ee1dc1 100644 --- a/src/rpc/evo.cpp +++ b/src/rpc/evo.cpp @@ -815,9 +815,9 @@ static UniValue protx_register_common_wrapper(const JSONRPCRequest& request, } else if (err != SigningResult::OK){ throw JSONRPCError(RPC_WALLET_ERROR, SigningResultString(err)); } - bool invalid = false; - ptx.vchSig = DecodeBase64(signed_payload.c_str(), &invalid); - if (invalid) throw JSONRPCError(RPC_INTERNAL_ERROR, "failed to decode base64 ready signature for protx"); + auto opt_vchSig = DecodeBase64(signed_payload); + if (!opt_vchSig.has_value()) throw JSONRPCError(RPC_INTERNAL_ERROR, "failed to decode base64 ready signature for protx"); + ptx.vchSig = opt_vchSig.value(); } // cs_wallet SetTxPayload(tx, ptx); return SignAndSendSpecialTx(request, chain_helper, chainman, tx, fSubmit); @@ -877,11 +877,11 @@ static RPCHelpMan protx_register_submit() throw JSONRPCError(RPC_INVALID_PARAMETER, "payload signature not empty"); } - bool decode_fail{false}; - ptx.vchSig = DecodeBase64(request.params[1].get_str().c_str(), &decode_fail); - if (decode_fail) { + auto opt_vchSig= DecodeBase64(request.params[1].get_str()); + if (!opt_vchSig.has_value()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "malformed base64 encoding"); } + ptx.vchSig = opt_vchSig.value(); SetTxPayload(tx, ptx); return SignAndSendSpecialTx(request, chain_helper, chainman, tx); diff --git a/src/rpc/governance.cpp b/src/rpc/governance.cpp index 2671149417f52..9509786655344 100644 --- a/src/rpc/governance.cpp +++ b/src/rpc/governance.cpp @@ -421,11 +421,10 @@ static bool SignVote(const CWallet& wallet, const CKeyID& keyID, CGovernanceVote LogPrintf("SignVote failed due to: %s\n", SigningResultString(err)); return false; } - bool failed = true; - const auto decoded = DecodeBase64(signature, &failed); - CHECK_NONFATAL(!failed); // DecodeBase64 should not fail + const auto opt_decoded = DecodeBase64(signature); + CHECK_NONFATAL(opt_decoded.has_value()); // DecodeBase64 should not fail - vote.SetSignature(std::vector(decoded.data(), decoded.data() + decoded.size())); + vote.SetSignature(std::vector(opt_decoded->data(), opt_decoded->data() + opt_decoded->size())); return true; } @@ -959,10 +958,9 @@ static RPCHelpMan voteraw() int64_t nTime = request.params[5].get_int64(); std::string strSig = request.params[6].get_str(); - bool fInvalid = false; - std::vector vchSig = DecodeBase64(strSig.c_str(), &fInvalid); + auto opt_vchSig = DecodeBase64(strSig); - if (fInvalid) { + if (!opt_vchSig.has_value()) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Malformed base64 encoding"); } @@ -975,7 +973,7 @@ static RPCHelpMan voteraw() CGovernanceVote vote(outpoint, hashGovObj, eVoteSignal, eVoteOutcome); vote.SetTime(nTime); - vote.SetSignature(vchSig); + vote.SetSignature(opt_vchSig.value()); bool onlyVotingKeyAllowed = govObjType == GovernanceObject::PROPOSAL && vote.GetSignal() == VOTE_SIGNAL_FUNDING; diff --git a/src/stacktraces.cpp b/src/stacktraces.cpp index 0bb9c5e4b18d7..212c2afa20a02 100644 --- a/src/stacktraces.cpp +++ b/src/stacktraces.cpp @@ -426,13 +426,12 @@ std::string GetCrashInfoStrFromSerializedStr(const std::string& ciStr) { static uint64_t basePtr = GetBaseAddress(); - bool dataInvalid = false; - auto buf = DecodeBase32(ciStr.c_str(), &dataInvalid); - if (buf.empty() || dataInvalid) { + auto opt_buf = DecodeBase32(ciStr.c_str()); + if (!opt_buf.has_value() || opt_buf->empty()) { return "Error while deserializing crash info"; } - CDataStream ds(buf, SER_DISK, 0); + CDataStream ds(*opt_buf, SER_DISK, 0); crash_info_header hdr; try { From 0ce624fd2e975975da37525c56b4f0e7f63d48ef Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 4 Apr 2022 15:05:47 -0400 Subject: [PATCH 6/8] Use std::string_view throughout util strencodings/string --- src/bitcoin-tx.cpp | 2 +- src/httprpc.cpp | 2 +- src/test/fuzz/base_encode_decode.cpp | 4 ++-- src/test/fuzz/http_request.cpp | 2 +- src/test/util_tests.cpp | 20 ++++++++-------- src/util/strencodings.cpp | 28 +++++++++++------------ src/util/strencodings.h | 34 ++++++++++++++-------------- src/util/string.h | 29 +++++++++++++++++------- src/util/system.cpp | 4 ++-- 9 files changed, 69 insertions(+), 56 deletions(-) diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp index 7875efb28c1a4..b2145a235fc85 100644 --- a/src/bitcoin-tx.cpp +++ b/src/bitcoin-tx.cpp @@ -223,7 +223,7 @@ static void MutateTxLocktime(CMutableTransaction& tx, const std::string& cmdVal) template static T TrimAndParse(const std::string& int_str, const std::string& err) { - const auto parsed{ToIntegral(TrimString(int_str))}; + const auto parsed{ToIntegral(TrimStringView(int_str))}; if (!parsed.has_value()) { throw std::runtime_error(err + " '" + int_str + "'"); } diff --git a/src/httprpc.cpp b/src/httprpc.cpp index b460281bea596..b75232c20a650 100644 --- a/src/httprpc.cpp +++ b/src/httprpc.cpp @@ -171,7 +171,7 @@ static bool RPCAuthorized(const std::string& strAuth, std::string& strAuthUserna return false; if (strAuth.substr(0, 6) != "Basic ") return false; - std::string strUserPass64 = TrimString(strAuth.substr(6)); + std::string_view strUserPass64 = TrimStringView(std::string_view{strAuth}.substr(6)); auto userpass_data = DecodeBase64(strUserPass64); std::string strUserPass; if (!userpass_data) return false; diff --git a/src/test/fuzz/base_encode_decode.cpp b/src/test/fuzz/base_encode_decode.cpp index 5d7715203ead3..0109964a2eee8 100644 --- a/src/test/fuzz/base_encode_decode.cpp +++ b/src/test/fuzz/base_encode_decode.cpp @@ -22,7 +22,7 @@ FUZZ_TARGET(base_encode_decode) std::vector decoded; if (DecodeBase58(random_encoded_string, decoded, buffer.size())) { const std::string encoded_string = EncodeBase58(decoded); - assert(encoded_string == TrimString(encoded_string)); + assert(encoded_string == TrimStringView(encoded_string)); assert(ToLower(encoded_string) == ToLower(TrimString(random_encoded_string))); } @@ -35,7 +35,7 @@ FUZZ_TARGET(base_encode_decode) auto result = DecodeBase32(random_encoded_string); if (result) { const std::string encoded_string = EncodeBase32(*result); - assert(encoded_string == TrimString(encoded_string)); + assert(encoded_string == TrimStringView(encoded_string)); assert(ToLower(encoded_string) == ToLower(TrimString(random_encoded_string))); } diff --git a/src/test/fuzz/http_request.cpp b/src/test/fuzz/http_request.cpp index a04f50f6922a1..1815d1ada8699 100644 --- a/src/test/fuzz/http_request.cpp +++ b/src/test/fuzz/http_request.cpp @@ -54,7 +54,7 @@ FUZZ_TARGET(http_request) // and is a consequence of our hacky but necessary use of the internal function evhttp_parse_firstline_ in // this fuzzing harness. The workaround is not aesthetically pleasing, but it successfully avoids the troublesome // code path. " http:// HTTP/1.1\n" was a crashing input prior to this workaround. - const std::string http_buffer_str = ToLower({http_buffer.begin(), http_buffer.end()}); + const std::string http_buffer_str = ToLower(std::string{http_buffer.begin(), http_buffer.end()}); if (http_buffer_str.find(" http://") != std::string::npos || http_buffer_str.find(" https://") != std::string::npos || evhttp_parse_firstline_(evreq, evbuf) != 1 || evhttp_parse_headers_(evreq, evbuf) != 1) { evbuffer_free(evbuf); diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index fbdb851ae19d8..d7604bd069fac 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -278,17 +278,17 @@ BOOST_AUTO_TEST_CASE(util_ReplaceAll) BOOST_AUTO_TEST_CASE(util_TrimString) { BOOST_CHECK_EQUAL(TrimString(" foo bar "), "foo bar"); - BOOST_CHECK_EQUAL(TrimString("\t \n \n \f\n\r\t\v\tfoo \n \f\n\r\t\v\tbar\t \n \f\n\r\t\v\t\n "), "foo \n \f\n\r\t\v\tbar"); + BOOST_CHECK_EQUAL(TrimStringView("\t \n \n \f\n\r\t\v\tfoo \n \f\n\r\t\v\tbar\t \n \f\n\r\t\v\t\n "), "foo \n \f\n\r\t\v\tbar"); BOOST_CHECK_EQUAL(TrimString("\t \n foo \n\tbar\t \n "), "foo \n\tbar"); - BOOST_CHECK_EQUAL(TrimString("\t \n foo \n\tbar\t \n ", "fobar"), "\t \n foo \n\tbar\t \n "); + BOOST_CHECK_EQUAL(TrimStringView("\t \n foo \n\tbar\t \n ", "fobar"), "\t \n foo \n\tbar\t \n "); BOOST_CHECK_EQUAL(TrimString("foo bar"), "foo bar"); - BOOST_CHECK_EQUAL(TrimString("foo bar", "fobar"), " "); + BOOST_CHECK_EQUAL(TrimStringView("foo bar", "fobar"), " "); BOOST_CHECK_EQUAL(TrimString(std::string("\0 foo \0 ", 8)), std::string("\0 foo \0", 7)); - BOOST_CHECK_EQUAL(TrimString(std::string(" foo ", 5)), std::string("foo", 3)); + BOOST_CHECK_EQUAL(TrimStringView(std::string(" foo ", 5)), std::string("foo", 3)); BOOST_CHECK_EQUAL(TrimString(std::string("\t\t\0\0\n\n", 6)), std::string("\0\0", 2)); - BOOST_CHECK_EQUAL(TrimString(std::string("\x05\x04\x03\x02\x01\x00", 6)), std::string("\x05\x04\x03\x02\x01\x00", 6)); + BOOST_CHECK_EQUAL(TrimStringView(std::string("\x05\x04\x03\x02\x01\x00", 6)), std::string("\x05\x04\x03\x02\x01\x00", 6)); BOOST_CHECK_EQUAL(TrimString(std::string("\x05\x04\x03\x02\x01\x00", 6), std::string("\x05\x04\x03\x02\x01", 5)), std::string("\0", 1)); - BOOST_CHECK_EQUAL(TrimString(std::string("\x05\x04\x03\x02\x01\x00", 6), std::string("\x05\x04\x03\x02\x01\x00", 6)), ""); + BOOST_CHECK_EQUAL(TrimStringView(std::string("\x05\x04\x03\x02\x01\x00", 6), std::string("\x05\x04\x03\x02\x01\x00", 6)), ""); } BOOST_AUTO_TEST_CASE(util_FormatParseISO8601DateTime) @@ -2694,13 +2694,13 @@ BOOST_AUTO_TEST_CASE(message_hash) BOOST_AUTO_TEST_CASE(remove_prefix) { BOOST_CHECK_EQUAL(RemovePrefix("./util/system.h", "./"), "util/system.h"); - BOOST_CHECK_EQUAL(RemovePrefix("foo", "foo"), ""); + BOOST_CHECK_EQUAL(RemovePrefixView("foo", "foo"), ""); BOOST_CHECK_EQUAL(RemovePrefix("foo", "fo"), "o"); - BOOST_CHECK_EQUAL(RemovePrefix("foo", "f"), "oo"); + BOOST_CHECK_EQUAL(RemovePrefixView("foo", "f"), "oo"); BOOST_CHECK_EQUAL(RemovePrefix("foo", ""), "foo"); - BOOST_CHECK_EQUAL(RemovePrefix("fo", "foo"), "fo"); + BOOST_CHECK_EQUAL(RemovePrefixView("fo", "foo"), "fo"); BOOST_CHECK_EQUAL(RemovePrefix("f", "foo"), "f"); - BOOST_CHECK_EQUAL(RemovePrefix("", "foo"), ""); + BOOST_CHECK_EQUAL(RemovePrefixView("", "foo"), ""); BOOST_CHECK_EQUAL(RemovePrefix("", ""), ""); } diff --git a/src/util/strencodings.cpp b/src/util/strencodings.cpp index efd66093f156d..84bb511f0dc92 100644 --- a/src/util/strencodings.cpp +++ b/src/util/strencodings.cpp @@ -97,7 +97,7 @@ std::vector ParseHex(std::string_view str) template std::vector ParseHex(std::string_view); template std::vector ParseHex(std::string_view); -void SplitHostPort(std::string in, uint16_t& portOut, std::string& hostOut) +void SplitHostPort(std::string_view in, uint16_t& portOut, std::string& hostOut) { size_t colon = in.find_last_of(':'); // if a : is found, and it either follows a [...], or no other : is in the string, treat it as port separator @@ -179,7 +179,7 @@ std::string EncodeBase32(Span input, bool pad) return str; } -std::string EncodeBase32(const std::string& str, bool pad) +std::string EncodeBase32(std::string_view str, bool pad) { return EncodeBase32(MakeUCharSpan(str), pad); } @@ -224,7 +224,7 @@ std::optional> DecodeBase32(std::string_view str) namespace { template -bool ParseIntegral(const std::string& str, T* out) +bool ParseIntegral(std::string_view str, T* out) { static_assert(std::is_integral::value); // Replicate the exact behavior of strtol/strtoll/strtoul/strtoull when @@ -243,37 +243,37 @@ bool ParseIntegral(const std::string& str, T* out) } }; // namespace -bool ParseInt32(const std::string& str, int32_t* out) +bool ParseInt32(std::string_view str, int32_t* out) { return ParseIntegral(str, out); } -bool ParseInt64(const std::string& str, int64_t* out) +bool ParseInt64(std::string_view str, int64_t* out) { return ParseIntegral(str, out); } -bool ParseUInt8(const std::string& str, uint8_t* out) +bool ParseUInt8(std::string_view str, uint8_t* out) { return ParseIntegral(str, out); } -bool ParseUInt16(const std::string& str, uint16_t* out) +bool ParseUInt16(std::string_view str, uint16_t* out) { return ParseIntegral(str, out); } -bool ParseUInt32(const std::string& str, uint32_t* out) +bool ParseUInt32(std::string_view str, uint32_t* out) { return ParseIntegral(str, out); } -bool ParseUInt64(const std::string& str, uint64_t* out) +bool ParseUInt64(std::string_view str, uint64_t* out) { return ParseIntegral(str, out); } -std::string FormatParagraph(const std::string& in, size_t width, size_t indent) +std::string FormatParagraph(std::string_view in, size_t width, size_t indent) { assert(width >= indent); std::stringstream out; @@ -342,7 +342,7 @@ static inline bool ProcessMantissaDigit(char ch, int64_t &mantissa, int &mantiss return true; } -bool ParseFixedPoint(const std::string &val, int decimals, int64_t *amount_out) +bool ParseFixedPoint(std::string_view val, int decimals, int64_t *amount_out) { int64_t mantissa = 0; int64_t exponent = 0; @@ -434,7 +434,7 @@ bool ParseFixedPoint(const std::string &val, int decimals, int64_t *amount_out) return true; } -std::string ToLower(const std::string& str) +std::string ToLower(std::string_view str) { std::string r; r.reserve(str.size()); @@ -442,7 +442,7 @@ std::string ToLower(const std::string& str) return r; } -std::string ToUpper(const std::string& str) +std::string ToUpper(std::string_view str) { std::string r; r.reserve(str.size()); @@ -491,7 +491,7 @@ std::string HexStr(const Span s) return rv; } -std::optional ParseByteUnits(const std::string& str, ByteUnit default_multiplier) +std::optional ParseByteUnits(std::string_view str, ByteUnit default_multiplier) { if (str.empty()) { return std::nullopt; diff --git a/src/util/strencodings.h b/src/util/strencodings.h index ab7ec8391cfeb..003aa539122ef 100644 --- a/src/util/strencodings.h +++ b/src/util/strencodings.h @@ -67,7 +67,7 @@ bool IsHexNumber(std::string_view str); std::optional> DecodeBase64(std::string_view str); std::string EncodeBase64(Span input); inline std::string EncodeBase64(Span input) { return EncodeBase64(MakeUCharSpan(input)); } -inline std::string EncodeBase64(const std::string& str) { return EncodeBase64(MakeUCharSpan(str)); } +inline std::string EncodeBase64(std::string_view str) { return EncodeBase64(MakeUCharSpan(str)); } std::optional> DecodeBase32(std::string_view str); /** @@ -82,9 +82,9 @@ std::string EncodeBase32(Span input, bool pad = true); * If `pad` is true, then the output will be padded with '=' so that its length * is a multiple of 8. */ -std::string EncodeBase32(const std::string& str, bool pad = true); +std::string EncodeBase32(std::string_view str, bool pad = true); -void SplitHostPort(std::string in, uint16_t &portOut, std::string &hostOut); +void SplitHostPort(std::string_view in, uint16_t &portOut, std::string &hostOut); // LocaleIndependentAtoi is provided for backwards compatibility reasons. // @@ -94,12 +94,12 @@ void SplitHostPort(std::string in, uint16_t &portOut, std::string &hostOut); // The goal of LocaleIndependentAtoi is to replicate the exact defined behaviour // of atoi and atoi64 as they behave under the "C" locale. template -T LocaleIndependentAtoi(const std::string& str) +T LocaleIndependentAtoi(std::string_view str) { static_assert(std::is_integral::value); T result; // Emulate atoi(...) handling of white space and leading +/-. - std::string s = TrimString(str); + std::string_view s = TrimStringView(str); if (!s.empty() && s[0] == '+') { if (s.length() >= 2 && s[1] == '-') { return 0; @@ -147,7 +147,7 @@ constexpr inline bool IsSpace(char c) noexcept { * parsed value is not in the range representable by the type T. */ template -std::optional ToIntegral(const std::string& str) +std::optional ToIntegral(std::string_view str) { static_assert(std::is_integral::value); T result; @@ -163,42 +163,42 @@ std::optional ToIntegral(const std::string& str) * @returns true if the entire string could be parsed as valid integer, * false if not the entire string could be parsed or when overflow or underflow occurred. */ -[[nodiscard]] bool ParseInt32(const std::string& str, int32_t *out); +[[nodiscard]] bool ParseInt32(std::string_view str, int32_t *out); /** * Convert string to signed 64-bit integer with strict parse error feedback. * @returns true if the entire string could be parsed as valid integer, * false if not the entire string could be parsed or when overflow or underflow occurred. */ -[[nodiscard]] bool ParseInt64(const std::string& str, int64_t *out); +[[nodiscard]] bool ParseInt64(std::string_view str, int64_t *out); /** * Convert decimal string to unsigned 8-bit integer with strict parse error feedback. * @returns true if the entire string could be parsed as valid integer, * false if not the entire string could be parsed or when overflow or underflow occurred. */ -[[nodiscard]] bool ParseUInt8(const std::string& str, uint8_t *out); +[[nodiscard]] bool ParseUInt8(std::string_view str, uint8_t *out); /** * Convert decimal string to unsigned 16-bit integer with strict parse error feedback. * @returns true if the entire string could be parsed as valid integer, * false if the entire string could not be parsed or if overflow or underflow occurred. */ -[[nodiscard]] bool ParseUInt16(const std::string& str, uint16_t* out); +[[nodiscard]] bool ParseUInt16(std::string_view str, uint16_t* out); /** * Convert decimal string to unsigned 32-bit integer with strict parse error feedback. * @returns true if the entire string could be parsed as valid integer, * false if not the entire string could be parsed or when overflow or underflow occurred. */ -[[nodiscard]] bool ParseUInt32(const std::string& str, uint32_t *out); +[[nodiscard]] bool ParseUInt32(std::string_view str, uint32_t *out); /** * Convert decimal string to unsigned 64-bit integer with strict parse error feedback. * @returns true if the entire string could be parsed as valid integer, * false if not the entire string could be parsed or when overflow or underflow occurred. */ -[[nodiscard]] bool ParseUInt64(const std::string& str, uint64_t *out); +[[nodiscard]] bool ParseUInt64(std::string_view str, uint64_t *out); /** * Convert a span of bytes to a lower-case hexadecimal string. @@ -211,7 +211,7 @@ inline std::string HexStr(const Span s) { return HexStr(MakeUCh * Format a paragraph of text to a fixed width, adding spaces for * indentation to any added line. */ -std::string FormatParagraph(const std::string& in, size_t width = 79, size_t indent = 0); +std::string FormatParagraph(std::string_view in, size_t width = 79, size_t indent = 0); /** * Timing-attack-resistant comparison. @@ -233,7 +233,7 @@ bool TimingResistantEqual(const T& a, const T& b) * @returns true on success, false on error. * @note The result must be in the range (-10^18,10^18), otherwise an overflow error will trigger. */ -[[nodiscard]] bool ParseFixedPoint(const std::string &val, int decimals, int64_t *amount_out); +[[nodiscard]] bool ParseFixedPoint(std::string_view, int decimals, int64_t *amount_out); namespace { /** Helper class for the default infn argument to ConvertBits (just returns the input). */ @@ -294,7 +294,7 @@ constexpr char ToLower(char c) * @param[in] str the string to convert to lowercase. * @returns lowercased equivalent of str */ -std::string ToLower(const std::string& str); +std::string ToLower(std::string_view str); /** * Converts the given character to its uppercase equivalent. @@ -320,7 +320,7 @@ constexpr char ToUpper(char c) * @param[in] str the string to convert to uppercase. * @returns UPPERCASED EQUIVALENT OF str */ -std::string ToUpper(const std::string& str); +std::string ToUpper(std::string_view str); /** * Capitalizes the first character of the given string. @@ -344,6 +344,6 @@ std::string Capitalize(std::string str); * @returns optional uint64_t bytes from str or nullopt * if ToIntegral is false, str is empty, trailing whitespace or overflow */ -std::optional ParseByteUnits(const std::string& str, ByteUnit default_multiplier); +std::optional ParseByteUnits(std::string_view str, ByteUnit default_multiplier); #endif // BITCOIN_UTIL_STRENCODINGS_H diff --git a/src/util/string.h b/src/util/string.h index beb161a2e6ef2..e21c30196431a 100644 --- a/src/util/string.h +++ b/src/util/string.h @@ -29,17 +29,22 @@ void ReplaceAll(std::string& in_out, const std::string& search, const std::strin return spanparsing::Split(str, separators); } -[[nodiscard]] inline std::string TrimString(const std::string& str, const std::string& pattern = " \f\n\r\t\v") +[[nodiscard]] inline std::string_view TrimStringView(std::string_view str, std::string_view pattern = " \f\n\r\t\v") { std::string::size_type front = str.find_first_not_of(pattern); if (front == std::string::npos) { - return std::string(); + return {}; } std::string::size_type end = str.find_last_not_of(pattern); return str.substr(front, end - front + 1); } -[[nodiscard]] inline std::string RemovePrefix(const std::string& str, const std::string& prefix) +[[nodiscard]] inline std::string TrimString(std::string_view str, std::string_view pattern = " \f\n\r\t\v") +{ + return std::string(TrimStringView(str, pattern)); +} + +[[nodiscard]] inline std::string_view RemovePrefixView(std::string_view str, std::string_view prefix) { if (str.substr(0, prefix.size()) == prefix) { return str.substr(prefix.size()); @@ -47,6 +52,11 @@ void ReplaceAll(std::string& in_out, const std::string& search, const std::strin return str; } +[[nodiscard]] inline std::string RemovePrefix(std::string_view str, std::string_view prefix) +{ + return std::string(RemovePrefixView(str, prefix)); +} + /** * Join a list of items * @@ -66,14 +76,14 @@ auto Join(const std::vector& list, const BaseType& separator, UnaryOp unary_o return ret; } -template -T Join(const std::vector& list, const T& separator) +template +T Join(const std::vector& list, const T2& separator) { return Join(list, separator, [](const T& i) { return i; }); } // Explicit overload needed for c_str arguments, which would otherwise cause a substitution failure in the template above. -inline std::string Join(const std::vector& list, const std::string& separator) +inline std::string Join(const std::vector& list, std::string_view separator) { return Join(list, separator); } @@ -89,9 +99,12 @@ inline std::string MakeUnorderedList(const std::vector& items) /** * Check if a string does not contain any embedded NUL (\0) characters */ -[[nodiscard]] inline bool ValidAsCString(const std::string& str) noexcept +[[nodiscard]] inline bool ValidAsCString(std::string_view str) noexcept { - return str.size() == strlen(str.c_str()); + for (auto c : str) { + if (c == 0) return false; + } + return true; } /** diff --git a/src/util/system.cpp b/src/util/system.cpp index 563a7bf4b0bf3..69085d55da947 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -904,8 +904,8 @@ static bool GetConfigOptions(std::istream& stream, const std::string& filepath, error = strprintf("parse error on line %i: %s, options in configuration file must be specified without leading -", linenr, str); return false; } else if ((pos = str.find('=')) != std::string::npos) { - std::string name = prefix + TrimString(str.substr(0, pos), pattern); - std::string value = TrimString(str.substr(pos + 1), pattern); + std::string name = prefix + TrimString(std::string_view{str}.substr(0, pos), pattern); + std::string_view value = TrimStringView(std::string_view{str}.substr(pos + 1), pattern); if (used_hash && name.find("rpcpassword") != std::string::npos) { error = strprintf("parse error on line %i, using # in rpcpassword can be ambiguous and should be avoided", linenr); return false; From f6bd5060e2e036bb21cd238544af71b84eac3e02 Mon Sep 17 00:00:00 2001 From: pasta Date: Sat, 11 Jan 2025 23:41:32 -0600 Subject: [PATCH 7/8] rename: ValidAsCString -> ContainsNoNUL --- src/base58.cpp | 4 ++-- src/netaddress.cpp | 2 +- src/netbase.cpp | 10 +++++----- src/test/fuzz/string.cpp | 4 ++-- src/util/moneystr.cpp | 2 +- src/util/string.h | 2 +- 6 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/base58.cpp b/src/base58.cpp index 6ad8343b074dd..a061f0d305eea 100644 --- a/src/base58.cpp +++ b/src/base58.cpp @@ -126,7 +126,7 @@ std::string EncodeBase58(Span input) bool DecodeBase58(const std::string& str, std::vector& vchRet, int max_ret_len) { - if (!ValidAsCString(str)) { + if (!ContainsNoNUL(str)) { return false; } return DecodeBase58(str.c_str(), vchRet, max_ret_len); @@ -160,7 +160,7 @@ std::string EncodeBase58Check(Span input) bool DecodeBase58Check(const std::string& str, std::vector& vchRet, int max_ret) { - if (!ValidAsCString(str)) { + if (!ContainsNoNUL(str)) { return false; } return DecodeBase58Check(str.c_str(), vchRet, max_ret); diff --git a/src/netaddress.cpp b/src/netaddress.cpp index 843f30c7c2724..73e632f5350f0 100644 --- a/src/netaddress.cpp +++ b/src/netaddress.cpp @@ -211,7 +211,7 @@ static void Checksum(Span addr_pubkey, uint8_t (&checksum)[CHECKS bool CNetAddr::SetSpecial(const std::string& addr) { - if (!ValidAsCString(addr)) { + if (!ContainsNoNUL(addr)) { return false; } diff --git a/src/netbase.cpp b/src/netbase.cpp index adaf3fa629639..5a1ebe2aa18c9 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -144,7 +144,7 @@ std::vector GetNetworkNames(bool append_unroutable) static std::vector LookupIntern(const std::string& name, unsigned int nMaxSolutions, bool fAllowLookup, DNSLookupFn dns_lookup_function) { - if (!ValidAsCString(name)) return {}; + if (!ContainsNoNUL(name)) return {}; { CNetAddr addr; // From our perspective, onion addresses are not hostnames but rather @@ -173,7 +173,7 @@ static std::vector LookupIntern(const std::string& name, unsigned int std::vector LookupHost(const std::string& name, unsigned int nMaxSolutions, bool fAllowLookup, DNSLookupFn dns_lookup_function) { - if (!ValidAsCString(name)) return {}; + if (!ContainsNoNUL(name)) return {}; std::string strHost = name; if (strHost.empty()) return {}; if (strHost.front() == '[' && strHost.back() == ']') { @@ -191,7 +191,7 @@ std::optional LookupHost(const std::string& name, bool fAllowLookup, D std::vector Lookup(const std::string& name, uint16_t portDefault, bool fAllowLookup, unsigned int nMaxSolutions, DNSLookupFn dns_lookup_function) { - if (name.empty() || !ValidAsCString(name)) { + if (name.empty() || !ContainsNoNUL(name)) { return {}; } uint16_t port{portDefault}; @@ -216,7 +216,7 @@ std::optional Lookup(const std::string& name, uint16_t portDefault, bo CService LookupNumeric(const std::string& name, uint16_t portDefault, DNSLookupFn dns_lookup_function) { - if (!ValidAsCString(name)) { + if (!ContainsNoNUL(name)) { return {}; } // "1.2:345" will fail to resolve the ip, but will still set the port. @@ -668,7 +668,7 @@ bool ConnectThroughProxy(const Proxy& proxy, const std::string& strDest, uint16_ bool LookupSubNet(const std::string& subnet_str, CSubNet& subnet_out) { - if (!ValidAsCString(subnet_str)) { + if (!ContainsNoNUL(subnet_str)) { return false; } diff --git a/src/test/fuzz/string.cpp b/src/test/fuzz/string.cpp index 4710f7b996074..d4b30767dab9b 100644 --- a/src/test/fuzz/string.cpp +++ b/src/test/fuzz/string.cpp @@ -40,7 +40,7 @@ bool LegacyParsePrechecks(const std::string& str) return false; if (str.size() >= 1 && (IsSpace(str[0]) || IsSpace(str[str.size() - 1]))) // No padding allowed return false; - if (!ValidAsCString(str)) // No embedded NUL characters allowed + if (!ContainsNoNUL(str)) // No embedded NUL characters allowed return false; return true; } @@ -185,7 +185,7 @@ FUZZ_TARGET(string) (void)TrimString(random_string_1); (void)TrimString(random_string_1, random_string_2); (void)urlDecode(random_string_1); - (void)ValidAsCString(random_string_1); + (void)ContainsNoNUL(random_string_1); (void)_(random_string_1.c_str()); try { throw scriptnum_error{random_string_1}; diff --git a/src/util/moneystr.cpp b/src/util/moneystr.cpp index 1aed7daacf6ac..02f7a2630ad03 100644 --- a/src/util/moneystr.cpp +++ b/src/util/moneystr.cpp @@ -40,7 +40,7 @@ std::string FormatMoney(const CAmount n) std::optional ParseMoney(const std::string& money_string) { - if (!ValidAsCString(money_string)) { + if (!ContainsNoNUL(money_string)) { return std::nullopt; } const std::string str = TrimString(money_string); diff --git a/src/util/string.h b/src/util/string.h index e21c30196431a..6cfa53708bb8b 100644 --- a/src/util/string.h +++ b/src/util/string.h @@ -99,7 +99,7 @@ inline std::string MakeUnorderedList(const std::vector& items) /** * Check if a string does not contain any embedded NUL (\0) characters */ -[[nodiscard]] inline bool ValidAsCString(std::string_view str) noexcept +[[nodiscard]] inline bool ContainsNoNUL(std::string_view str) noexcept { for (auto c : str) { if (c == 0) return false; From e71460712b19df9ad74ad2d8b746549a692fcc0b Mon Sep 17 00:00:00 2001 From: pasta Date: Sat, 11 Jan 2025 23:43:46 -0600 Subject: [PATCH 8/8] Merge bitcoin#25001: Modernize util/strencodings and util/string