Skip to content

Conversation

@laanwj
Copy link
Member

@laanwj laanwj commented Feb 28, 2014

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.

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.
@BitcoinPullTester
Copy link

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/1b8fd35aadfad6a1e55391f02add6076c8c9ea8f for binaries and test log.
This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/
Contact BlueMatt on freenode if something looks broken.

@sipa
Copy link
Member

sipa commented Feb 28, 2014

Do we really want to continue in case of a code typo?

@laanwj
Copy link
Member Author

laanwj commented Feb 28, 2014

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.

@gavinandresen
Copy link
Contributor

ACK from me, much better to have errors in debug.log than assertion failures.

gavinandresen added a commit that referenced this pull request Feb 28, 2014
Make tinyformat errors raise an exception instead of assert()ing
@gavinandresen gavinandresen merged commit 829f822 into bitcoin:master Feb 28, 2014
@laanwj laanwj deleted the 2014_02_tinyformat_errors_non_fatal branch April 9, 2014 14:09
laanwj added a commit to laanwj/bitcoin that referenced this pull request Jun 27, 2016
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.
@laanwj laanwj mentioned this pull request Jun 27, 2016
zkbot pushed a commit to zcash/zcash that referenced this pull request Oct 25, 2016
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
protonn pushed a commit to argentumproject/argentum that referenced this pull request May 7, 2017
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.
lateminer pushed a commit to lateminer/bitcoin that referenced this pull request Jan 3, 2019
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.
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants