Skip to content

Implement is_human_readable to return false#5

Merged
KizzyCode merged 1 commit intoKizzyCode:masterfrom
CBenoit:master
Mar 7, 2023
Merged

Implement is_human_readable to return false#5
KizzyCode merged 1 commit intoKizzyCode:masterfrom
CBenoit:master

Conversation

@CBenoit
Copy link
Copy Markdown
Contributor

@CBenoit CBenoit commented Mar 6, 2023

Currently, is_human_readable is not manually implemented and thus returns true (default). As a consequence, types supporting a more compact binary representation are serialized inefficiently (typically into a string representation).

This is a breaking change: previously serialized types supporting a compact representation will fail at deserialization with this commit.

This is an issue preventing adoption by some projects.

Currently, [is_human_readable](https://docs.rs/serde/latest/serde/trait.Serializer.html#method.is_human_readable)
is not manually implemented and thus returns `true` (default).
As a consequence, types supporting a more compact binary representation
are serialized inefficiently (typically into a string representation).

This is a breaking change: previously serialized types supporting a
compact representation will fail at deserialization with this commit.
@KizzyCode
Copy link
Copy Markdown
Owner

KizzyCode commented Mar 6, 2023

I see. As far as I understand, this is only an issue for formats that use multiple intermediate representations during serialization: T → serde_asn1_der supported native type → DER?

E.g. a date type which could either use:

  • Date <=> Human-Readable String <=> DER UTF-8 String
  • Date <=> Compact String <=> DER UTF-8 String

The intermediate representation is selected by is_human_readable, and if we change this, another intermediate representation and subsequently a different (de-)serialization chain is selected which could break compatibility?

@CBenoit
Copy link
Copy Markdown
Contributor Author

CBenoit commented Mar 7, 2023

I see. As far as I understand, this is only an issue for formats that use multiple intermediate representations during serialization: T → serde_asn1_der supported native type → DER?

E.g. a date type which could either use:

* `Date <=> Human-Readable String <=> DER UTF-8 String`

* `Date <=> Compact String <=> DER UTF-8 String`

The intermediate representation is selected by is_human_readable, and if we change this, another intermediate representation and subsequently a different (de-)serialization chain is selected which could break compatibility?

Correct.

It depends on the Serialize and Deserialize implementation of the type itself. See for instance this Serialize implementation for an IPv4 address type.
An IPv4 address takes 4 bytes to encode in non human readable binary format, but between 7 and 15 bytes when encoded as characters (x.x.x.x - xxx.xxx.xxx.xxx).

This might or might not cause compatibility issues depending on whether downstream user used such a type. The library version number should be bumped appropriately.

KizzyCode added a commit that referenced this pull request Mar 7, 2023
KizzyCode added a commit that referenced this pull request Mar 7, 2023
@KizzyCode KizzyCode merged commit eb6d1f4 into KizzyCode:master Mar 7, 2023
@KizzyCode
Copy link
Copy Markdown
Owner

Merged and published. Thank you very much! 😊

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