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

Change most of the Deserializer to use forward_to_deserialize_any #46

Merged
merged 5 commits into from
Oct 3, 2023

Conversation

Boog900
Copy link
Member

@Boog900 Boog900 commented Jun 23, 2023

fixes #27

This also changes my old commit fixing structs with embedded structs to use a different method. A bool read_header instead of putting a struct marker at the front of the array of data.

This also fixes the panics on incomplete structs, just forwarding to forward_to_deserialize_any.

I could have removed more of the de-serialize but I feel the explicit errors on unsupported types may be helpful.

I have also changed how the Deserializer handles string markers, this is because epee marks byte arrays as strings so if someone sends a bytes array in a field we don't have this will get forwarded to deserialize_any which will then fail if the byte array isn't an actual string.

@thomaseizinger

Copy link
Contributor

@thomaseizinger thomaseizinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Two comments but overall ACK.

src/de.rs Outdated Show resolved Hide resolved
src/de.rs Outdated Show resolved Hide resolved
@Boog900
Copy link
Member Author

Boog900 commented Jun 27, 2023

I have opened another PR for changing how embedded structs are handled

@Boog900 Boog900 requested a review from thomaseizinger June 27, 2023 12:47
This also changes my old commit fixing structs with embedded structs to use a different method. (A bool `read_header` instead of putting a struct marker at the front of the array of data.

This also fixes the panics on incomplete structs.
@Boog900 Boog900 merged commit ca84da5 into monero-rs:main Oct 3, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Crate panics on incomplete struct deserialization
2 participants