Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Recurrence of recursive mutator bug #34

Closed
teymour-aldridge opened this issue Aug 13, 2022 · 6 comments
Closed

Recurrence of recursive mutator bug #34

teymour-aldridge opened this issue Aug 13, 2022 · 6 comments

Comments

@teymour-aldridge
Copy link
Collaborator

Unfortunately I think that #31 is still here: I tried writing a recursive JSON generator recently, but this crashes pretty quickly after being launched. I thought this might be because I modify the fuzzcheck generated types before converting them to the serde_json::Value enum (stripping invalid characters from the strings), but I removed that code and it didn't seem to fix the problem. The code is at https://git.sr.ht/~teymour/fuzzcheck_generators/tree/ (in fuzzcheck_serde_json_generator), and I get this error output (using the latest commit in the fuzzcheck repository)

thread 'main' panicked at 'The mutator used by the fuzz test does not evaluate the complexity of the test cases consistently.
                    This is a bug in the implementation of fuzzcheck::mutators::map::MapMutator<fuzzcheck_serde_json_generator::InternalJsonValue, serde_json::value::Value, fuzzcheck::mutators::recursive::RecursiveMutator<fuzzcheck_serde_json_generator::InternalJsonValueMutator>, fuzzcheck_serde_json_generator::json_value_mutator::{{closure}}, fuzzcheck_serde_json_generator::json_value_mutator::{{closure}}, fuzzcheck_serde_json_generator::json_value_mutator::{{closure}}>
                    =============
                    
                    {"":[]}

                    =============
                    ', /Users/teymour/.cargo/git/checkouts/fuzzcheck-rs-531eb3f95f61a5dd/2bfa9f0/fuzzcheck/src/fuzzer.rs:445:17
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
FAILED
@loiclec
Copy link
Owner

loiclec commented Aug 13, 2022

ah, that’s too bad :( I will have another look at it tonight. Recursive mutators are definitely the trickiest ones to write, so I am not surprised that there are some problems with them, but it’s disappointing nonetheless 😞

@loiclec
Copy link
Owner

loiclec commented Aug 13, 2022

I am on my phone and can’t look properly into it, but a potential problem might come from the usage of MapMutator. Since you’re removing some characters from the strings, you should be modifying their complexity as well. Admittedly the API and documentation for this is not good. I’ll give more details on this later again :)

@teymour-aldridge
Copy link
Collaborator Author

but a potential problem might come from the usage of MapMutator. Since you’re removing some characters from the strings, you should be modifying their complexity as well.

I think this was the issue; I wrote some code to compute the complexity of serde_json::Value and it seems to have fixed the problem :).

Apologies for the incorrect bug report.

@teymour-aldridge
Copy link
Collaborator Author

Unfortunately although writing a manual implementation has solved the problem, I still get the issue if I remove the manual complexity computation code (even after removing the code to strip from strings) - this branch should work I think, but doesn't https://git.sr.ht/~teymour/fuzzcheck_generators/tree/e2aebbcbf8389125b9345a2aa5767a8d3f8682fb/item/fuzzcheck_serde_json_generator/ (sorry about my confusion on this).

@loiclec
Copy link
Owner

loiclec commented Aug 19, 2022

No worries at all :-)

The bug comes from when an InternalJsonValue with duplicate keys is mapped to a serde_json::Value. For example:

Object {
    inner: [
        (
            "",
            Array {
                inner: [],
            },
        ),
        (
            "",
            Number {
                inner: 12359031397468760889,
            },
        ),
        (
            "",
            Null,
        ),
    ],
}

complexity = 78.0

gets mapped to:

Object({
    "": Null,
})
complexity = 78.0  // <--- wrong!

@loiclec
Copy link
Owner

loiclec commented Aug 19, 2022

The problem, I think, is that when I first wrote MapMutator, there wasn't any consistency requirement on the complexity of the values, so the API was very easy to use.

Now I have made fuzzcheck a lot stricter about this, and it is too easy to create a MapMutator that doesn't handle the complexity correctly. So the fuzzer crashes with a confusing message and no easy way to debug it :(

@loiclec loiclec closed this as completed Aug 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants