tls_codec: add variable-length integer type TlsVarInt#1656
tls_codec: add variable-length integer type TlsVarInt#1656franziskuskiefer merged 10 commits intoRustCrypto:masterfrom
Conversation
As defined in #[rfc9000]. Also use this type (with an internal thin wrapper `ContentLength`) when encoding/deconding the content length of vectors. [rfc9000]: https://www.rfc-editor.org/rfc/rfc9000#name-variable-length-integer-enc
|
This totally fell of my radar. Sorry for that. I'll look at this soon. |
franziskuskiefer
left a comment
There was a problem hiding this comment.
Can you say a little more about what this PR is supposed to achieve? I struggle a little with understanding what it tries to do beyond refactoring. The variable length encoding is really only internal to the TLS encoding.
The idea was to expose an additional type in the codec, to allow encoding/decoding of variable length integers. This is especially useful when prefixing small TLS encoded structs with such integers. However, personally I stopped using this type, so if there no need for it for the TLS codec users, we can also close this PR. WDYT? |
Thanks for clarifying. |
|
@boxdot do you want to address the comments? I'd be happy to get this in then (with public or non-public varint, if it's something that's useful sometimes, there's no harm in exposing it). |
I addressed review comments. We can do another review round. |
franziskuskiefer
left a comment
There was a problem hiding this comment.
Great, thanks!
One nit, and can you add a line to the CHANGELOG for the new public TlsVarInt type?
tls_codec/src/varint.rs
Outdated
| } | ||
|
|
||
| fn check_min_len(value: u64, len: usize) -> Result<(), Error> { | ||
| if cfg!(feature = "mls") { |
There was a problem hiding this comment.
This could be compile time, rather than a runtime check, or do I miss something?
|
@franziskuskiefer Thank you for the review! |
As defined in #rfc9000.
Also use this type (with an internal thin wrapper
ContentLength) whenencoding/deconding the content length of vectors.