Panic when a config file is incomplete
danmx opened this issue ยท 4 comments
When a config file is incomplete binserve
should use secure defaults instead of panicing. When a config file is malformed there should be an understandable error message and an exit with a non-zero status code.
Ran in alpine:latest
container:
# ./binserve-v0.1.0-x86_64-unknown-linux-musl
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', src/security.rs:109:47
stack backtrace:
0: 0x65daaa - std::backtrace_rs::backtrace::libunwind::trace::hf222ece681d618dd
at /rustc/7f7a1cbfd3b55daee191247770627afab09eece2/library/std/src/../../backtrace/src/backtrace/libunwind.rs:96
1: 0x65daaa - std::backtrace_rs::backtrace::trace_unsynchronized::h7bfcf0be2fa82989
at /rustc/7f7a1cbfd3b55daee191247770627afab09eece2/library/std/src/../../backtrace/src/backtrace/mod.rs:66
2: 0x65daaa - std::sys_common::backtrace::_print_fmt::h236d9b171ad18828
at /rustc/7f7a1cbfd3b55daee191247770627afab09eece2/library/std/src/sys_common/backtrace.rs:79
3: 0x65daaa - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h4b36b828e94cdbaa
at /rustc/7f7a1cbfd3b55daee191247770627afab09eece2/library/std/src/sys_common/backtrace.rs:58
4: 0x54eb3c - core::fmt::write::h0dd4368b249898df
at /rustc/7f7a1cbfd3b55daee191247770627afab09eece2/library/core/src/fmt/mod.rs:1080
5: 0x65d1b1 - std::io::Write::write_fmt::hc892ee261e6ddf46
at /rustc/7f7a1cbfd3b55daee191247770627afab09eece2/library/std/src/io/mod.rs:1516
6: 0x65cb9d - std::sys_common::backtrace::_print::h8b2bc12d86f6d6ce
at /rustc/7f7a1cbfd3b55daee191247770627afab09eece2/library/std/src/sys_common/backtrace.rs:61
7: 0x65cb9d - std::sys_common::backtrace::print::ha0d71e055133e020
at /rustc/7f7a1cbfd3b55daee191247770627afab09eece2/library/std/src/sys_common/backtrace.rs:48
8: 0x65cb9d - std::panicking::default_hook::{{closure}}::h1db1a3550f89322b
at /rustc/7f7a1cbfd3b55daee191247770627afab09eece2/library/std/src/panicking.rs:208
9: 0x65c222 - std::panicking::default_hook::hfe146431cb18e73a
at /rustc/7f7a1cbfd3b55daee191247770627afab09eece2/library/std/src/panicking.rs:227
10: 0x65c222 - std::panicking::rust_panic_with_hook::hef9392580f57df9b
at /rustc/7f7a1cbfd3b55daee191247770627afab09eece2/library/std/src/panicking.rs:577
11: 0x65bf08 - std::panicking::begin_panic_handler::{{closure}}::hc22f01b65500c5e4
at /rustc/7f7a1cbfd3b55daee191247770627afab09eece2/library/std/src/panicking.rs:484
12: 0x65bed4 - std::sys_common::backtrace::__rust_end_short_backtrace::h23c7f1d7574039e8
at /rustc/7f7a1cbfd3b55daee191247770627afab09eece2/library/std/src/sys_common/backtrace.rs:153
13: 0x65be8d - rust_begin_unwind
at /rustc/7f7a1cbfd3b55daee191247770627afab09eece2/library/std/src/panicking.rs:483
14: 0x54d360 - core::panicking::panic_fmt::h5f46bd9f58c47694
at /rustc/7f7a1cbfd3b55daee191247770627afab09eece2/library/core/src/panicking.rs:85
15: 0x54d17c - core::panicking::panic::hfc8a9856fb2eca80
at /rustc/7f7a1cbfd3b55daee191247770627afab09eece2/library/core/src/panicking.rs:50
16: 0x4130aa - binserve::binserve_init::hc5b7ef2db614b4b6
17: 0x522db2 - std::thread::local::LocalKey<T>::with::h16e9249519e7350b
18: 0x522ab6 - std::thread::local::LocalKey<T>::with::h14c0541cdbdc56bc
19: 0x414810 - binserve::main::h5a440c368087745b
20: 0x522943 - std::sys_common::backtrace::__rust_begin_short_backtrace::h413f99a6607c1379
21: 0x415a58 - main
Panic in Arbiter thread.
Content of the config file:
{}
Thank You @danmx for raising this issue. The bug is valid but why would a user empty/break their configuration file? If the configuration file isn't valid, binserve
shouldn't run but I definitely agree that it must respond with an error message rather than panic
king.
I will implement a feature where it will rewrite the configuration to secure defaults when it's malformed and exit with a non-zero status code in the next release. ๐
Also, if anybody else wants to get it done sooner:
Write a default JSON schema for the configuration file and compare the input JSON if it matches the scheme or not. You can achieve this with valico.
All the configuration related functions and features are in the config.rs
file.
๐ Resources:
As a user I wouldn't like to have a tool rewriting my config unless I explicitly ask it to do it. I consider configs an addition to a default configuration, something that allows me to personalise the experience. I shouldn't be forced to fill all the fields of a config file. In general in config files you put only things that you want to change so if the config is a valid JSON, but empty or with some fields missing, it should be fine.
Yeah, I agree, that seems like the ideal implementation! This would require some work but I will definitely implement this in the next release. ๐
There is a required
fields option in the valico library:
"required": [ ]
I will put the required fields in that and omit the optional ones and proceed to serve. Thank You for the suggestion! โค๏ธ๐
This has been fixed!
v0.2.0 Released! ๐