quinn-rs / quinn

Async-friendly QUIC implementation in Rust
Apache License 2.0
3.76k stars 380 forks source link

Expose encoding and decoding of VarInt #1277

Closed flub closed 2 years ago

flub commented 2 years ago

The VarInt type is currently exposed, including VarInt::encoded_size. However there seems no public API to encode and decode it and I wanted to use it for my own wire-format inside QUIC streams.

A simple wrapper would probably already be sufficient:

impl VarInt {
    /// Parses a VarInt from the byte slice.
    pub fn parse_bytes(mut buf: &[u8]) -> coding::Result<VarInt> {
        Self::decode(&mut buf)
    }

    /// Writes the VarInt encoded as bytes.
    pub fn write_bytes<B: BufMut>(&self, w: &mut B) {
        self.encode(w)
    }
}

Could adding something like this be considered?

I also noticed that the quinn-proto::coding::Codec trait seems to be public-but-hidden. I assume we should not start depending on this either, but maybe making it public, at least for people directly depending on quinn-proto is also an alternative?

Ralith commented 2 years ago

I appreciate that VarInt is a generally useful type, but quinn isn't intended to provide tools for serializing application-layer information; we only expose VarInt at all because it's convenient for ensuring constructive correctness of various configuration fields that are inherently bound to its range. Given that it's fairly simple, I'd encourage you to just duplicate the implementation in your crate, which has much less stability risk.

Ralith commented 2 years ago

The unintended APIs have been removed from VarInt in #1278.

flub commented 2 years ago

sounds fair :)