From fa103db2bb736bce4440f0bde564e6671e36311d Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Thu, 18 Jul 2024 22:24:38 +0200 Subject: [PATCH] scripted-diff: Rename SetHex to SetHexDeprecated SetHex is fragile, because it accepts any non-hex input or any length of input, without error feedback. This can lead to issues when the input is truncated or otherwise corrupted. Document the problem by renaming the method. In the future, the fragile method should be removed from the public interface. -BEGIN VERIFY SCRIPT- sed -i 's/SetHex/SetHexDeprecated/g' $( git grep -l SetHex ./src ) -END VERIFY SCRIPT- --- src/core_read.cpp | 2 +- src/qt/transactiontablemodel.cpp | 2 +- src/qt/transactionview.cpp | 6 +++--- src/test/uint256_tests.cpp | 18 +++++++++--------- src/uint256.cpp | 6 +++--- src/uint256.h | 4 ++-- 6 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/core_read.cpp b/src/core_read.cpp index 0ba271a8d29..3e0485eb7bf 100644 --- a/src/core_read.cpp +++ b/src/core_read.cpp @@ -239,7 +239,7 @@ bool ParseHashStr(const std::string& strHex, uint256& result) if ((strHex.size() != 64) || !IsHex(strHex)) return false; - result.SetHex(strHex); + result.SetHexDeprecated(strHex); return true; } diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp index d4267fcf61f..9214e7723d3 100644 --- a/src/qt/transactiontablemodel.cpp +++ b/src/qt/transactiontablemodel.cpp @@ -277,7 +277,7 @@ void TransactionTableModel::updateAmountColumnTitle() void TransactionTableModel::updateTransaction(const QString &hash, int status, bool showTransaction) { uint256 updated; - updated.SetHex(hash.toStdString()); + updated.SetHexDeprecated(hash.toStdString()); priv->updateWallet(walletModel->wallet(), updated, status, showTransaction); } diff --git a/src/qt/transactionview.cpp b/src/qt/transactionview.cpp index 7e24dbd3ec9..2aaa65c6f73 100644 --- a/src/qt/transactionview.cpp +++ b/src/qt/transactionview.cpp @@ -396,7 +396,7 @@ void TransactionView::contextualMenu(const QPoint &point) // check if transaction can be abandoned, disable context menu action in case it doesn't uint256 hash; - hash.SetHex(selection.at(0).data(TransactionTableModel::TxHashRole).toString().toStdString()); + hash.SetHexDeprecated(selection.at(0).data(TransactionTableModel::TxHashRole).toString().toStdString()); abandonAction->setEnabled(model->wallet().transactionCanBeAbandoned(hash)); bumpFeeAction->setEnabled(model->wallet().transactionCanBeBumped(hash)); copyAddressAction->setEnabled(GUIUtil::hasEntryData(transactionView, 0, TransactionTableModel::AddressRole)); @@ -416,7 +416,7 @@ void TransactionView::abandonTx() // get the hash from the TxHashRole (QVariant / QString) uint256 hash; QString hashQStr = selection.at(0).data(TransactionTableModel::TxHashRole).toString(); - hash.SetHex(hashQStr.toStdString()); + hash.SetHexDeprecated(hashQStr.toStdString()); // Abandon the wallet transaction over the walletModel model->wallet().abandonTransaction(hash); @@ -431,7 +431,7 @@ void TransactionView::bumpFee([[maybe_unused]] bool checked) // get the hash from the TxHashRole (QVariant / QString) uint256 hash; QString hashQStr = selection.at(0).data(TransactionTableModel::TxHashRole).toString(); - hash.SetHex(hashQStr.toStdString()); + hash.SetHexDeprecated(hashQStr.toStdString()); // Bump tx fee over the walletModel uint256 newHash; diff --git a/src/test/uint256_tests.cpp b/src/test/uint256_tests.cpp index 669983c0e95..59290692715 100644 --- a/src/test/uint256_tests.cpp +++ b/src/test/uint256_tests.cpp @@ -62,7 +62,7 @@ static std::string ArrayToString(const unsigned char A[], unsigned int width) inline uint160 uint160S(std::string_view str) { uint160 rv; - rv.SetHex(str); + rv.SetHexDeprecated(str); return rv; } @@ -157,7 +157,7 @@ BOOST_AUTO_TEST_CASE( comparison ) // <= >= < > uint256S("1000000000000000000000000000000000000000000000000000000000000002")); } -BOOST_AUTO_TEST_CASE( methods ) // GetHex SetHex begin() end() size() GetLow64 GetSerializeSize, Serialize, Unserialize +BOOST_AUTO_TEST_CASE( methods ) // GetHex SetHexDeprecated begin() end() size() GetLow64 GetSerializeSize, Serialize, Unserialize { BOOST_CHECK_EQUAL(R1L.GetHex(), R1L.ToString()); BOOST_CHECK_EQUAL(R2L.GetHex(), R2L.ToString()); @@ -166,12 +166,12 @@ BOOST_AUTO_TEST_CASE( methods ) // GetHex SetHex begin() end() size() GetLow64 G uint256 TmpL(R1L); BOOST_CHECK_EQUAL(TmpL, R1L); // Verify previous values don't persist when setting to truncated string. - TmpL.SetHex("21"); + TmpL.SetHexDeprecated("21"); BOOST_CHECK_EQUAL(TmpL.ToString(), "0000000000000000000000000000000000000000000000000000000000000021"); - TmpL.SetHex(R2L.ToString()); BOOST_CHECK_EQUAL(TmpL, R2L); - TmpL.SetHex(ZeroL.ToString()); BOOST_CHECK_EQUAL(TmpL, uint256()); + TmpL.SetHexDeprecated(R2L.ToString()); BOOST_CHECK_EQUAL(TmpL, R2L); + TmpL.SetHexDeprecated(ZeroL.ToString()); BOOST_CHECK_EQUAL(TmpL, uint256()); - TmpL.SetHex(R1L.ToString()); + TmpL.SetHexDeprecated(R1L.ToString()); BOOST_CHECK_EQUAL_COLLECTIONS(R1L.begin(), R1L.end(), R1Array, R1Array + R1L.size()); BOOST_CHECK_EQUAL_COLLECTIONS(TmpL.begin(), TmpL.end(), R1Array, R1Array + TmpL.size()); BOOST_CHECK_EQUAL_COLLECTIONS(R2L.begin(), R2L.end(), R2Array, R2Array + R2L.size()); @@ -214,10 +214,10 @@ BOOST_AUTO_TEST_CASE( methods ) // GetHex SetHex begin() end() size() GetLow64 G BOOST_CHECK_EQUAL(MaxS.GetHex(), MaxS.ToString()); uint160 TmpS(R1S); BOOST_CHECK_EQUAL(TmpS, R1S); - TmpS.SetHex(R2S.ToString()); BOOST_CHECK_EQUAL(TmpS, R2S); - TmpS.SetHex(ZeroS.ToString()); BOOST_CHECK_EQUAL(TmpS, uint160()); + TmpS.SetHexDeprecated(R2S.ToString()); BOOST_CHECK_EQUAL(TmpS, R2S); + TmpS.SetHexDeprecated(ZeroS.ToString()); BOOST_CHECK_EQUAL(TmpS, uint160()); - TmpS.SetHex(R1S.ToString()); + TmpS.SetHexDeprecated(R1S.ToString()); BOOST_CHECK_EQUAL_COLLECTIONS(R1S.begin(), R1S.end(), R1Array, R1Array + R1S.size()); BOOST_CHECK_EQUAL_COLLECTIONS(TmpS.begin(), TmpS.end(), R1Array, R1Array + TmpS.size()); BOOST_CHECK_EQUAL_COLLECTIONS(R2S.begin(), R2S.end(), R2Array, R2Array + R2S.size()); diff --git a/src/uint256.cpp b/src/uint256.cpp index f15150ca956..2756a7f5cd8 100644 --- a/src/uint256.cpp +++ b/src/uint256.cpp @@ -18,7 +18,7 @@ std::string base_blob::GetHex() const } template -void base_blob::SetHex(const std::string_view str) +void base_blob::SetHexDeprecated(const std::string_view str) { std::fill(m_data.begin(), m_data.end(), 0); @@ -52,12 +52,12 @@ std::string base_blob::ToString() const // Explicit instantiations for base_blob<160> template std::string base_blob<160>::GetHex() const; template std::string base_blob<160>::ToString() const; -template void base_blob<160>::SetHex(std::string_view); +template void base_blob<160>::SetHexDeprecated(std::string_view); // Explicit instantiations for base_blob<256> template std::string base_blob<256>::GetHex() const; template std::string base_blob<256>::ToString() const; -template void base_blob<256>::SetHex(std::string_view); +template void base_blob<256>::SetHexDeprecated(std::string_view); const uint256 uint256::ZERO(0); const uint256 uint256::ONE(1); diff --git a/src/uint256.h b/src/uint256.h index 406a9c7203c..3f7dce69834 100644 --- a/src/uint256.h +++ b/src/uint256.h @@ -59,7 +59,7 @@ public: // Hex string representations are little-endian. std::string GetHex() const; - void SetHex(std::string_view str); + void SetHexDeprecated(std::string_view str); std::string ToString() const; constexpr const unsigned char* data() const { return m_data.data(); } @@ -119,7 +119,7 @@ public: inline uint256 uint256S(std::string_view str) { uint256 rv; - rv.SetHex(str); + rv.SetHexDeprecated(str); return rv; }