Allow unknown CBOR fields (ref #66)#110
Merged
Merged
Conversation
2 tasks
Collaborator
msirringhaus
left a comment
There was a problem hiding this comment.
Added some thoughts
cdf44b4 to
bc946b7
Compare
d01862e to
6273416
Compare
6273416 to
c4ba62c
Compare
msirringhaus
approved these changes
Jun 16, 2025
Collaborator
msirringhaus
left a comment
There was a problem hiding this comment.
Looking good to me! I added two small comments, but good to go as is.
| map.insert(1, 10u8); | ||
| map.insert(2, 20u8); | ||
|
|
||
| let cbor = to_vec(&map).unwrap(); |
Collaborator
There was a problem hiding this comment.
Not necessarily for this PR, but I wonder if we could have additionally actual raw (cbor) bytes in these tests to compare the outputs to.
| fn eq(&self, other: &Self) -> bool { | ||
| match (self, other) { | ||
| (CborError::SerdeCbor(e1), CborError::SerdeCbor(e2)) => { | ||
| e1.to_string() == e2.to_string() |
Collaborator
There was a problem hiding this comment.
Maybe we could implement PartialEq in serde_cbor for this, because this is really ugly :D
But obviously, not for this PR.
AlfioEmanueleFresta
added a commit
that referenced
this pull request
Jul 21, 2025
## 1. Tungstenite now requiring selecting a crypto provider A panic at runtime when attempting to establish a secure WebSocket connection: ``` DEBUG tokio_tungstenite::tls::encryption::rustls: Added 148/148 native root certificates (ignored 0) thread 'main' panicked at /home/afresta/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/rustls-0.23.27/src/crypto/mod.rs:249:14: no process-level CryptoProvider available -- call CryptoProvider::install_default() before this point note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace ``` For more details, see: * snapview/tokio-tungstenite#336 * snapview/tokio-tungstenite#353 This is now fixed by explicitly selecting `rustls` as the `tungstenite` TLS backend, as recommended in `tokio-tungstenite`. ## 2. Hybrid initial GetInfo deserialization fails An error deserializing the caBLE initial message containing the GetInfo response: ``` DEBUG libwebauthn::transport::cable::tunnel: Responding to GetInfo request with cached response ERROR webauthn_make_credential{dev=CableChannel}:ctap2_get_info: libwebauthn::proto::ctap2::protocol: Failed to parse Ctap2GetInfoResponse from CBOR-data provided by the device. Parsing error: SerdeCbor(ErrorImpl { code: Message("invalid type: byte array, expected Ctap2GetInfoResponse"), offset: 0 }) thread 'main' panicked at libwebauthn/examples/webauthn_cable.rs:139:10: called `Result::unwrap()` on an `Err` value: Platform(InvalidDeviceResponse) note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace ``` This was accidentally introduced in #110: https://github.com/linux-credentials/libwebauthn/pull/110/files#diff-d061af4011a3c577c5c2dbef3b1933b0e8c8a9ebc0bf70a0ebc3358e890ed725R587 Fixed by using the appropriate serialization method.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Depends on trussed-dev/serde-indexed#19.