Skip to content
This repository was archived by the owner on Jan 29, 2023. It is now read-only.

Panic when loading hocon document, for entering unreachable code #26

Open
alexhexabeam opened this issue Aug 22, 2020 · 10 comments
Open

Comments

@alexhexabeam
Copy link

alexhexabeam commented Aug 22, 2020

src/main.rs

use hocon::HoconLoader;

fn main() -> Result<(), Box<dyn std::error::Error>> {
    let doc = HoconLoader::new()
        .load_file(
            "/path/to/my/hocon.conf",
        )?
        .hocon()?;
    println!("{:?}", doc["DefaultParsers"]);
    Ok(())
}

Panics when it loads the document (run in release mode, as debug mode doesn't seem to ever finish loading):

thread 'main' panicked at 'internal error: entered unreachable code', /Users/alexhunt/.cargo/registry/src/suiyiyu.us.kg-1ecc6299db9ec823/hocon-0.3.5/src/internals/value.rs:332:18
stack backtrace:
   0: <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt
   1: core::fmt::write
   2: std::io::Write::write_fmt
   3: std::panicking::default_hook::{{closure}}
   4: std::panicking::default_hook
   5: std::panicking::rust_panic_with_hook
   6: std::panicking::begin_panic
   7: core::hash::impls::<impl core::hash::Hash for [T]>::hash
   8: hashbrown::map::make_hash
   9: hashbrown::rustc_entry::<impl hashbrown::map::HashMap<K,V,S>>::rustc_entry
  10: hocon::internals::internal::HoconInternal::merge
  11: hocon::HoconLoader::hocon
  12: ironlime::main
  13: std::rt::lang_start::{{closure}}
  14: std::rt::lang_start_internal
  15: main

I unfortunately cannot give you the document(s) that cause this, as they are proprietary. I also do not have a minimum working example, as I have no idea what triggers it yet. The file being loaded includes many other files, and exercises many hocon features. In total, it's almost 100,000 lines of hocon, so it may take me a while to find a minimum reproducible example.

It seems to be hitting the unreachable() in src/internals/value.rs:

impl std::hash::Hash for HoconValue {
    fn hash<H>(&self, state: &mut H)
    where
        H: std::hash::Hasher,
    {
        match self {
            HoconValue::Integer(i) => i.hash(state),
            HoconValue::String(s) => s.hash(state),
            _ => unreachable!(),
        };
    }
}

Tested with hocon v0.3.5

@alexhexabeam
Copy link
Author

alexhexabeam commented Aug 22, 2020

I tried cloning master locally and putting in a debug println! to print self in the match catch all arm, and I got the following:

Null("484ceb4e-313f-40df-8e4c-e7ed9279e2cc-1")

This UUID does not appear anywhere in my configs.

Modified code:

impl std::hash::Hash for HoconValue {
    fn hash<H>(&self, state: &mut H)
    where
        H: std::hash::Hasher,
    {
        match self {
            HoconValue::Integer(i) => i.hash(state),
            HoconValue::String(s) => s.hash(state),
            _ => {
                println!("{:?}", self);
                unreachable!()
            }
        };
    }
}

mockersf added a commit that referenced this issue Aug 24, 2020
mockersf added a commit that referenced this issue Aug 24, 2020
mockersf added a commit that referenced this issue Aug 24, 2020
mockersf added a commit that referenced this issue Aug 24, 2020
@mockersf
Copy link
Owner

Hello, and thank you for reporting this !

I added a few debug prints on branch unreachable-null, could you try it ?

Those Null("484ceb4e-313f-40df-8e4c-e7ed9279e2cc-1") are created when you concat two arrays with a substitution, like so:

{
    a : [ 7 ]
    b : ${a} [ 12 ]
}

Parsing this will print the following:

new index "ae3a287e-d282-48ee-9b8e-07308f68a0d9" for [HoconInternal { internal: [([], PathSubstitutionInParent(UnquotedString("a")))] }, HoconInternal { internal: [([], Integer(12))] }]
merging
adding [UnquotedString("a "), Integer(0)] - Integer(7)
-- value: Integer(7)
----> item: String("a") | []
----> item: Integer(0) | [String("a")]
-- done for value
adding [UnquotedString("b "), Null("ae3a287e-d282-48ee-9b8e-07308f68a0d9-0")] - PathSubstitutionInParent(UnquotedString("a"))
adding [UnquotedString("b "), Null("ae3a287e-d282-48ee-9b8e-07308f68a0d9-1")] - Integer(12)
-- value: Integer(12)
----> item: String("b") | []
----> item: Null("ae3a287e-d282-48ee-9b8e-07308f68a0d9-1") | [String("b")]
-- done for value

lines before merging are when this Null value is created
lines after the merging are when the error occurs in your case. Hopefully it should help you find a reproducible smaller case or at least more information

@alexhexabeam
Copy link
Author

I haven't yet found the source of the original error, but I did find another situation that panics:

DefaultParsers = ${DefaultParsers}${WazuhParsers}

Just loading a file with this single line causes it to panic with a stack overflow.

merging
adding [UnquotedString("DefaultParsers ")] - Concat([PathSubstitution { target: UnquotedString("DefaultParsers"), optional: false, original: None }, PathSubstitution { target: UnquotedString("WazuhParsers"), optional: false, original: None }])
-- value: Concat([PathSubstitution { target: UnquotedString("DefaultParsers"), optional: false, original: None }, PathSubstitution { target: UnquotedString("WazuhParsers"), optional: false, original: None }])
----> item: String("DefaultParsers") | []
-- done for value

thread 'main' has overflowed its stack
fatal runtime error: stack overflow
Abort trap: 6

@alexhexabeam
Copy link
Author

alexhexabeam commented Aug 24, 2020

I have found a simplified example that causes a different unreachable code issue:

include "parsers_wazuh.conf"
DefaultParsers = []
DefaultParsers = ${DefaultParsers}${WazuhParsers}

parsers_wazuh.conf

Fields = []

WazuhParsers = [
  {
    Fields = ${Fields} []
  },
]

Causes:

new index "25933419-25be-4776-828f-f92e55e3aaab" for [HoconInternal { internal: [([], PathSubstitutionInParent(UnquotedString("Fields")))] }]
new index "131f8911-39c5-45ae-86d4-f076f6be9425" for [HoconInternal { internal: [([UnquotedString("Fields "), Null("25933419-25be-4776-828f-f92e55e3aaab-0")], PathSubstitutionInParent(UnquotedString("Fields")))] }]
merging
adding [UnquotedString("Fields ")] - Included { value: EmptyArray, include_root: None, original_path: [UnquotedString("Fields ")] }
-- value: Included { value: EmptyArray, include_root: None, original_path: [UnquotedString("Fields ")] }
----> item: String("Fields") | []
-- done for value
adding [UnquotedString("WazuhParsers "), Null("131f8911-39c5-45ae-86d4-f076f6be9425-0"), UnquotedString("Fields "), Null("25933419-25be-4776-828f-f92e55e3aaab-0")] - Included { value: PathSubstitutionInParent(UnquotedString("Fields")), include_root: None, original_path: [UnquotedString("WazuhParsers "), Null("131f8911-39c5-45ae-86d4-f076f6be9425-0"), UnquotedString("Fields "), Null("25933419-25be-4776-828f-f92e55e3aaab-0")] }
-- value: Included { value: PathSubstitutionInParent(UnquotedString("Fields")), include_root: None, original_path: [UnquotedString("WazuhParsers "), Null("131f8911-39c5-45ae-86d4-f076f6be9425-0"), UnquotedString("Fields "), Null("25933419-25be-4776-828f-f92e55e3aaab-0")] }
----> item: String("WazuhParsers") | []
----> item: Null("131f8911-39c5-45ae-86d4-f076f6be9425-0") | [String("WazuhParsers")]
----> item: String("Fields") | [String("WazuhParsers"), Null("131f8911-39c5-45ae-86d4-f076f6be9425-0")]
----> item: Null("25933419-25be-4776-828f-f92e55e3aaab-0") | [String("WazuhParsers"), Null("131f8911-39c5-45ae-86d4-f076f6be9425-0"), String("Fields")]
-- done for value
adding [UnquotedString("DefaultParsers ")] - EmptyArray
-- value: EmptyArray
----> item: String("DefaultParsers") | []
-- done for value
adding [UnquotedString("DefaultParsers ")] - Concat([PathSubstitution { target: UnquotedString("DefaultParsers"), optional: false, original: None }, PathSubstitution { target: UnquotedString("WazuhParsers"), optional: false, original: None }])
-- value: Concat([PathSubstitution { target: UnquotedString("DefaultParsers"), optional: false, original: None }, PathSubstitution { target: UnquotedString("WazuhParsers"), optional: false, original: None }])
----> item: String("DefaultParsers") | []
-- done for value
thread 'main' panicked at 'internal error: entered unreachable code', /Users/alexhunt/rust/hocon.rs/src/internals/value.rs:177:56
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
            // These cases should have been replaced during substitution
            // and not exist anymore at this point
            HoconValue::Temp => unreachable!(),
            HoconValue::EmptyObject => unreachable!(),
            HoconValue::EmptyArray => unreachable!(),
            HoconValue::PathSubstitutionInParent(_) => unreachable!(),  // PANICS HERE
            HoconValue::ToConcatToArray { .. } => unreachable!(),

Note that this only seems to happen if I'm including it from a separate file, not if it is all in one file.

EDIT: the value in the PathSubstitutionInParent is UnquotedString("Fields")

mockersf added a commit that referenced this issue Aug 25, 2020
…h a substitution #26

alsoo fix case for stack overflow when substituting a value that hasn't been defined in some cases
@mockersf
Copy link
Owner

I pushed a fix for those two issues to master, and also a fix to #27 that I found while trying stuff...

Could you test master if it's better for you ?

@alexhexabeam
Copy link
Author

This seems to have resolved all three panics, thank you! I now get BadValue(MissingKey) for the two panics, and a string null for the stack overflow.

I am still getting some strange behavior. Maybe this is normal for hocon or this library, though.
This:

DefaultParsers = ${DefaultParsers} ${WazuhParsers}

Gets me:

String("null  ")

and this:

DefaultParsers = ["asdf"]
WazuhParsers = ["qwer"]
DefaultParsers = ${DefaultParsers} ${WazuhParsers}

Gets me:

String(" ")

@mockersf
Copy link
Owner

should be fixed also, that now gets

{
  "DefaultParsers": [
    "asdf",
    "qwer"
  ],
  "WazuhParsers": [
    "qwer"
  ]
}

@alexhexabeam
Copy link
Author

Much better!

DefaultParsers = ["asdf"]
WazuhParsers = ["qwer"]
DefaultParsers = ${DefaultParsers} ${WazuhParsers}

now gives Array([String("asdf"), String(""), String("qwer")])

I'm not sure if that is normal for it to add that empty string in the middle.

@mockersf
Copy link
Owner

You're right, the "" is not normal... https://github.com/lightbend/config/blob/master/HOCON.md#note-concatenation-with-whitespace-and-substitutions
It's not there anymore now, don't know why I removed it from my test case when I copy pasted your example

@alexhexabeam
Copy link
Author

alexhexabeam commented Aug 27, 2020

Thanks, that one works now.

I'm still getting

String("null  ")

from

DefaultParsers = ${DefaultParsers} ${WazuhParsers}

Not sure if that's normal. I would have thought that both would be "null", or both would be an empty string "".

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants