From 7f9c935a8b4ace6b6fdbbbdd6e62a85b61b738ba Mon Sep 17 00:00:00 2001 From: Chris Beams Date: Wed, 12 Feb 2020 17:58:22 +0100 Subject: [PATCH] Allow unrecognized options in config file 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. --- common/src/main/java/bisq/common/config/Config.java | 6 ++++++ .../src/test/java/bisq/common/config/ConfigTests.java | 11 +++++++++++ 2 files changed, 17 insertions(+) diff --git a/common/src/main/java/bisq/common/config/Config.java b/common/src/main/java/bisq/common/config/Config.java index a4f5dcd2369..a9fc517a215 100644 --- a/common/src/main/java/bisq/common/config/Config.java +++ b/common/src/main/java/bisq/common/config/Config.java @@ -612,6 +612,12 @@ public Config(String defaultAppName, File defaultUserDataDir, String... args) { OptionSet cliOpts = parser.parse(args); options.addOptionSet(cliOpts); + // Option parsing is strict at the command line, but we relax it now for any + // subsequent config file processing. This is for compatibility with pre-1.2.6 + // versions that allowed unrecognized options in the bisq.properties config + // file and because it follows suit with Bitcoin Core's config file behavior. + parser.allowsUnrecognizedOptions(); + // Parse config file specified at the command line only if it was specified as // an absolute path. Otherwise, the config file will be processed later below. File configFile = null; diff --git a/common/src/test/java/bisq/common/config/ConfigTests.java b/common/src/test/java/bisq/common/config/ConfigTests.java index 91554893881..cd73287f6eb 100644 --- a/common/src/test/java/bisq/common/config/ConfigTests.java +++ b/common/src/test/java/bisq/common/config/ConfigTests.java @@ -113,6 +113,17 @@ public void whenUnrecognizedOptionIsSet_thenConfigExceptionIsThrown() { configWithOpts(opt("bogus")); } + @Test + public void whenUnrecognizedOptionIsSetInConfigFile_thenNoExceptionIsThrown() throws IOException { + File configFile = File.createTempFile("bisq", "properties"); + try (PrintWriter writer = new PrintWriter(configFile)) { + writer.println(new ConfigFileOption("bogusOption", "bogusValue")); + writer.println(new ConfigFileOption(APP_NAME, "BisqTest")); + } + Config config = configWithOpts(opt(CONFIG_FILE, configFile.getAbsolutePath())); + assertThat(config.appName, equalTo("BisqTest")); + } + @Test public void whenOptionFileArgumentDoesNotExist_thenConfigExceptionIsThrown() { exceptionRule.expect(ConfigException.class);