chore: downgrade to rust 1.85.1

[sasa-tomic]

This reverts commit d1dc4c2 which broke the x86_64-darwin build of PocketIC. To improve test coverage, we include the prefix //packages/pocket-ic (in addition to //rs) for macOS Intel tests.

A minimal example (PoC) of the PocketIC issue on x86_64-darwin is available on the branch mraszyk/hyper-issue and can be executed as follows:

bazel test //rs/pocket_ic_server:hyper_issue --cache_test_results=no --test_output=streamed --test_arg="--nocapture"

It passes on Rust version 1.85.1 (reverted to by this PR), but fails on 1.86.0 and 1.87.0 (note that ./bin/bazel-pin.sh must be run after updating the Rust version in WORKSPACE.bazel).

Also including the PoC here in case the branch was (accidentally) deleted:

diff --git a/rs/pocket_ic_server/BUILD.bazel b/rs/pocket_ic_server/BUILD.bazel
index fc4be6d421..9363533869 100644
--- a/rs/pocket_ic_server/BUILD.bazel
+++ b/rs/pocket_ic_server/BUILD.bazel
@@ -280,3 +280,23 @@ rust_test(
     }),
     deps = TEST_DEPENDENCIES,
 )
+
+rust_test(
+    name = "hyper_issue",
+    size = "small",
+    srcs = [
+        "tests/hyper.rs",
+    ],
+    aliases = {},
+    tags = [
+        "requires-network",
+        "test_macos",
+    ],
+    deps = [
+        "@crate_index//:axum",
+        "@crate_index//:http-body-util",
+        "@crate_index//:hyper",
+        "@crate_index//:hyper-util",
+        "@crate_index//:tokio",
+    ],
+)
diff --git a/rs/pocket_ic_server/tests/hyper.rs b/rs/pocket_ic_server/tests/hyper.rs
new file mode 100644
index 0000000000..a96691f8d3
--- /dev/null
+++ b/rs/pocket_ic_server/tests/hyper.rs
@@ -0,0 +1,53 @@
+use axum::{response::Html, routing::get, Router};
+use http_body_util::{BodyExt, Empty};
+use hyper::body::{Buf, Bytes};
+use hyper_util::rt::TokioIo;
+use std::io::Read;
+use tokio::net::TcpStream;
+
+async fn handler() -> Html<&'static str> {
+    Html("<h1>Hello, World!</h1>")
+}
+
+#[tokio::test]
+async fn test() {
+    // spawn a webserver
+    tokio::spawn(async {
+        // build our application with a route
+        let app = Router::new().route("/index.html", get(handler));
+
+        // run it
+        let listener = tokio::net::TcpListener::bind("127.0.0.1:3000")
+            .await
+            .unwrap();
+        println!("listening on {}", listener.local_addr().unwrap());
+        axum::serve(listener, app).await.unwrap();
+    });
+
+    // wait until the server starts up
+    tokio::time::sleep(std::time::Duration::from_secs(5)).await;
+
+    // send an HTTP request to the webserver
+    let stream = TcpStream::connect("127.0.0.1:3000").await.unwrap();
+    let io = TokioIo::new(stream);
+
+    let (mut sender, conn) = hyper::client::conn::http1::handshake(io).await.unwrap();
+    tokio::task::spawn(conn);
+
+    let req = hyper::Request::builder()
+        .uri("http://127.0.0.1:3000/index.html")
+        .body(Empty::<Bytes>::new())
+        .unwrap();
+
+    let res = sender.send_request(req).await.unwrap();
+    println!("hyper response: {:?}", res);
+
+    // asynchronously aggregate the chunks of the body
+    let mut body = res.collect().await.unwrap().aggregate().reader();
+    let mut buffer = Vec::new();
+    body.read_to_end(&mut buffer).unwrap();
+    let body = String::from_utf8(buffer).unwrap();
+
+    println!("hyper body: {}", body);
+    assert!(body.contains("Hello, World!"));
+}

When debugging this further we discovered that the hyper web client library on x86_64-darwin doesn't write the HTTP version into the HTTP request. So the request looks like:

GET /index.html \r\n\r\n

while a successful request with curl looks like:

GET /index.html HTTP/1.1\r\n\r\n

Looking into this further and in particular how the hyper client encodes HTTP requests.
It goes something like this:

match msg.head.version {
...
    Version::HTTP_11 => extend(dst, b"HTTP/1.1"),
...
}
extend(dst, b"\r\n");
... /* move on to headers */

now here’s the weird part. In our case msg.head.version is Version::HTTP_11, i.e. with this change below the test passes (we receive 200 OK):

match msg.head.version {
    Version::HTTP_11 => extend(dst, b"HTTP/1.1"),
    other => {
        panic!("bad version");
    }
}

unsurprisingly, with this next snippet the test passes too:

match msg.head.version {
    Version::HTTP_10 => panic!("not expecting this version"), /* test passes */
    Version::HTTP_11 => extend(dst, b"HTTP/1.1"),
    other => {
        panic!("bad version");
    }
}

here this is just an extra handling of a potentially bad version and we still get a 200 OK back. On this other hand, this one causes the test to fail (we get 400 Bad Request, i.e. the version is missing):

match msg.head.version {
    version::http_11 => extend(dst, b"http/1.1"),
    version::http_10 => extend(dst, b"http/1.0"),
    version::http_2 => {
        debug!("request with http2 version coerced to http/1.1");
        extend(dst, b"http/1.1");
    },
    other => {
        panic!("unexpected request version: {:?}", other);
        extend(dst, b"http/1.1")
    },
}

In theory the runtime behavior should be pretty much the same; and while this snippet makes the test fail, changing characters here an then (adding match cases, etc) can make it pass. So unfortunately this looks like it might be a compiler issue on x86-64 darwin.

[github-actions]

If this pull request changes the behavior of any canister owned by
the Governance team in an externally visible way, remember to
update the corresponding unreleased_changelog.md file(s).

To acknowldge this reminder (and unblock the PR), dismiss this
code review by going to the bottom of the pull request page, look
for where it says this bot is requesting changes, click the three
dots on the right, select "Dismiss review", and supply one of the
following reasons:

1. Done.

2. No canister behavior changes.

To be more precise, "externally visible behavior change" usually
means that responses differ in some way. However, "externally
visible behavior change" is not limited to that. For example, it
could also means that the canister makes different requests to
other canisters.

No canister behavior changes.

[mbjorkqvist]

FI change LGTM!