Add support for `[u8; N]` by sgued · Pull Request #28 · serde-rs/bytes

sgued · 2022-03-24T23:09:23Z

Hi.

This PR adds support for constant length byte arrays [u8; N] and adds a ByteArray<const N: usize> wrapper type similar to BytesBuf. This would close #26.

Motivations

Fixed sized array are often used for cryptography and embedded systems. Having efficient support to serialize/deserialize keys would be great.

Parts of the PR requiring discussion

I'm not sure why ByteBuf and Bytes implements PartialEq and PartialOrd for types implementing AsRef<[u8]> and not Borrow<[u8]>. ByteArray implements PartialEq/Ord with types implementing Borrow<[u8; N]> so that it can be compared to itself and to [u8; N]

It might be worth considering adding a new_mut(&mut [u8]) -> Bytes function to Bytes so that the same (unsafe) code is reused for the BorrowMut<Bytes> implementations of ByteArray and ByteBuf.

Drawbacks

Due to the use of const generics, this would require raising the MSRV to 1.53 (which could be lowered to 1.51 by removing the IntoIterator implementation).

Edit: Since 0504fcb already bumped MSRV to 1.56, this drawback doesn't apply anymore.

Xiretza · 2022-05-27T07:55:33Z

I'd love to see this added, would it be possible for a maintainer to have a look at it?

src/bytearray.rs

kamulos · 2022-08-08T11:16:49Z

I am also really interested in this. Any chance you could have a look @dtolnay ?

src/bytearray.rs

Cargo.toml

martin-g

LGTM!

It is based on serde-rs/bytes#28 which is not yet merged. Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

elichai · 2022-10-19T17:53:21Z

Note that unlike serde serde_bytes doesn't have the impl Serialize for [T; 0] problem

src/bytearray.rs

…uid. Squashed commit of the following: commit 3a1fac3 Author: Martin Tzvetanov Grigorov <mgrigorov@apache.org> Date: Fri Oct 7 10:48:13 2022 +0300 AVRO-3631: Use official serde_bytes crate Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org> commit f7f0ec5 Author: Martin Tzvetanov Grigorov <mgrigorov@apache.org> Date: Thu Oct 6 14:01:19 2022 +0300 AVRO-3631: Fix clippy issues Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org> commit 0ee5b00 Author: Martin Tzvetanov Grigorov <mgrigorov@apache.org> Date: Thu Oct 6 13:38:10 2022 +0300 AVRO-3631: Add more test cases Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org> commit 70d6cf0 Author: Martin Tzvetanov Grigorov <mgrigorov@apache.org> Date: Thu Oct 6 12:05:08 2022 +0300 AVRO-3631: Fix clippy and Rat issues Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org> commit 7c9e938 Author: Martin Tzvetanov Grigorov <mgrigorov@apache.org> Date: Thu Oct 6 11:45:50 2022 +0300 AVRO-3631: Add serde serialize_with functions Those should be used for hinting the serialization process how to serialize a byte array to Value::(Bytes|Fixed) Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org> commit 3e3f125 Author: Martin Tzvetanov Grigorov <mgrigorov@apache.org> Date: Mon Oct 3 23:45:11 2022 +0300 AVRO-3631: Use #[serde(with)] attribute to get rid of implementation detail ByteArray Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org> commit 7b1cdd8 Author: Martin Tzvetanov Grigorov <mgrigorov@apache.org> Date: Mon Oct 3 14:52:24 2022 +0300 AVRO-3631: Add support for ser_de Value::Fixed It is based on serde-rs/bytes#28 which is not yet merged. Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org> commit 5c2d197 Author: Rik Heijdens <r.heijdens@lithic.com> Date: Thu Sep 29 14:15:49 2022 +0200 AVRO-3631: Reformat using rustfmt commit a31fcfc Author: Rik Heijdens <r.heijdens@lithic.com> Date: Thu Sep 29 14:14:35 2022 +0200 AVRO-3631: Add test for serializing fixed fields This test-case mainly demonstrates the issue reported in AVRO-3631. It is unclear to me whether we should actually expect the serializer to serialize to a Value::Fixed right away given that Schema information is not available at this stage. commit 12ef14b Author: Rik Heijdens <r.heijdens@lithic.com> Date: Thu Sep 29 14:12:51 2022 +0200 AVRO-3651: Add test to de.rs to illustrate issue with Fixed fields commit d32d436 Author: Rik Heijdens <r.heijdens@lithic.com> Date: Wed Sep 28 15:48:29 2022 +0200 AVRO-3631: Add test-case to reproduce

markfarnan · 2023-01-04T20:49:52Z

This is blocking downstream feature in Avro/Rust for Fixed Types.
I've tested it, and it works fine for my use case with Avro.

Can this be merged soon please ?

emchristiansen · 2023-02-05T01:41:15Z

Haha, I created a pull request with almost exactly the same name, not realizing that this existed (I'll delete it now).
But, can we please get this merged in?

It is based on serde-rs/bytes#28 which is not yet merged. Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

sosthene-nitrokey · 2023-02-24T09:57:11Z

Given the lack of activity in this PR we published a similar crate specifically for [u8; N] as serde-byte-array

jonasbb · 2023-02-24T11:39:46Z

The serde_with::Bytes type supports const generic arrays in a bunch of ways.

It is based on serde-rs/bytes#28 which is not yet merged. Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

emchristiansen · 2023-10-11T20:38:07Z

Does anyone know why this is taking so long?
I.e., should I wait for a fix or should I switch to a different crate?

so-schen · 2023-12-10T14:43:05Z

in case anyone looking for drop-in replacement with all bytes/slice support in one crate, can use this https://github.com/so-schen/serde-bytes-ng

dtolnay

Thank you! This looks great, especially with Lucretiel great suggestions.

It is based on serde-rs/bytes#28 which is not yet merged. Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

Xiretza reviewed May 27, 2022

View reviewed changes

src/bytearray.rs Outdated Show resolved Hide resolved

sgued force-pushed the byte-array branch 2 times, most recently from e3a2120 to 846ccba Compare July 16, 2022 12:53

martin-g reviewed Aug 30, 2022

View reviewed changes

src/bytearray.rs Outdated Show resolved Hide resolved

src/bytearray.rs Outdated Show resolved Hide resolved

martin-g reviewed Aug 30, 2022

View reviewed changes

Cargo.toml Show resolved Hide resolved

sgued force-pushed the byte-array branch 2 times, most recently from 7b125bc to 1bdc428 Compare August 30, 2022 10:53

martin-g reviewed Aug 30, 2022

View reviewed changes

martin-g added a commit to apache/avro that referenced this pull request Oct 3, 2022

AVRO-3631: Add support for ser_de Value::Fixed

7b1cdd8

It is based on serde-rs/bytes#28 which is not yet merged. Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

martin-g mentioned this pull request Oct 3, 2022

AVRO-3631: SerDe support for Value::Fixed(). apache/avro#1893

Closed

Lucretiel reviewed Oct 27, 2022

View reviewed changes

src/bytearray.rs Outdated Show resolved Hide resolved

Lucretiel reviewed Oct 27, 2022

View reviewed changes

src/bytearray.rs Outdated Show resolved Hide resolved

sgued force-pushed the byte-array branch from c688fdc to 3a758ee Compare October 29, 2022 08:02

Lucretiel reviewed Nov 2, 2022

View reviewed changes

src/bytearray.rs Outdated Show resolved Hide resolved

src/bytearray.rs Outdated Show resolved Hide resolved

src/bytearray.rs Outdated Show resolved Hide resolved

Lucretiel reviewed Nov 2, 2022

View reviewed changes

src/bytearray.rs Outdated Show resolved Hide resolved

sgued force-pushed the byte-array branch from 167c622 to 8fcf21f Compare December 16, 2022 23:05

ryoqun mentioned this pull request Dec 19, 2022

Add fully-reproducible online tracer for banking solana-labs/solana#29196

Merged

11 tasks

martin-g added a commit to apache/avro that referenced this pull request Feb 9, 2023

AVRO-3631: Add support for ser_de Value::Fixed

d674db9

It is based on serde-rs/bytes#28 which is not yet merged. Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

sgued force-pushed the byte-array branch from 8fcf21f to e903fbd Compare February 17, 2023 13:04

sosthene-nitrokey mentioned this pull request Feb 22, 2023

Improve byte array serialization trussed-dev/trussed-auth#12

Merged

martin-g added a commit to apache/avro that referenced this pull request Mar 13, 2023

AVRO-3631: Add support for ser_de Value::Fixed

e9b7853

It is based on serde-rs/bytes#28 which is not yet merged. Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

sgued force-pushed the byte-array branch from e903fbd to 7761be6 Compare March 18, 2023 21:20

martin-g added a commit to apache/avro that referenced this pull request Mar 24, 2023

AVRO-3631: Add support for ser_de Value::Fixed

9ab5358

It is based on serde-rs/bytes#28 which is not yet merged. Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

sgued force-pushed the byte-array branch from 7761be6 to 2eb600d Compare August 28, 2023 14:04

sgued added 2 commits September 28, 2023 10:34

Add byte array type

7ffee6f

Add tests for ByteArray

95d9f83

sgued force-pushed the byte-array branch from 2eb600d to 95d9f83 Compare September 28, 2023 08:35

dtolnay approved these changes Dec 27, 2023

View reviewed changes

dtolnay merged commit 5da9904 into serde-rs:master Dec 27, 2023

sgued mentioned this pull request Dec 27, 2023

serde-bytes has merged support for [u8; N] so-schen/serde-bytes-ng#1

Open

sosthene-nitrokey mentioned this pull request Dec 27, 2023

Mark as deprecated Nitrokey/serde-byte-array#7

Open

martin-g added a commit to apache/avro that referenced this pull request Apr 9, 2024

AVRO-3631: Add support for ser_de Value::Fixed

c999938

It is based on serde-rs/bytes#28 which is not yet merged. Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

martin-g added a commit to apache/avro that referenced this pull request Jul 17, 2024

AVRO-3631: Add support for ser_de Value::Fixed

172fb85

It is based on serde-rs/bytes#28 which is not yet merged. Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

Conversation

sgued commented Mar 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivations

Parts of the PR requiring discussion

Drawbacks

Uh oh!

Xiretza commented May 27, 2022

Uh oh!

Uh oh!

kamulos commented Aug 8, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

martin-g left a comment

Choose a reason for hiding this comment

Uh oh!

elichai commented Oct 19, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

markfarnan commented Jan 4, 2023

Uh oh!

emchristiansen commented Feb 5, 2023

Uh oh!

sosthene-nitrokey commented Feb 24, 2023

Uh oh!

jonasbb commented Feb 24, 2023

Uh oh!

emchristiansen commented Oct 11, 2023

Uh oh!

so-schen commented Dec 10, 2023

Uh oh!

dtolnay left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

12 participants

sgued commented Mar 24, 2022 •

edited

Loading