Skip to content
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

Broken bisq.properties file prevents app from starting #3966

Closed
ripcurlx opened this issue Feb 12, 2020 · 0 comments · Fixed by #3967
Closed

Broken bisq.properties file prevents app from starting #3966

ripcurlx opened this issue Feb 12, 2020 · 0 comments · Fixed by #3967
Assignees
Labels

Comments

@ripcurlx
Copy link
Contributor

ripcurlx commented Feb 12, 2020

Description

If the content of the bisq.properties file gets corrupted somehow the app is not starting anymore.

Version

Current master and release branch. Introduced by config refactoring in #3889.

Steps to reproduce

Add invalid properties like:

#Wed Feb 12 17:05:20 CET 2020
\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000=
baseCurrencyNetwork=BTC_REGTEST

into your bisq.properties file

Expected behaviour

App starts and ignores invalid keys (old behavior) or warns the user that she needs to check her bisq.properties file to clean it up from any invalid content.

Actual behaviour

App is not starting without any user feedback

@ripcurlx ripcurlx added the a:bug label Feb 12, 2020
cbeams added a commit to cbeams/bisq that referenced this issue Feb 12, 2020
Prior to this commit, Bisq's new `Config` infrastructure would throw an
exception on encountering an unrecognized option in the
`bisq.properties` config file and the Bisq application would handle this
by reporting an error at the command line indicating that the given
option was not recognized.

For example, given a bisq.properties file that reads as follows:

    $ cat $APP_DATA_DIR/bisq.properties
    bogus=42

running Bisq would exit with the following error message:

    $ java -jar bisq.jar
    error: problem parsing option 'bogus': bogus is not a recognized option.

This was reasonable enough behavior, but it had two problems. The first
is that the error message did not indicate to the user that the
unrecognized option was found in the config file as opposed to the
command line. Users unfamiliar with the config file might not know to
look there to eliminate or fix the offending option. The second problem
surfaced when testing the Windows executable build for the v1.2.6
release. When such an error was encountered, the executable would just
fail silently, reporting nothing to the user.

Both of these problems should be addressed on their own, i.e. option
parsing errors should report to the user whether the offending option
was at the command line or in the config file, and our packaged
executables should never just fail silently if we can avoid it.

However, this change does not address either of these problems directly.
It rather avoids the problems completely by relaxing config file
parsing to silently allow unrecognized options. This behavior mimics
prior (pre-`Config`) Bisq versions, and it also follows suit with
Bitcoin Core's own config file processing.

This fixes bisq-network#3966, which surfaced this problem. In that particular issue,
an option line had somehow been added to the user's bisq.properties
config file that consisted of an option key containing 452 octal null
characters (\u0000) followed by an equals sign and an empty option
value. It is unknown how this bogus option was ever added to the user's
config file in the first place, but on the chance that previous Bisq
versions somehow added this, the changes in this commit ensure that such
entries will not cause affected users' Bisq applications to silently
fail in versions 1.2.6 and beyond.
ripcurlx pushed a commit that referenced this issue Feb 12, 2020
Prior to this commit, Bisq's new `Config` infrastructure would throw an
exception on encountering an unrecognized option in the
`bisq.properties` config file and the Bisq application would handle this
by reporting an error at the command line indicating that the given
option was not recognized.

For example, given a bisq.properties file that reads as follows:

    $ cat $APP_DATA_DIR/bisq.properties
    bogus=42

running Bisq would exit with the following error message:

    $ java -jar bisq.jar
    error: problem parsing option 'bogus': bogus is not a recognized option.

This was reasonable enough behavior, but it had two problems. The first
is that the error message did not indicate to the user that the
unrecognized option was found in the config file as opposed to the
command line. Users unfamiliar with the config file might not know to
look there to eliminate or fix the offending option. The second problem
surfaced when testing the Windows executable build for the v1.2.6
release. When such an error was encountered, the executable would just
fail silently, reporting nothing to the user.

Both of these problems should be addressed on their own, i.e. option
parsing errors should report to the user whether the offending option
was at the command line or in the config file, and our packaged
executables should never just fail silently if we can avoid it.

However, this change does not address either of these problems directly.
It rather avoids the problems completely by relaxing config file
parsing to silently allow unrecognized options. This behavior mimics
prior (pre-`Config`) Bisq versions, and it also follows suit with
Bitcoin Core's own config file processing.

This fixes #3966, which surfaced this problem. In that particular issue,
an option line had somehow been added to the user's bisq.properties
config file that consisted of an option key containing 452 octal null
characters (\u0000) followed by an equals sign and an empty option
value. It is unknown how this bogus option was ever added to the user's
config file in the first place, but on the chance that previous Bisq
versions somehow added this, the changes in this commit ensure that such
entries will not cause affected users' Bisq applications to silently
fail in versions 1.2.6 and beyond.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants