Upgrade to Hyper version 1 #6898

Serock3 · 2024-10-02T07:21:12Z

Fixes DES-1270

This change is

linear · 2024-10-02T07:21:14Z

DES-1270 Upgrade to `hyper` 1 in app repo

MarkusPettersson98

Reviewed 15 of 15 files at r1, all commit messages.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @Serock3)

mullvad-api/src/rest.rs line 17 at r1 (raw file):

use hyper::{
    body::{Body, Bytes, Incoming},
    // client::{connect::Connect, Client},

Unused imports

Code quote:

// client::{connect::Connect, Client},

mullvad-api/src/rest.rs line 89 at r1 (raw file):

    pub fn is_network_error(&self) -> bool {
        matches!(self, Error::HyperError(_) | Error::TimeoutError)
    }

Should this also match on Error::LegacyHyperError?

Code quote:

    pub fn is_network_error(&self) -> bool {
        matches!(self, Error::HyperError(_) | Error::TimeoutError)
    }

mullvad-api/src/rest.rs line 105 at r1 (raw file):

            _ => false,
        }
    }

Yes, I think so.. We'll have to figure out what to look for though, I am not certain at a quick glance :)

Code quote:

    /// Return true if there was no route to the destination
    pub fn is_offline(&self) -> bool {
        match self {
            Error::LegacyHyperError(error) if error.is_connect() => {
                if let Some(cause) = error.source() {
                    if let Some(err) = cause.downcast_ref::<std::io::Error>() {
                        return err.raw_os_error() == Some(libc::ENETUNREACH);
                    }
                }
                false
            }
            // TODO: Match on `Error::HyperError` too?
            _ => false,
        }
    }

mullvad-api/src/rest.rs line 137 at r1 (raw file):

    command_rx: mpsc::UnboundedReceiver<RequestCommand>,
    connector_handle: HttpsConnectorWithSniHandle,
    client: hyper_util::client::legacy::Client<HttpsConnectorWithSni, BoxBody<Bytes, Error>>,

Is it worth it to create a type alias for this type? I don't think it tells the reader much as is unfortunately:/

Code quote:

BoxBody<Bytes, Error>

mullvad-api/src/rest.rs line 305 at r1 (raw file):

// TODO: merge with `RequestFactory::get`
/// Constructs a GET request with the given URI. Returns an error if the URI is not valid.
pub fn get(uri: &str) -> Result<Request<Empty<Bytes>>> {

I assume this TODO is left for some other day and won't be resolved this time around? :)

Code quote:

// TODO: merge with `RequestFactory::get`
/// Constructs a GET request with the given URI. Returns an error if the URI is not valid.
pub fn get(uri: &str) -> Result<Request<Empty<Bytes>>> {

mullvad-api/src/rest.rs line 600 at r1 (raw file):

        let body_length = json_body.as_bytes().len();
        let body = Full::new(Bytes::from(json_body.into_bytes()));
        *request.body_mut() = body;

From our small pair programming session:

let json_body = serde_json::to_vec(&body)?;
let body_length = json_body.len();
*request.body_mut() = Full::new(Bytes::from(json_body));

Code quote:

        let json_body = serde_json::to_string(&body)?;
        let body_length = json_body.as_bytes().len();
        let body = Full::new(Bytes::from(json_body.into_bytes()));
        *request.body_mut() = body;

mullvad-api/src/rest.rs line 680 at r1 (raw file):

}

async fn deserialize_body_inner<T, B>(response: hyper::Response<B>, _: usize) -> Result<T>

Surely the unused argument can be removed? :D

Code quote:

async fn deserialize_body_inner<T, B>(response: hyper::Response<B>, _: usize) -> Result<T>

mullvad-api/src/tls_stream.rs line 33 at r1 (raw file):

            ))
            .with_protocol_versions(&[&rustls::version::TLS13])
            .expect("Ring crypt-prover should support TLS 1.3")

ring*

Code quote:

.expect("Ring crypt-prover should support TLS 1.3")

Serock3

Reviewable status: 9 of 15 files reviewed, 7 unresolved discussions (waiting on @MarkusPettersson98)

mullvad-api/src/rest.rs line 17 at r1 (raw file):