Use base64 for serde_bytes annotated fields

[fengalin]

With the following struct, small will be represented as an array of u8 and large as a base64 String.

extern crate serde_derive;

extern crate serde;
extern crate serde_bytes;

#[derive(Serialize, Deserialize)]
struct BytesStruct {
    small: Vec<u8>,
    #[serde(with = "serde_bytes")]
    large: Vec<u8>,
}

[kvark]

I think this is a great improvement. Thank you for the PR!
We should bump the minor version of the crate for this is a breaking change, technically.
@torkleyy do you want to review as well?

[torkleyy]

Not very active currently. Let's bump the version and merge this :)

[fengalin]

I'm glad you find this PR interesting! Is there anything I could do to help push this forward?

[kvark]

@fengalin sorry about the wait. Please rebase in the meantime :)
@torkleyy must have just missed this. PING!

[kvark]

oh I see the response now, oops, sorry @torkleyy :)
@fengalin please bump the minor version and rebase

[fengalin]

@kvark no problem, I was just checking.
Rebased and bumped version.

[torkleyy]

One last concern, how can we recognize a bytes value in deserialize_any?

[fengalin]

@torkleyy I'm not quite sure about this. I think base64 handling should only be selected when user explicitly uses Bytes/_buf. I'll take a deeper look at this tomorrow.

[torkleyy]

The problem is that even though there's a `Value::Bytes` variant, it will never be the result of a deserialization.

…

On Fri, Jun 8, 2018, 11:00 François Laignel ***@***.***> wrote: @torjkleyy I'm not quite sure about this. I think base64 handling should only be selected when user explicitly uses Bytes/_buf. I'll take a deeper look at this tomorrow. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#109 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AWFFuT1J_fo7nj5hiLDaWhuYMUFPwG0pks5t6j0QgaJpZM4UVOFQ> .

[fengalin]

I marked the Value::Bytes variant as unreachable! in deserialize_any and implemented deserialize_bytes and deserialize_byte_buf. Does it address your concerns @torkleyy?

[torkleyy]

The problem is that this violates the following assumption:

(value -> serialization -> deserialization) == value

[fengalin]

AFAIU, you will get the same value if the origin and target struct are annotated with serde_bytes or use ByteBuf. Are you saying we should use a specific notation in the serialized field to show this is base64?

[torkleyy]

Yes, with fields this holds, but not with ron::Value.

[fengalin]

I think I got it:

• Removed Bytes from Value.
• base64 ser / de should only apply to serde_bytes annotated / typed fields now.

[torkleyy]

Sounds like a good solution to me! Thanks!

[torkleyy]

@kvark Do you still approve the PR with the new changes?

[kvark]

@fengalin thanks for your changes!
@torkleyy thanks for reviewing!
bors r=torkleyy

[bors]

Build succeeded

• continuous-integration/travis-ci/push