Skip to content

feat: unstable HttpClient Config #86

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

Merged
merged 4 commits into from
May 17, 2021

Conversation

Fishrock123
Copy link
Member

@Fishrock123 Fishrock123 commented May 4, 2021

This adds an unstable-config feature, with a new Config struct, which can be used to configure any HttpClient which implements support for it.

Currently it supports two features - the most important and most generally supported:

  • timeout (Duration)
  • tcp_no_delay (bool)

Implementations are provided for async-h1, isahc, and hyper (partial, no no_delay support due to the tls connector).

No serious attempt has been made to add this to the wasm client at this point, since I don't understand well how to even build the wasm client or if it even works anymore with the state of rust wasm web build tools.

This represents the development of the older draft PR: #84
Should address #61


Possible next targets:

@Fishrock123 Fishrock123 requested review from jbr and yoshuawuyts May 4, 2021 23:09
@Fishrock123 Fishrock123 force-pushed the configuration branch 2 times, most recently from be9c60f to ad3d8c9 Compare May 5, 2021 17:06
This adds an `unstable-config` feature, with a new `Config` struct, which can be used to configure any `HttpClient` which implements support for it.

Currently it supports two features - the most important and most generally supported:
- `timeout` (`Duration`)
- `no_delay` (`bool`)

Implementations are provided for async-h1, isahc, and hyper (partial, no `no_delay` support due to the tls connector).

No serious attempt has been made to add this to the wasm client at this point, since I don't understand well how to even build the wasm client or if it even works anymore with the state of rust wasm web build tools.
@Fishrock123
Copy link
Member Author

Updated with an http_keep_alive option.

@jbr
Copy link
Member

jbr commented May 5, 2021

I think I've mentioned this before, but I'd be curious how tls configuration fits into this — would that be a later enhancement?

Also renamed `no_delay` to `tcp_no_delay`.

the options for configuring this in Isahc and Hyper are unfortunately not super clear.
@Fishrock123
Copy link
Member Author

Fishrock123 commented May 5, 2021

Probably - as it stands a third of our configurations are completely unable to use TLS configuration of any kind, because they use dependencies that offer no options, namely:

  • hyper-tls
  • async-tls (the rustls adaptor, but rustls itself is very configurable)

We could support PEM or DER encoded X.509 certificates for isahc and async-native-tls, and maybe a couple extra options:

  • isahc: can set ca certs to trust, ssl ciphers, client cert, or be told to trust invalid things
  • async-native-tls: can add ca certs to trust, can be told to trust invalid things, ssl ciphers controlled by tls version flags

EDIT: Turns out I was very wrong and these things do indeed have configuration for this but it's a little hidden.

@Fishrock123
Copy link
Member Author

Ok updated with tls options for the async-h1 client. Different configuration is accepted depending if native-tls or rustls is selected.

This is not implemented for the isahc or hyper clients as those can be set directly and then wrapped, and the complexity of doing that integration fully would be too substantial for this PR.

@Fishrock123 Fishrock123 force-pushed the configuration branch 3 times, most recently from 6266127 to 5e793df Compare May 6, 2021 00:20
Only supports the h1 client, but has two different options, one each for native-tls and rustls.
@Fishrock123
Copy link
Member Author

I'd rather avoid max_connections for this unstable pass, it should really be re-thought for http-client 7.0...

@Fishrock123
Copy link
Member Author

merging this as an unstable feature and releasing it.

@Fishrock123 Fishrock123 merged commit 7d8db8a into http-rs:main May 17, 2021
Fishrock123 added a commit to Fishrock123/surf that referenced this pull request Jul 5, 2021
Finally adds top-level configuration to surf, built on top of http-client's still as of yet unstable config.

Related to http-rs/http-client#86

Closes http-rs#274
Closes http-rs#277
Fishrock123 added a commit to Fishrock123/surf that referenced this pull request Jul 5, 2021
Finally adds top-level configuration to surf, built on top of http-client's still as of yet unstable config.

Related to http-rs/http-client#86

Closes http-rs#274
Closes http-rs#277
Fishrock123 added a commit to Fishrock123/surf that referenced this pull request Jul 5, 2021
Finally adds top-level configuration to surf, built on top of http-client's still as of yet unstable config.

Related to http-rs/http-client#86

Closes http-rs#274
Closes http-rs#277
Fishrock123 added a commit to Fishrock123/surf that referenced this pull request Jul 30, 2021
Finally adds top-level configuration to surf, built on top of http-client's still as of yet unstable config.

Related to http-rs/http-client#86

Closes http-rs#274
Closes http-rs#277
Fishrock123 added a commit to Fishrock123/http-client that referenced this pull request Aug 10, 2021
I've been using this in projects for a couple months now, and would like to land support in Surf directly, for which this should be stabilized.

Refs: http-rs/surf#310
Refs: http-rs#86
Closes: http-rs#93
Fishrock123 added a commit to Fishrock123/http-client that referenced this pull request Aug 10, 2021
I've been using this in projects for a couple months now, and would like to land support in Surf directly, for which this should be stabilized.

Refs: http-rs/surf#310
Refs: http-rs#86
Closes: http-rs#93
Fishrock123 added a commit to Fishrock123/http-client that referenced this pull request Aug 10, 2021
I've been using this in projects for a couple months now, and would like to land support in Surf directly, for which this should be stabilized.

Refs: http-rs/surf#310
Refs: http-rs#86
Closes: http-rs#93
Fishrock123 added a commit to Fishrock123/http-client that referenced this pull request Aug 10, 2021
I've been using this in projects for a couple months now, and would like to land support in Surf directly, for which this should be stabilized.

Refs: http-rs/surf#310
Refs: http-rs#86
Closes: http-rs#93
Fishrock123 added a commit to Fishrock123/surf that referenced this pull request Aug 23, 2021
Finally adds top-level configuration to surf, built on top of http-client's
newly stabilized `Config`.

Related to http-rs/http-client#86

Closes http-rs#274
Closes http-rs#277
@Fishrock123
Copy link
Member Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants