rust-cli / rust-cli/config-rs

The `unreachable!()`s in `de.rs` are reachable

Open
#126 6 comments 7 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

E-help-wanted
Dominant language
Rust
Stars
3.2k
Forks
265
Avg merge
2h 42m
Merged PRs (30d)
4

Description

It is possible to make this crate to panic when supplying malformed configuration options. We do have a fair amount of wrapper around this crate, so it is not trivial to minimize the reproducer, so here's a broad approximation of what we have and how this happens:

struct Config { 
    enum: EnumConfig,
}

enum EnumConfig {
    Foo,
    Bar { filename: std::path::PathBuf },
}

which then works fine when the following toml is supplied:

enum.bar.filename = "blah"

and will also work this way:

enum = "foo"

but will panic when the configuration is mutated to be more along the lines of:

enum = "bar"

The relevant portion of the stack trace is:

thread 'main' panicked at 'internal error: entered unreachable code', ~/.cargo/registry/src/github.com-1ecc6299db9ec823/config-0.9.3/src/de.rs:459:18

<snip>

  11: std::panicking::begin_panic
             at /rustc/5c5b8afd80e6fa1d24632153cb2257c686041d41/src/libstd/panicking.rs:400
  12: <config::de::EnumAccess as serde::de::VariantAccess>::struct_variant
             at ./<::std::macros::panic macros>:3
  13: <crate::builder::_IMPL_DESERIALIZE_FOR_EnumConfig::<impl serde::de::Deserialize for crate::builder::EnumConfig>::deserialize::__Visitor as serde::de::Visitor>::visit_enum
             at src/builder.rs:13
  14: config::de::<impl serde::de::Deserializer for config::value::Value>::deserialize_enum
             at ~/.cargo/registry/src/github.com-1ecc6299db9ec823/config-0.9.3/src/de.rs:254
  15: crate::builder::_IMPL_DESERIALIZE_FOR_EnumConfig::<impl serde::de::Deserialize for crate::builder::EnumConfig>::deserialize
             at src/builder.rs:13
  16: <core::marker::PhantomData<T> as serde::de::DeserializeSeed>::deserialize
             at ~/.cargo/registry/src/github.com-1ecc6299db9ec823/serde-1.0.102/src/de/mod.rs:786
  17: <config::de::MapAccess as serde::de::MapAccess>::next_value_seed
             at ~/.cargo/registry/src/github.com-1ecc6299db9ec823/config-0.9.3/src/de.rs:358
  18: serde::de::MapAccess::next_value
             at ~/.cargo/registry/src/github.com-1ecc6299db9ec823/serde-1.0.102/src/de/mod.rs:1847
  19: <crate::_IMPL_DESERIALIZE_FOR_Config::<impl serde::de::Deserialize for crate::Config>::deserialize::__Visitor as serde::de::Visitor>::visit_map
             at src/main.rs:13
  20: config::de::<impl serde::de::Deserializer for config::value::ValueWithKey>::deserialize_any
             at ~/.cargo/registry/src/github.com-1ecc6299db9ec823/config-0.9.3/src/de.rs:26
  21: config::de::<impl serde::de::Deserializer for config::value::ValueWithKey>::deserialize_struct
             at ./<::serde::macros::forward_to_deserialize_any_method macros>:7
  22: crate::_IMPL_DESERIALIZE_FOR_Config::<impl serde::de::Deserialize for crate::Config>::deserialize
             at src/main.rs:13
  23: config::config::Config::get
             at ~/.cargo/registry/src/github.com-1ecc6299db9ec823/config-0.9.3/src/config.rs:164

<snip>

I recommend replacing all of these unreachables with proper errors.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in de.rs at the VariantAccess::struct_variant path reported in the stack trace, then inspect the related deserialization code and src/builder.rs. Reproduce the malformed enum configuration shown in the issue and identify each reachable unreachable!() path. Done means malformed configurations return proper errors instead of panicking, with coverage for the reported case.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
tooling
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.