bincode-org / bincode

A binary encoder / decoder implementation in Rust.
MIT License
2.69k stars 272 forks source link

Made arrays never encode their length #625

Closed VictorKoenders closed 1 year ago

VictorKoenders commented 1 year ago

Closes #613

Turns out serde never encodes a fixed array length with 32 elements or less. If an array is 33 elements or more, it silently falls back to the &[T] implementation (which does encode the length). Our test cases were only tested with large arrays.

Because the config flags write_fixed_array_length and skip_fixed_array_length were based on a wrong assumption on how serde/bincode 1 worked, these flags have been removed. They may be re-added in the future.

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 84.61% and project coverage change: +0.48 :tada:

Comparison is base (ebc3b6b) 53.89% compared to head (c9fd18d) 54.37%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## trunk #625 +/- ## ========================================== + Coverage 53.89% 54.37% +0.48% ========================================== Files 51 51 Lines 4457 4454 -3 ========================================== + Hits 2402 2422 +20 + Misses 2055 2032 -23 ``` | [Impacted Files](https://codecov.io/gh/bincode-org/bincode/pull/625?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bincode-org) | Coverage Δ | | |---|---|---| | [src/de/impls.rs](https://codecov.io/gh/bincode-org/bincode/pull/625?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bincode-org#diff-c3JjL2RlL2ltcGxzLnJz) | `59.77% <ø> (+1.53%)` | :arrow_up: | | [src/enc/impls.rs](https://codecov.io/gh/bincode-org/bincode/pull/625?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bincode-org#diff-c3JjL2VuYy9pbXBscy5ycw==) | `88.77% <ø> (-0.12%)` | :arrow_down: | | [src/features/serde/de\_borrowed.rs](https://codecov.io/gh/bincode-org/bincode/pull/625?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bincode-org#diff-c3JjL2ZlYXR1cmVzL3NlcmRlL2RlX2JvcnJvd2VkLnJz) | `20.61% <ø> (-0.18%)` | :arrow_down: | | [src/features/serde/de\_owned.rs](https://codecov.io/gh/bincode-org/bincode/pull/625?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bincode-org#diff-c3JjL2ZlYXR1cmVzL3NlcmRlL2RlX293bmVkLnJz) | `76.61% <ø> (+1.99%)` | :arrow_up: | | [src/features/serde/mod.rs](https://codecov.io/gh/bincode-org/bincode/pull/625?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bincode-org#diff-c3JjL2ZlYXR1cmVzL3NlcmRlL21vZC5ycw==) | `30.43% <ø> (-17.40%)` | :arrow_down: | | [src/features/serde/ser.rs](https://codecov.io/gh/bincode-org/bincode/pull/625?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bincode-org#diff-c3JjL2ZlYXR1cmVzL3NlcmRlL3Nlci5ycw==) | `76.72% <ø> (+2.53%)` | :arrow_up: | | [compatibility/src/lib.rs](https://codecov.io/gh/bincode-org/bincode/pull/625?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bincode-org#diff-Y29tcGF0aWJpbGl0eS9zcmMvbGliLnJz) | `50.00% <40.00%> (-1.79%)` | :arrow_down: | | [tests/utils.rs](https://codecov.io/gh/bincode-org/bincode/pull/625?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bincode-org#diff-dGVzdHMvdXRpbHMucnM=) | `47.36% <52.94%> (+2.13%)` | :arrow_up: | | [src/config.rs](https://codecov.io/gh/bincode-org/bincode/pull/625?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bincode-org#diff-c3JjL2NvbmZpZy5ycw==) | `78.94% <87.50%> (-3.67%)` | :arrow_down: | | [compatibility/src/misc.rs](https://codecov.io/gh/bincode-org/bincode/pull/625?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bincode-org#diff-Y29tcGF0aWJpbGl0eS9zcmMvbWlzYy5ycw==) | `100.00% <100.00%> (ø)` | | | ... and [3 more](https://codecov.io/gh/bincode-org/bincode/pull/625?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bincode-org) | | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bincode-org). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bincode-org)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

TommyLike commented 1 year ago

@VictorKoenders hello, I use bincode in my project as below:

 signed.write_all(&bincode::encode_to_vec(
            &sig_struct,
            config::standard()
                .skip_fixed_array_length()
                .with_fixed_int_encoding()
                .with_big_endian(),
        )?)?;

it works well for the version of bincode = "2.0.0-rc.2" but when upgrade into rc.3 I found the skip_fixed_array_length has been removed, so what should I do for now, How to remove the length when serializing?

VictorKoenders commented 1 year ago

@TommyLike currently there is no such option. Bincode 1 and 2 should work as you'd expect without the configuration. Is there a particular issue you're running into?

TommyLike commented 1 year ago

@TommyLike currently there is no such option. Bincode 1 and 2 should work as you'd expect without the configuration. Is there a particular issue you're running into?

No, I realized it works as I expected, thanks!