Allow the server_name_resolver to be overriden

[0xIO32]

This adds the

with_server_name_resolver(mut self, server_name_resolver: Arc<dyn hyper_rustls::ResolveServerName + Send + Sync>)

to ClientBuilder (if rustls is used).

It allows to override, which name is used for tls instead of using the one from the url.

This pr relys on rustls/hyper-rustls#301 being merged.

Also this hasn't been fully tested yet, I will remove this disclaimer after it has been tested.

[seanmonstar]

Thanks for contributing!

I notice this makes the dependency on hyper-rustls public. We try to avoid that, so we can increase versions internally without affecting the public API.

But a more general SNI mechanism could be explored.

[0xIO32]

Thanks for contributing!

I notice this makes the dependency on hyper-rustls public. We try to avoid that, so we can increase versions internally without affecting the public API.

But a more general SNI mechanism could be explored.

Would you prefer it, to have an own trait, which then gets converted to hyper-rustls?

[0xIO32]

Thanks for contributing!

I notice this makes the dependency on hyper-rustls public. We try to avoid that, so we can increase versions internally without affecting the public API.

But a more general SNI mechanism could be explored.

On the second time reading, do you mean to not export the "ResolveServerName" trait and instead let the user add the crate himself?

[0xIO32]

@seanmonstar Could you comment on rustls/hyper-rustls#301 ?

Tldr: This pr first needs a change from hyper-rustls (which I implemented in the pr linked above). The question now is, would this pr get merged in the first place? (if not, what needs to be changed, or is this something that will never be a feature in reqwest?)

[seanmonstar]

I don't have a strong opinion on how any such API would look in hyper-rustls.

But what I mean is designing a possible generic way to define server name resolution might be fine to add, but not something that publicly exposed the dependency. That might mean using the PR you mentioned internally, I cannot say. But it will require a bit of research and design iteration to come up with an API we can try to keep stable.

[0xIO32]

I don't have a strong opinion on how any such API would look in hyper-rustls.

But what I mean is designing a possible generic way to define server name resolution might be fine to add, but not something that publicly exposed the dependency. That might mean using the PR you mentioned internally, I cannot say. But it will require a bit of research and design iteration to come up with an API we can try to keep stable.

Did this change, given that rustls is now the default in reqwest? I don't think making a generic wrapper of each type is a better way of doing it. Notably https://docs.rs/rustls-pki-types/1.12.0/rustls_pki_types/enum.ServerName.html and https://docs.rs/rustls-pki-types/1.12.0/rustls_pki_types/struct.DnsName.html.

[seanmonstar]

No, the constraint did not change. We still do not want to expose the dependencies publicly. Still for two reasons:

• We want to be able to upgrade TLS libraries without affecting our public API (breaking changes).
• A significant amount of people still need native-tls support for whatever reason.

[0xIO32]

No, the constraint did not change. We still do not want to expose the dependencies publicly. Still for two reasons:

* We want to be able to upgrade TLS libraries without affecting our public API (breaking changes).

* A significant amount of people still need native-tls support for whatever reason.

We already export http::StatusCode. Is http::Uri from the same crate fine?