Skip to content

Commit

Permalink
Fuzz serde enum representations (#502)
Browse files Browse the repository at this point in the history
* Fuzz untagged enums

* Fuzz adjacently tagged enums

* Fuzz internally tagged enums

* Add fuzzing of other variants for adjacently and internally tagged enums

* Fuzz internally tagged structs

* Fuzz flattened structs and struct variants

* Fix missing code coverage

* Document limitations of RON and serde

* Add many test cases for known limitations
  • Loading branch information
juntyr committed Oct 7, 2023
1 parent 2f3e5a8 commit 5f12b4a
Show file tree
Hide file tree
Showing 19 changed files with 8,045 additions and 421 deletions.
20 changes: 19 additions & 1 deletion .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,23 @@ jobs:
- run: cargo clippy --features integer128 -- -D warnings
- run: cargo clippy --features indexmap -- -D warnings
- run: cargo clippy --all-features -- -D warnings

clippy-fuzz:
name: "Clippy: Fuzzer"
runs-on: ubuntu-latest
continue-on-error: true
steps:
- uses: actions/checkout@v3
- uses: ./.github/actions/setup
with:
key: clippy-fuzz
- uses: actions-rs/toolchain@v1
with:
toolchain: stable
profile: minimal
components: clippy
override: true
- run: cd fuzz && cargo clippy --all -- -D warnings

rustfmt:
name: "Format: stable"
Expand All @@ -69,7 +86,8 @@ jobs:
profile: minimal
components: rustfmt
override: true
- run: cargo fmt -- --check
- run: cargo fmt --all -- --check
- run: cd fuzz && cargo fmt --all -- --check

coverage:
name: "Coverage: stable"
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Add CIFuzz GitHub action ([#429](https://github.com/ron-rs/ron/pull/429))
- Update the arbitrary fuzzer to check arbitrary serde data types, values, and `ron::ser::PrettyConfig`s ([#465](https://github.com/ron-rs/ron/pull/465))
- Add a benchmark for PRs that runs over the latest fuzzer corpus ([#465](https://github.com/ron-rs/ron/pull/465))
- Fuzz serde enum representation and flatten attributes and collect current limitations in ron and serde ([#502](https://github.com/ron-rs/ron/pull/502))

## [0.8.1] - 2023-08-17

Expand Down
36 changes: 28 additions & 8 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -185,15 +185,35 @@ RON is not designed to be a fully self-describing format (unlike JSON) and is th

While data structures with any of these attributes should generally roundtrip through RON, some restrictions apply [^serde-restrictions] and their textual representation may not always match your expectation:

- flattened structs are only serialised as maps and deserialised from maps
- struct names inside an internally (or adjacently) tagged or untagged enum, e.g. by enabling the `PrettyConfig::struct_types` setting, are not supported
- enabling the `#![enable(implicit_some)]` extension on a document with internally (or adjacently) tagged or untagged enums is not supported
- ron only supports string keys inside maps flattened into structs
- internally (or adjacently) tagged or untagged enum variants or `#[serde(flatten)]`ed fields must not contain:
- struct names, e.g. by enabling the `PrettyConfig::struct_names` setting
- newtypes
- zero-length arrays / tuples / tuple structs / structs / tuple variants / struct variants
- `Option`s with `#[enable(implicit_some)]` must not contain any of these or a unit, unit struct, or an untagged unit variant
- externally tagged tuple variants with just one field (that are not newtype variants)
- tuples or arrays with just one element are not supported inside newtype variants with `#[enable(unwrap_variant_newtypes)]`
- a `ron::value::RawValue`
- internally tagged newtype variants and `#[serde(flatten)]`ed fields must not contain:
- a unit or a unit struct inside an untagged newtype variant
- an untagged unit variant
- untagged tuple / struct variants with no fields are not supported
- untagged tuple variants with just one field (that are not newtype variants) are not supported when the `#![enable(unwrap_variant_newtypes)]` extension is enabled
- internally tagged newtype variants must not contain a unit / unit struct inside an untagged newtype variant, or an untagged unit variant
- serde does not yet support `i128` and `u128` inside internally (or adjacently) tagged or untagged enums
- newtypes and zero-length arrays / tuples / tuple structs / structs / tuple variants / struct variants are not supported inside internally (or adjacently) tagged or untagged enums
- externally tagged tuple variants with just one field (that are not newtype variants) are not supported inside internally (or adjacently) tagged or untagged enums
- serializing a `ron::value::RawValue` using a `PrettyConfig` may add leading and trailing whitespace and comments, which the `ron::value::RawValue` absorbs upon deserialization

Furthermore, serde imposes the following restrictions for data to roundtrip:

- structs or struct variants that contain a `#[serde(flatten)]`ed field:
- are only serialised as maps and deserialised from maps
- must not contain duplicate fields / keys, e.g. where an inner-struct field matches an outer-struct or inner-struct field
- must not contain more than one (within the super-struct of all flattened structs) `#[serde(flatten)]`ed map field, which collects all unknown fields
- if they contain a `#[serde(flatten)]`ed map, they must not contain:
- a struct that is not flattened itself but contains some flattened fields and is flattened into the outer struct (variant)
- an untagged struct variant that contains some flattened fields
- a flattened externally tagged newtype, tuple, or struct variant, flattened internally tagged unit, newtype, or struct variant, or any flattened adjacently tagged variant
- a flattened tagged struct
- internally (or adjacently) tagged or untagged enum variants or `#[serde(flatten)]`ed fields must not contain:
- `i128` or `u128` values

Please file a [new issue](https://github.com/ron-rs/ron/issues/new) if you come across a use case which is not listed among the above restrictions but still breaks.

Expand All @@ -203,7 +223,7 @@ While RON guarantees roundtrips like Rust -> RON -> Rust for Rust types using no

[^serde-flatten-hack]: Deserialising a flattened struct from a map requires that the struct's [`Visitor::expecting`](https://docs.rs/serde/latest/serde/de/trait.Visitor.html#tymethod.expecting) implementation formats a string starting with `"struct "`. This is the case for automatically-derived [`Deserialize`](https://docs.rs/serde/latest/serde/de/trait.Deserialize.html) impls on structs. See [#455](https://github.com/ron-rs/ron/pull/455) for more details.

[^serde-restrictions]: Most of these restrictions are currently blocked on [serde#1183](https://github.com/serde-rs/serde/issues/1183), which limits non-self-describing formats from roundtripping format-specific information through internally (or adjacently) tagged or untagged enums.
[^serde-restrictions]: Most of these restrictions are currently blocked on [serde#1183](https://github.com/serde-rs/serde/issues/1183), which limits non-self-describing formats from roundtripping format-specific information through internally (or adjacently) tagged or untagged enums or `#[serde(flatten)]`ed fields.

## License

Expand Down
2 changes: 2 additions & 0 deletions fuzz/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@ arbitrary = { version = "1.0", features = ["derive"] }
libfuzzer-sys = "0.4"
ron = { path = "..", features = ["integer128"] }
serde = { version = "1.0", features = ["derive"] }
erased-serde = { version = "0.3" }
anyhow = { version = "1.0" }
criterion = { version = "0.5" }
serde_path_to_error = { version = "0.1" }

# Prevent this from interfering with workspaces
[workspace]
Expand Down
16 changes: 16 additions & 0 deletions fuzz/fuzz_targets/arbitrary.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,19 @@
#![deny(clippy::correctness)]
#![deny(clippy::suspicious)]
#![deny(clippy::complexity)]
#![deny(clippy::perf)]
#![deny(clippy::style)]
#![warn(clippy::pedantic)]
#![deny(clippy::unwrap_used)]
#![deny(clippy::expect_used)]
#![allow(clippy::panic)]
#![deny(clippy::todo)]
#![deny(clippy::unimplemented)]
#![deny(clippy::unreachable)]
#![allow(unsafe_code)]
#![allow(clippy::match_same_arms)]
#![allow(clippy::too_many_lines)]
#![allow(clippy::items_after_statements)]
#![no_main]

use libfuzzer_sys::fuzz_target;
Expand Down
Loading

0 comments on commit 5f12b4a

Please sign in to comment.