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

Add "HumanBinaryData" as an alternative to "Base64UrlSafeData" #354

Merged
merged 8 commits into from
Oct 2, 2023

Conversation

micolous
Copy link
Collaborator

@micolous micolous commented Sep 29, 2023

Related to issue #352, it doesn't fix things, but it's a start.

HumanBinaryData uses a more efficient "bytes" format when using non-human-readable serialisation formats (like CBOR). The deserialiser still uses deserialize_any – I haven't got a great way around this yet.

I've done a few things differently to Base64UrlSafeData; for example, this doesn't implement Display or FromStr – they're different to what Vec<u8> does, and we should move away from this (this will be the focus of #356).

It also implements Deref and DerefMut, which should make the type more transparent – and all of the common impls are in macros.

I've added bytes support to Base64UrlSafeData, so it can read what HumanBinaryData produces. Unfortunately this makes migration a two-step process.

While we're here, this makes serde_json a dev-dependency of base64urlsafedata, because it's only used in tests; and this also adds a bunch more tests for expected input formats.

  • cargo test has been run and passes
  • documentation has been updated with relevant examples (if relevant)

@micolous
Copy link
Collaborator Author

micolous commented Sep 29, 2023

Clippy appears to fail due to checks in 1.70.0, possibly triggered by #348. #355 should fix that.

@micolous micolous force-pushed the human branch 3 times, most recently from b26169b to 1d7d75d Compare September 29, 2023 06:30
Copy link
Member

@Firstyear Firstyear left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a great idea and a great way to approach this. Great work @micolous <3

@micolous micolous merged commit c20e83f into kanidm:master Oct 2, 2023
kikuomax pushed a commit to codemonger-io/webauthn-rs that referenced this pull request Nov 24, 2024
…m#354)

* Add "HumanBinaryData" as alternative to "Base64UrlSafeData" (kanidm#352)

* Add bytes support to Base64UrlSafeData, and copy across the tests

* Rework the docs

* dedupe functionality into macros, make tests consistent

* more tests and conversions

* move Borrow impl into common

* add some more vec features

* fix clippy
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