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

serializer: force serialization as array #546

Merged
merged 1 commit into from
Jun 28, 2024

Conversation

baloo
Copy link
Contributor

@baloo baloo commented Jun 26, 2024

The implementation of array deserializer expects the payload to be serialized as array. Sadly the serializer left the door open for the underlying serializer to choice either bytes or array, if available.

This would only occur when the objects are used outside yubihsm.rs.

This change was tested on both mockhsm and yubihsm on usb.

@baloo baloo changed the title serializer: enforce serialization as array serializer: force serialization as array Jun 27, 2024
@baloo baloo force-pushed the baloo/serializer-fixup branch 2 times, most recently from 2594629 to 8851a9d Compare June 28, 2024 03:29
The implementation of array deserializer expects the payload to be
serialized as array. Sadly the serializer left the door open for the
underlying serializer to choice either bytes or array, if available.

This would only occur when the objects are used outside yubihsm.rs.

This change was tested on both mockhsm and yubihsm on usb.
@tony-iqlusion
Copy link
Member

What concrete problem is the current implementation causing? It's an issue when using these serializers with some other serialization format?

@baloo
Copy link
Contributor Author

baloo commented Jun 28, 2024

I'm reusing the yubihsm::authentication::key::Key object for a project, and I'm sending it over the wire.
Sadly the Deserialize implementation (located further down) expects an array, while the Serialize doesn't specify the format.

Because we don't specify the serialization (whether we want array or bytes), this leaves the choice open to the serializer implementation used. This works out here because our serializer implementation defaults to array, but the other serializer I'm using (postcard) uses another default (I believe it uses bytes)

This change keeps both implementations of serde::Serialize and serde::Deserialize consistent.

@tony-iqlusion tony-iqlusion merged commit b5ae19f into iqlusioninc:main Jun 28, 2024
6 checks passed
@baloo baloo deleted the baloo/serializer-fixup branch June 28, 2024 17:53
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

Successfully merging this pull request may close these issues.

2 participants