micolous
commented
11 months ago The difficulty is going to be migration. The serialisable Credential struct includes a CredentialID (which is currently Base64UrlSafeData); so if you serialise to/from a binary format, you'll need to get everything updated to something that can deserialise both (which #354 added).
We could remove it from parts of our API which do not get serialised – that could happen after this.
The goal of this PR is to get this into a state that we could swap it out with a Vec<u8> or a HumanBinaryData type, and making sure that everything which actually needs Base64-related functionality (depending on it via FromStr or Display) instead explicitly encodes it as Base64. This is one of the sources of confusion about the Base64UrlSafeData type.
Firstyear
Goal is to remove all functionality from
Base64UrlSafeDatawhich is not about serialisation, and stop dependants from using internal-only interfaces.HumanBinaryDatawon't provide these interfaces, so it needs to make do.Make
Base64UrlSafeData.0(the innerVec<u8>) private, likeHumanBinaryData.This switches over everything which used the inner field to using
From,Derefor::new(). These interfaces are also available onHumanBinaryData, and makes the types pretty much interchangable.Remove
impl Display for Base64UrlSafeData(which converted the inner value into a Base64 string).The alternative is to use
base64::Engine::encode.Remove
impl TryFrom<&str> for Base64UrlSafeData(which attempted to parse strings as Base64).The alternative is to use
base64::Engine::decode.Migrate
WinWrapper::newto use owned types rather than pass-by-reference.Most of the existing implementations made copies of that data, and callers don't need to clone anyway.
TODO:
363
[ ] cargo test has been run and passes
[ ] documentation has been updated with relevant examples (if relevant)