-
Notifications
You must be signed in to change notification settings - Fork 38.8k
Make tinyformat errors raise an exception instead of assert()ing #3767
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make tinyformat errors raise an exception instead of assert()ing #3767
Conversation
By default tinyformat errors such as 'wrong number of conversion specifiers in format string' cause an assertion failure. Raise an exception instead so that error handling can recover or can show an appropriate error.
|
Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/1b8fd35aadfad6a1e55391f02add6076c8c9ea8f for binaries and test log. |
|
Do we really want to continue in case of a code typo? |
|
It's a matter of robustness. Let's think about the following (completely imaginary) scenario: Some rarely-hit debug logging code in the network code is using the wrong formatting. An attacker that can manage to hit this can bring down the entire node. If we just raise a runtime exception, all it does is log and exception and kill the current connection. A nuisance instead of a DoS. |
|
ACK from me, much better to have errors in debug.log than assertion failures. |
Make tinyformat errors raise an exception instead of assert()ing
Updates `tinyformat.h` to commit c42f/tinyformat@3a33bbf upstream. Makes sure that our local changes are kept: - bitcoin#3767 1b8fd35 Make tinyformat errors raise an exception instead of assert()ing - bitcoin#4735 9b6d4c5 Move strprintf define to tinyformat.h - bitcoin#4748 6e5fd00 include stdexcept (for std::exception) - bitcoin#8000 9eaa0af force USE_VARIADIC_TEMPLATES - Add `std::string format(const std::string &fmt...` added this at the time, as we want to be able to do `strprintf(_(...), ...)` Inspired by bitcoin#8264.
util: Update tinyformat Updates `tinyformat.h` to commit c42f/tinyformat@3a33bbf upstream. Makes sure that our local changes are kept: - bitcoin/bitcoin#3767 1b8fd35 Make tinyformat errors raise an exception instead of assert()ing - bitcoin/bitcoin#4735 9b6d4c5 Move strprintf define to tinyformat.h - bitcoin/bitcoin#4748 6e5fd00 include stdexcept (for std::exception) - bitcoin/bitcoin#8000 9eaa0af force USE_VARIADIC_TEMPLATES - Add `std::string format(const std::string &fmt...` added this at the time, as we want to be able to do `strprintf(_(...), ...)` Inspired by bitcoin/bitcoin#8264. For Zcash: ref #1349
Updates `tinyformat.h` to commit c42f/tinyformat@3a33bbf upstream. Makes sure that our local changes are kept: - bitcoin#3767 1b8fd35 Make tinyformat errors raise an exception instead of assert()ing - bitcoin#4735 9b6d4c5 Move strprintf define to tinyformat.h - bitcoin#4748 6e5fd00 include stdexcept (for std::exception) - bitcoin#8000 9eaa0af force USE_VARIADIC_TEMPLATES - Add `std::string format(const std::string &fmt...` added this at the time, as we want to be able to do `strprintf(_(...), ...)` Inspired by bitcoin#8264.
Updates `tinyformat.h` to commit c42f/tinyformat@3a33bbf upstream. Makes sure that our local changes are kept: - bitcoin#3767 1b8fd35 Make tinyformat errors raise an exception instead of assert()ing - bitcoin#4735 9b6d4c5 Move strprintf define to tinyformat.h - bitcoin#4748 6e5fd00 include stdexcept (for std::exception) - bitcoin#8000 9eaa0af force USE_VARIADIC_TEMPLATES - Add `std::string format(const std::string &fmt...` added this at the time, as we want to be able to do `strprintf(_(...), ...)` Inspired by bitcoin#8264.
By default tinyformat errors such as 'wrong number of conversion specifiers in format string' cause an assertion failure.
Raise an exception instead so that error handling can recover or can show an appropriate error.