Page MenuHomePhabricator

Prevent downloading the payment request if it's larger than the max allowed
ClosedPublic

Authored by Fabien on Jul 3 2024, 21:34.

Details

Summary

When a URI contains a r=<url> field, the payment request is downloaded from this url then the size is checked against a limit (currently 50kB). This diff improves this behavior so that the node will not bother downloading the whole request if it detects it's oversized.

Test Plan

Use "File->Open URI" to load a URI containing r=<url> where the url contains a file > 50kB. Check the download is aborted quickly and the node reports a useful error message. Repeat the process several times.

ninja check-bitcoin-qt

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Fabien requested review of this revision.Jul 3 2024, 21:34

Tail of the build log:

[521/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/platformstyle.cpp.o
[522/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/qvalidatedlineedit.cpp.o
[523/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/optionsdialog.cpp.o
[524/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/optionsmodel.cpp.o
[525/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/qvaluecombobox.cpp.o
[526/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/peertablemodel.cpp.o
[527/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/utilitydialog.cpp.o
[528/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/addressbookpage.cpp.o
[529/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/trafficgraphwidget.cpp.o
[530/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/qrc_bitcoin_locale.cpp.o
[531/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/splashscreen.cpp.o
[532/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/coincontroltreewidget.cpp.o
[533/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/rpcconsole.cpp.o
[534/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/createwalletdialog.cpp.o
[535/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/askpassphrasedialog.cpp.o
[536/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/addresstablemodel.cpp.o
[537/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/editaddressdialog.cpp.o
[538/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/coincontroldialog.cpp.o
[539/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/qrimagewidget.cpp.o
[540/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/openuridialog.cpp.o
[541/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/overviewpage.cpp.o
[542/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/receivecoinsdialog.cpp.o
[543/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/receiverequestdialog.cpp.o
[544/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/recentrequeststablemodel.cpp.o
[545/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/sendcoinsentry.cpp.o
[546/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/transactiondescdialog.cpp.o
[547/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/transactionfilterproxy.cpp.o
[548/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/sendcoinsdialog.cpp.o
[549/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/signverifymessagedialog.cpp.o
[550/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/transactionrecord.cpp.o
[551/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/transactiontablemodel.cpp.o
[552/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/transactionview.cpp.o
[553/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/walletmodeltransaction.cpp.o
[554/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/paymentrequestplus.cpp.o
[555/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/walletcontroller.cpp.o
[556/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/walletframe.cpp.o
[557/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/walletview.cpp.o
[558/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/qrc_bitcoin.cpp.o
[559/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/transactiondesc.cpp.o
[560/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/paymentserver.cpp.o
FAILED: src/qt/CMakeFiles/bitcoin-qt-base.dir/paymentserver.cpp.o 
/usr/bin/ccache /usr/bin/clang++ -DBOOST_ALL_NO_LIB -DBUILD_BITCOIN_INTERNAL -DENABLE_AVX2 -DENABLE_SHANI -DENABLE_SSE41 -DHAVE_BUILD_INFO -DHAVE_CONFIG_H -DHAVE_CONSENSUS_LIB -DHAVE_DECL_EVP_MD_CTX_NEW=1 -DLEVELDB_ATOMIC_PRESENT -DLEVELDB_PLATFORM_POSIX -DOS_LINUX -DQT_CORE_LIB -DQT_DBUS_LIB -DQT_GUI_LIB -DQT_NETWORK_LIB -DQT_NO_DEBUG -DQT_WIDGETS_LIB -DUSE_PROTOBUF_MESSAGE_BYTESIZELONG -Isrc/qt/bitcoin-qt-base_autogen/include -I../../src/. -Isrc -I../../src/univalue/include -Isrc/crypto/.. -I../../src/secp256k1/include -I../../src/leveldb/include -Isrc/qt -isystem /usr/include/jemalloc -isystem /usr/include/miniupnpc -isystem /usr/include/x86_64-linux-gnu/qt5 -isystem /usr/include/x86_64-linux-gnu/qt5/QtWidgets -isystem /usr/include/x86_64-linux-gnu/qt5/QtGui -isystem /usr/include/x86_64-linux-gnu/qt5/QtCore -isystem /usr/lib/x86_64-linux-gnu/qt5/mkspecs/linux-g++ -isystem /usr/include/x86_64-linux-gnu/qt5/QtNetwork -isystem /usr/include/x86_64-linux-gnu/qt5/QtDBus -Werror -g -O2 -fPIC -fvisibility=hidden -fstack-protector-all -Wstack-protector -fcf-protection=full -fstack-clash-protection -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Wall -Wextra -Wformat -Wgnu -Wvla -Wcast-align -Wunused-parameter -Wmissing-braces -Wthread-safety -Wrange-loop-analysis -Wredundant-decls -Wunreachable-code-loop-increment -Wsign-compare -Wconditional-uninitialized -Wdocumentation -Wformat-security -Wredundant-move -Woverloaded-virtual -Wshadow -Wshadow-field -Wno-unused-parameter -Wno-implicit-fallthrough -Wno-psabi -pthread -fPIC -std=gnu++17 -MD -MT src/qt/CMakeFiles/bitcoin-qt-base.dir/paymentserver.cpp.o -MF src/qt/CMakeFiles/bitcoin-qt-base.dir/paymentserver.cpp.o.d -o src/qt/CMakeFiles/bitcoin-qt-base.dir/paymentserver.cpp.o -c ../../src/qt/paymentserver.cpp
../../src/qt/paymentserver.cpp:810:55: error: declaration shadows a field of 'PaymentServer' [-Werror,-Wshadow]
void PaymentServer::netRequestFinished(QNetworkReply *reply) {
                                                      ^
../../src/./qt/paymentserver.h:168:20: note: previous declaration is here
    QNetworkReply *reply{nullptr};
                   ^
../../src/qt/paymentserver.cpp:872:52: error: declaration shadows a field of 'PaymentServer' [-Werror,-Wshadow]
void PaymentServer::reportSslErrors(QNetworkReply *reply,
                                                   ^
../../src/./qt/paymentserver.h:168:20: note: previous declaration is here
    QNetworkReply *reply{nullptr};
                   ^
2 errors generated.
[561/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/bitcoin-qt-base_autogen/mocs_compilation.cpp.o
[562/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/bitcoin.cpp.o
[563/569] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/walletmodel.cpp.o
ninja: build stopped: cannot make progress due to previous errors.
Build build-clang failed with exit code 1
This revision is now accepted and ready to land.Jul 3 2024, 22:42