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

Simple API for prototyping #449

Closed
wants to merge 1 commit into from
Closed

Simple API for prototyping #449

wants to merge 1 commit into from

Conversation

Ralith
Copy link
Collaborator

@Ralith Ralith commented Sep 28, 2019

We've sought to expose an extremely flexible API, which is as a consequence somewhat complicated to get started with. This PR presents a straw proposal for an optional alternative API that's much friendlier and less versatile.

Drawbacks include:

  • No specialization for a particular application protocol
  • Valid certificates are even more strongly required than with the main API, since there's no way to manually trust one. Therefore prototyping a complete client-server system may actually be harder than with the main API, defeating the purpose.

Are there ways we can improve on those? If not, is this worth doing at all?

@djc
Copy link
Member

djc commented Sep 30, 2019

I like the direction of this, I've also been thinking about how we can "scale down" the minimum required API examples to get something useful.

Some stream-of-consciousness thoughts:

  • Not sure the feature carries its weight, as most applications relying on quinn will pull in the executor anyway -- it shouldn't be a problem. On the flip side, this makes the simple API less accessible. So this seems like a suboptimal trade-off.
  • BTW, also not completely sure about hiding the driver spawning. Have you looked at other tokio crates and checked if "internal" vs "external" spawning is more often used?
  • We need to consider the "cliff" between the simple API and the complex API. Ideally it's really easy to understand how to translate the simple API to the more powerful API.
  • So I think we should just add connect() and listen() methods to Endpoint, probably.
  • Did you actually test that these work? Since the client doesn't set any trust roots by default, I don't think this could actually work as is. In other words, I also think we should set default trust roots (in my recollection you removed the webpki-roots as default).

@Ralith
Copy link
Collaborator Author

Ralith commented Sep 30, 2019

On the flip side, this makes the simple API less accessible

A default feature means it's there unless you specifically opt out, so I'm not sure there's an accessibility cost. It is a pretty marginal dependency cost, though...

Have you looked at other tokio crates and checked if "internal" vs "external" spawning is more often used?

Haven't done a principled review. I think external is more correct in the general case because it ensures the caller doesn't have to jump through hoops to control executor use, but it's a minor footgun.

So I think we should just add connect() and listen() methods to Endpoint, probably.

Associated functions, you mean? connect of course already exists as a method, and listen is something you decide before constructing the endpoint. I do strongly agree that a more gracefully integrated "simple" API would be preferable to this draft. Having an associated function on Endpoint rather than a free function is only cosmetic, though.

Did you actually test that these work?

Nope! :D

In other words, I also think we should set default trust roots (in my recollection you removed the webpki-roots as default).

Default web trust roots are currently set in ClientConfigBuilder::default. On review they're not making it into EndpointBuilder::default, but I think that was an oversight. It's important to me that it be possible to operate without the web roots, but I agree that it's a fine default. Though #451 would be much preferable to what we're doing now.

@djc
Copy link
Member

djc commented Sep 30, 2019

I did mean associated functions. If we have those and default web trust roots, this would be pretty good. We could potentially iterate on internal vs external driver spawning from there.

Personally I'm always a bit on the fence about #451 (because I feel Mozilla is a better steward for my default trust roots than some of the OS vendors), but it probably makes sense for this.

@Ralith
Copy link
Collaborator Author

Ralith commented Sep 30, 2019

The big problem with webpki-roots is not that it's less trustworthy, but that it doesn't provide updates without updating the entire program; I think most projects can't be expected to distribute updates as frequently and reliably as Firefox does.

@djc
Copy link
Member

djc commented Oct 1, 2019

Ah, very good point.

@djc
Copy link
Member

djc commented Oct 1, 2019

Inspiration for simple API from quiche's README: https://github.com/cloudflare/quiche

@djc djc changed the base branch from master to main October 4, 2020 19:29
@Ralith
Copy link
Collaborator Author

Ralith commented Dec 17, 2023

Closing in favor of #1250.

@Ralith Ralith closed this Dec 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants