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

[serde-ser] Remove validation of expected length #42

Open
Marcono1234 opened this issue Jan 29, 2024 · 1 comment
Open

[serde-ser] Remove validation of expected length #42

Marcono1234 opened this issue Jan 29, 2024 · 1 comment
Labels
enhancement New feature or request feature:serde Issue or pull related to Serde integration

Comments

@Marcono1234
Copy link
Owner

Problem solved by the enhancement

Multiple Serde Serializer methods take a len argument, for example Serializer::serialize_seq. Serde JSON does not seem to actually validate this argument, it just checks it to special-case empty containers, see e.g. serialize_seq for writing to Write or serialize_seq for creating a Value.

Struson currently validates the length and returns a SerializerError::IncorrectElementsCount on mismatch. The intention was to help users detect incorrect handwritten Serialize implementations. However, this validation causes the following issues:

Enhancement description

Remove the validation of the expected length, and remove SerializerError::IncorrectElementsCount

Alternatives / workarounds

Fix the bugs in the current length checks, and verify that it behaves correctly for derived Serialize

@Marcono1234 Marcono1234 added the enhancement New feature or request label Jan 29, 2024
@Marcono1234 Marcono1234 added the feature:serde Issue or pull related to Serde integration label Jan 30, 2024
Marcono1234 added a commit that referenced this issue Jan 30, 2024
@Marcono1234
Copy link
Owner Author

Have implemented this experimentally in the serde-ser-remove-length-validation branch, however for now I will not merge this since the validation helps with detecting bad Serialize implementations (assuming the validation is correct).

In case the validation turns out to be incorrect or too strict, I might consider merging this in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request feature:serde Issue or pull related to Serde integration
Projects
None yet
Development

When branches are created from issues, their pull requests are automatically linked.

1 participant