Update `http` dependency to `1.x` #9

MadLittleMods · 2025-02-05T02:10:27Z

Update http dependency to the latest version 1.x.

Since this is a breaking incompatible change in http from 0.2.9 to 1.2.0, a lot of libraries don't interoperate with each other unless they also use 1.x of http. This is why we have a slew of dependent crates being updated.
For example, if we're updating hyper, then we're going to want hyper-rustls, etc to be up to date to get rid of any incompatibility headache. If I had to update one crate, I just updated the whole relevant category.
Each crate update comes with it's own set of changes that need to be figured out. Most commits have some references for how I found the upgrade path and recipes.

This is a breaking change so the tagged release version should reflect that.

This is spawning from wanting to update the http dependency in the Secure Border Gateway (SBG) (https://github.com/element-hq/sbg/pull/521).

Dev notes

https://hyper.rs/guides/1/upgrading/

> The free functions `hyper::body::to_bytes` and `aggregate` have been removed. Similar functionality is on `http_body_util::BodyExt::collect`. (0888623d) > > https://github.com/hyperium/hyper/blob/master/CHANGELOG.md#v100-rc1-2022-10-25 > remove higher-level `hyper::Client` ([#2941](hyperium/hyper#2941)) (bb3af17c) > > https://github.com/hyperium/hyper/blob/master/CHANGELOG.md#v100-rc1-2022-10-25 > remove client::connect module ([#2949](hyperium/hyper#2949)) (5e206883) > > > https://github.com/hyperium/hyper/blob/master/CHANGELOG.md#v100-rc1-2022-10-25 > remove `server::conn::{Http, Connection}` types ([#3013](hyperium/hyper#3013)) (0766d3f7, closes #3012) > > https://github.com/hyperium/hyper/blob/master/CHANGELOG.md#v100-rc1-2022-10-25 > The `poll_ready` method has been removed. (fee7d361) > > https://github.com/hyperium/hyper/blob/master/CHANGELOG.md#v100-rc1-2022-10-25 > replace IO traits with hyper::rt ones ([#3230](hyperium/hyper#3230)) (f9f65b7a, closes #3110) > > https://github.com/hyperium/hyper/blob/master/CHANGELOG.md#v100-rc1-2022-10-25

> Tower `Service` utilities will exist in `hyper-util`. (889fa2d8) > > https://github.com/hyperium/hyper/blob/master/CHANGELOG.md#v100-rc1-2022-10-25 Also see hyperium/hyper#1782

> ConfigBuilder::with_safe_defaults - calls to this can simply be deleted since safe defaults are now implicit. > > https://github.com/rustls/rustls/releases/tag/v%2F0.22.0

``` use of deprecated method `hyper_rustls::HttpsConnectorBuilder::<hyper_rustls::builderstates::WantsProtocols1>::with_server_name`: use Self::with_server_name_resolver with FixedServerNameResolver instead `#[warn(deprecated)]` on by default ```

MadLittleMods · 2025-02-05T02:15:30Z

src/client.rs

 ///
 /// Either use [`SigningFederationClient`] if you want requests to be automatically
 /// signed, or [`sign_and_build_json_request`] to sign the requests.
 #[derive(Debug, Clone)]
 pub struct FederationClient {
-    pub client: hyper::Client<MatrixConnector>,
+    pub client: Client<MatrixConnector, Full<Bytes>>,


Full<Bytes> seems like the meta for hyper::Body looking at the hyper-util examples and hyper examples, at-least for Request<...> and Client<...>.

We do use Response<hyper::body::Incoming> though (hyper::Body was renamed to hyper::body::Incoming, see hyperium/hyper#2971)

Edit: There is also some interesting notes on https://hyper.rs/guides/1/upgrading/

Body

The Body type has changed to be a trait (what used to be HttpBody).

The 0.14 Body could be multiple variants, and in v1 they have been split into distinct types. You’ll benefit from analzying each place you use hyper::Body to decide which solution to switch to.

In general, if you don’t need a specific variant, consider making your usage generic, accepting an impl Body (or where B: Body).

If you want a type that can be any variant, you could use BoxBody.

Otherwise, the more specific variants allow for a more explicit API in your code.

If we want to be generic, we'd have to add some more generic's to FederationClient and SigningFederationClient

The B here is for the body of the request right? My only concern is that generally you don't want to use Full<_> style APIs when receiving data from remotes, as then stuff gets buffered up in memory and it becomes easy to DoS. That's not a problem if its just used by the calling code

Re-reading, it looks like this could use a reply.

From the hyper code itself, it's really unclear and they don't have any doc comments for it but in all of our usage, it's only Request<Full<Bytes>> and Response<hyper::body::Incoming> which matches your hopes.

src/client.rs

MadLittleMods · 2025-02-05T02:18:15Z

src/client.rs

@@ -79,7 +91,7 @@ impl SigningFederationClient<MatrixConnector> {
        let connector = MatrixConnector::with_default_resolver()?;

        Ok(SigningFederationClient {
-            client: Client::builder().build(connector),
+            client: Client::builder(TokioExecutor::new()).build(connector),


Matches the hyper-util example

MadLittleMods · 2025-02-05T02:25:37Z

src/server_resolver.rs

@@ -314,8 +312,8 @@ impl MatrixConnector {
    /// Create new [`MatrixConnector`] with the given [`MatrixResolver`].
    pub fn with_resolver(resolver: MatrixResolver) -> MatrixConnector {
        let client_config = ClientConfig::builder()
-            .with_safe_defaults()


ConfigBuilder::with_safe_defaults - calls to this can simply be deleted since safe defaults are now implicit.

https://github.com/rustls/rustls/releases/tag/v%2F0.22.0

MadLittleMods · 2025-02-05T02:25:59Z

src/server_resolver.rs

+                    .with_server_name_resolver(hyper_rustls::FixedServerNameResolver::new(
+                        endpoint.tls_name.clone().try_into().context(format!(
+                            "failed to create `ServerName` from `endpoint.tls_name`: {}",
+                            endpoint.tls_name
+                        ))?,
+                    ))


Context:

use of deprecated method `hyper_rustls::HttpsConnectorBuilder::<hyper_rustls::builderstates::WantsProtocols1>::with_server_name`: use Self::with_server_name_resolver with FixedServerNameResolver instead `#[warn(deprecated)]` on by default

MadLittleMods · 2025-02-05T02:27:09Z

src/server_resolver.rs

-#[cfg(test)]
-mod test {


I removed these tests because they don't seem to test anything that we're exposing. It's just testing the TestConnector and TestConnection.

Do these provide any value?

Hmm, yeah looks like these are just test infrastructure without any actual tests...

MadLittleMods · 2025-02-05T02:28:23Z

src/client.rs

@@ -148,7 +174,7 @@ where
        let content = if body.is_end_stream() {
            None
        } else {
-            let bytes = to_bytes(body).await?;


The free functions hyper::body::to_bytes and aggregate have been removed. Similar functionality is on http_body_util::BodyExt::collect. (0888623d)

https://github.com/hyperium/hyper/blob/master/CHANGELOG.md#v100-rc1-2022-10-25

MadLittleMods · 2025-02-05T02:29:29Z

src/server_resolver.rs

 use hyper_rustls::{ConfigBuilderExt, MaybeHttpsStream};
+use hyper_util::client::legacy::connect::Connect;


We have to use hyper_util::client::legacy::connect::Connect now,

remove client::connect module (#2949) (5e206883)

https://github.com/hyperium/hyper/blob/master/CHANGELOG.md#v100-rc1-2022-10-25

MadLittleMods · 2025-02-05T02:29:39Z

src/server_resolver.rs

 use hyper_rustls::{ConfigBuilderExt, MaybeHttpsStream};
+use hyper_util::client::legacy::connect::Connect;
+use hyper_util::client::legacy::Client;


We have to use hyper_util::client::legacy::Client now,

remove higher-level hyper::Client (#2941) (bb3af17c)

https://github.com/hyperium/hyper/blob/master/CHANGELOG.md#v100-rc1-2022-10-25

MadLittleMods · 2025-02-05T02:35:02Z

src/server_resolver.rs

 use log::{debug, trace, warn};
 use serde::{Deserialize, Serialize};
 use tokio::net::TcpStream;
 use tokio_rustls::rustls::ClientConfig;
+use tower_service::Service;


We're using tower_service::Service instead of hyper::service::Service because the hyper_util::client::legacy::connect::Connect trait only works with tower_service::Service.

I tried fixing hyper-util so that it would also work with hyper::service::Service but I banged my head for too long and decided to give up for now. It looks like hyper people played around with using the tower package but eventually went back to their own hyper::service::Service so now we have these weird artifacts.

feat: create Service trait hyperium/hyper#2920

feat(client): remove hyper::client::server hyperium/hyper#2940

There are additional complications with the hyper_rustls::HttpsConnectorBuilder using tower_service::Service so you have to use hyper_util::service::TowerToHyperService::new(https_connector) in order to make it compatible again. You can see my workings around this in 3a447c9 before I just moved to using tower_service::Service

erikjohnston · 2025-02-13T12:02:54Z

src/client.rs

 ///
 /// Either use [`SigningFederationClient`] if you want requests to be automatically
 /// signed, or [`sign_and_build_json_request`] to sign the requests.
 #[derive(Debug, Clone)]
 pub struct FederationClient {
-    pub client: hyper::Client<MatrixConnector>,
+    pub client: Client<MatrixConnector, Full<Bytes>>,


The B here is for the body of the request right? My only concern is that generally you don't want to use Full<_> style APIs when receiving data from remotes, as then stuff gets buffered up in memory and it becomes easy to DoS. That's not a problem if its just used by the calling code

erikjohnston · 2025-02-13T12:05:15Z

src/server_resolver.rs

-#[cfg(test)]
-mod test {


Hmm, yeah looks like these are just test infrastructure without any actual tests...

MadLittleMods · 2025-02-13T15:12:58Z

Thanks for the review @erikjohnston 🦣

MadLittleMods added 8 commits February 4, 2025 17:39

Figure out tower_service::Service vs hyper::service::Service

7100149

> Tower `Service` utilities will exist in `hyper-util`. (889fa2d8) > > https://github.com/hyperium/hyper/blob/master/CHANGELOG.md#v100-rc1-2022-10-25 Also see hyperium/hyper#1782

Fix some more cases

e4ce843

Try use tower_service::Service

3a447c9

Convert to bytes

5916884

Figure out rustls ConfigBuilder changes

cba4b06

> ConfigBuilder::with_safe_defaults - calls to this can simply be deleted since safe defaults are now implicit. > > https://github.com/rustls/rustls/releases/tag/v%2F0.22.0

Fix deprecated with_server_name usage

3239b04

``` use of deprecated method `hyper_rustls::HttpsConnectorBuilder::<hyper_rustls::builderstates::WantsProtocols1>::with_server_name`: use Self::with_server_name_resolver with FixedServerNameResolver instead `#[warn(deprecated)]` on by default ```

Restore is_end_stream() checks

dc3e44b

MadLittleMods commented Feb 5, 2025

View reviewed changes

Simplify to Response<hyper::body::Incoming>

194d7f7

erikjohnston approved these changes Feb 13, 2025

View reviewed changes

erikjohnston merged commit 69caf10 into erikjohnston:main Feb 13, 2025
3 checks passed

		use hyper_rustls::{ConfigBuilderExt, MaybeHttpsStream};
		use hyper_util::client::legacy::connect::Connect;

		#[cfg(test)]
		mod test {

		#[cfg(test)]
		mod test {

Update http dependency to 1.x #9

Update http dependency to 1.x #9

Uh oh!

Conversation

MadLittleMods commented Feb 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Dev notes

Uh oh!

MadLittleMods Feb 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Body

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MadLittleMods commented Feb 13, 2025

Uh oh!

Uh oh!

Update `http` dependency to `1.x` #9

Update `http` dependency to `1.x` #9

MadLittleMods commented Feb 5, 2025 •

edited

Loading

MadLittleMods Feb 5, 2025 •

edited

Loading

`Body`