Fix PaymentServer::handleURIOrFile warning message #225

pull hebasto wants to merge 1 commits into bitcoin-core:master from hebasto:210224-server changing 1 files +1 −1

hebasto commented at 12:58 PM on February 24, 2021: member

GUIUtil::parseBitcoinURI does not check address validity. Therefore, an invalid address cannot cause parsing failure.

An invalid address case is covered by its own message.

This PR fixes the warning messages.
hebasto renamed this:
~~qt: Fix PaymentServer::handleURIOrFile warning message~~
Fix PaymentServer::handleURIOrFile warning message
on Feb 24, 2021
Talkless approved
Talkless commented at 2:50 PM on February 28, 2021: none

utACK 2434d004219f90f8d5408d22f222cfb7060565ac, no any testing, just built on Debian Sid with Qt 5.15.2
hebasto force-pushed on May 10, 2021
hebasto commented at 10:26 PM on May 10, 2021: member
Updated 2434d004219f90f8d5408d22f222cfb7060565ac -> 45ebd9e1825886767efc38ddc3e300c638fcae6c (pr225.01 -> pr225.02):
- rebased on top of the #280
- dropped all refactoring changes
hebasto commented at 10:27 PM on May 10, 2021: member

@prayank23

Do you mind reviewing this PR?
hebasto added the label UX on May 10, 2021
ghost commented at 12:47 AM on May 11, 2021: none

GUIUtil::parseBitcoinURI does not check address validity. Therefore, an invalid address cannot cause parsing failure.

If address validity is not checked, can it be the cause for parsing failure?

An invalid address case is covered by its own message "Invalid payment address %1".

Invalid address case is covered by IsValidDestination() and updated in https://github.com/bitcoin-core/gui/pull/280/ however, if code for else statement is executed can we assume address was never checked for validity?

https://github.com/bitcoin-core/gui/blob/4a267057617a8aa6dc9793c4d711725df5338025/src/qt/paymentserver.cpp#L252
hebasto commented at 4:36 PM on May 11, 2021: member

GUIUtil::parseBitcoinURI does not check address validity. Therefore, an invalid address cannot cause parsing failure.

If address validity is not checked, can it be the cause for parsing failure?

Internally the GUIUtil::parseBitcoinURI uses the QUrlQuery class, so if an invalid address does not contain delimiters, it shouldn't cause parsing failure.

An invalid address case is covered by its own message "Invalid payment address %1".

Invalid address case is covered by IsValidDestination() and updated in #280 however, if code for else statement is executed can we assume address was never checked for validity?

I think we can.

qt: Fix PaymentServer::handleURIOrFile warning message

GUIUtil::parseBitcoinURI does not check address validity. Therefore, an
invalid address cannot cause parsing failure.
An invalid address case is covered by its own message "Invalid payment
address %1".

5b20d46f8b

hebasto force-pushed on May 29, 2021
hebasto commented at 1:53 PM on May 29, 2021: member

Rebased 45ebd9e1825886767efc38ddc3e300c638fcae6c -> 5b20d46f8be6b7fc08012d80a9fc42e9dbc812ff (pr225.02 -> pr225.03) on top of the recent CI changes.
Talkless approved
Talkless commented at 6:49 PM on June 10, 2021: none

re-utACK 5b20d46f8be6b7fc08012d80a9fc42e9dbc812ff after rebase.
jarolrod commented at 4:26 AM on July 6, 2021: member

Code Review ACK 5b20d46f8be6b7fc08012d80a9fc42e9dbc812ff 🥃
ghost commented at 11:47 AM on July 6, 2021: none

@hebasto Can you share any examples that I can try for which GUIUtil::parseBitcoinURI will return false and https://github.com/hebasto/gui/blob/5b20d46f8be6b7fc08012d80a9fc42e9dbc812ff/src/qt/paymentserver.cpp#L254 will be executed ?
hebasto commented at 3:44 PM on July 8, 2021: member

@hebasto Can you share any examples that I can try for which GUIUtil::parseBitcoinURI will return false and https://github.com/hebasto/gui/blob/5b20d46f8be6b7fc08012d80a9fc42e9dbc812ff/src/qt/paymentserver.cpp#L254 will be executed ?

I can't. It looks like this code is never executed due to checks in earlier stages, e.g. in PaymentServer::ipcParseCommandLine.

Closing for now.
hebasto closed this on Jul 8, 2021
bitcoin-core locked this on Aug 16, 2022

Contributors

Labels