mufeedvh/binserve

Panic when a config file is incomplete

danmx opened this issue ยท 4 comments

danmx commented

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 panicking.

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:

danmx commented

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! ๐ŸŽ‰