The custom version of the api is weird

[Dreamacro]

Hi, I recently used quinn-proto to replace rustls with noise protocol and successfully implemented PoC with Noise_IKpsk2_25519_ChaChaPoly_BLAKE2b. But when I set up a custom version, I meet a problem.

Before I started setting up the custom version, I had the following test code

let bind_addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::LOCALHOST), 0);
let quinn_s_cfg = quinn::ServerConfig::with_crypto(Arc::new(s_cfg));
let quinn_c_cfg = quinn::ClientConfig::new(Arc::new(c_cfg));

let s_endpoint = quinn::Endpoint::server(quinn_s_cfg, bind_addr).unwrap();
let s_addr = s_endpoint.local_addr().unwrap();

let handle = task::spawn(async move {
    let mut c_endpoint = quinn::Endpoint::client(bind_addr).unwrap();
    c_endpoint.set_default_client_config(quinn_c_cfg);

    let conn = c_endpoint.connect(s_addr, "").unwrap().await.unwrap();

    let (mut send, mut recv) = conn.open_bi().await.unwrap();
    send.write_all(b"ping").await.unwrap();

    let mut buf = [0u8; 4];
    recv.read_exact(&mut buf).await.unwrap();
    assert_eq!(&buf[..4], b"pong");
});

let conn = s_endpoint.accept().await.unwrap().await.unwrap();
let (mut send, mut recv) = conn.accept_bi().await.unwrap();

let mut buf = [0u8; 4];
recv.read_exact(&mut buf).await.unwrap();
assert_eq!(&buf[..4], b"ping");

send.write_all(b"pong").await.unwrap();

handle.await.unwrap();

After a quick scan of the doc, I modified some of the code.

Set the default version for ClientConfig and the supported version list for ServerConfig's Endpoint.

let mut quinn_c_cfg = quinn::ClientConfig::new(Arc::new(c_cfg));
quinn_c_cfg.version(DEFAULT_QUIC_VERSION);

let mut endpoint_cfg = EndpointConfig::default();
endpoint_cfg.supported_versions(vec![DEFAULT_QUIC_VERSION]);
let s_endpoint = Endpoint::new(
    endpoint_cfg,
    Some(quinn_s_cfg),
    UdpSocket::bind(bind_addr).unwrap(),
    TokioRuntime,
)
.unwrap();

At this point, the test failed. When I used RUST_LOG=trace, I saw that the relevant packets were being dropped on the client side.

DEBUG quinn_proto::endpoint: dropping packet with unsupported version

Then I found it:

https://github.com/quinn-rs/quinn/blob/e5a5d3eeb80f637e6bb55730456efc452a9ac9d2/quinn-proto/src/endpoint.rs#L165-L168

It checks the server_config on the client side, but I explicitly set a specific version in the client_config.

The workaround is to add a server_config to the client as well, but the client doesn't have a real server private key, it has to mock an server_config.

[Dreamacro]

Update, no need for server_config, but still need to manually set the endpoint

let mut endpoint_cfg = EndpointConfig::default();
endpoint_cfg.supported_versions(vec![DEFAULT_QUIC_VERSION]);

let bind_addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::LOCALHOST), 0);
let mut endpoint = Endpoint::new(
    endpoint_cfg,
    None,
    UdpSocket::bind(bind_addr)?,
    TokioRuntime,
)?;
endpoint.set_default_client_config(self.client_cfg.clone());

[Dreamacro]

Although my problem has been solved, it would be better if the api could be more friendly.

[djc]

Can you articulate in brief what you found unfriendly about the API? After reading through your comments here I'm not sure I understand what the issue was -- but if there is a way we can make the API easier to use, I'd like to know!

[Ralith]

My understanding is that they set the (supported) version(s) to a nonstandard value on the client connection and server endpoint, but not the client endpoint. We should probably generate a ConnectError if you initiate a connection with a version that is not supported by the local endpoint.

[Dreamacro]

What bothered me was that since ClientConfig has the version method to set the QUIC version, why does this version need to be in the default list (i.e. supported_versions)? When I looked up the code, I realized how it worked.

https://github.com/quinn-rs/quinn/blob/e5a5d3eeb80f637e6bb55730456efc452a9ac9d2/quinn-proto/src/config.rs#L611-L616

If this is part of the design, It would be better to add some comments to version.