Skip to content

Commit

Permalink
chore(fuzz): address hyper deprecations in fuzz tests (#3455)
Browse files Browse the repository at this point in the history
* chore(app/inbound): address hyper deprecations in http/1 tests

this is a follow-up commit related to 24dc5d8 (#3445).

see <linkerd/linkerd2#8733> for more
information on upgrading to hyper 1.0.

---

this addresses hyper deprecations in the http/1 tests for the inbound
proxy.

prior, we made use of `tower::ServiceExt::oneshot`, which consumes a
service and drops it after sending a request and polling the response
future to completion.

<https://docs.rs/tower/0.5.2/src/tower/util/oneshot.rs.html#96-100>

tower is not a 1.0 library yet, so `SendRequest` does not provide an
implementation of `tower::Service` in hyper's 1.0 interface:

- <https://docs.rs/hyper/0.14.31/hyper/client/conn/struct.SendRequest.html#impl-Service%3CRequest%3CB%3E%3E-for-SendRequest%3CB%3E>
- <https://docs.rs/hyper/1.5.1/hyper/client/conn/http1/struct.SendRequest.html#trait-implementations>

consequentially, we must drop the sender ourselves after receiving a
response now.

---

this commit *also* addresses hyper deprecations in the http/1 downgrade
tests for the inbound proxy.

because these tests involve a http/2 client and an http/1 server, we
take the choice of inlining the body of
`http_util::connect_and_accept()` rather than introducing a new, third
`http_util::connect_and_accept_http_downgrade()` function.

we will refactor these helper functions in follow-on commits.

NB: because `ContextError` is internal to the `linkerd-app-test` crate,
we do not wrap the errors. these are allegedly used by the fuzzing tests
(_see f.ex #986 and #989_), but for our purposes with respect to the
inbound proxy we can elide them rather than making `ctx()` a public
method.

---

Signed-off-by: katelyn martin <[email protected]>

* refactor(app/test): remove unused `http_util::connect_and_accept(..)`

this removes `connect_and_accept(..)`. this will break fuzzing builds,
but it is not used elsewhere.

Signed-off-by: katelyn martin <[email protected]>

* chore(fuzz): address hyper deprecation in inbound fuzz tests

Signed-off-by: katelyn martin <[email protected]>

* chore(fuzz): address preëxisting fuzz breakage

this commit addresses other breakage found in the fuzz tests, tied to
other previous work.

after these changes, one can observe that the fuzz tests build and run
once more by running the following:

```sh
cargo +nightly fuzz run --fuzz-dir=linkerd/app/inbound/fuzz/ fuzz_target_1
```

Signed-off-by: katelyn martin <[email protected]>

* nit(fuzz): remove stray newline from manifest

Signed-off-by: katelyn martin <[email protected]>

---------

Signed-off-by: katelyn martin <[email protected]>
  • Loading branch information
cratelyn authored Dec 13, 2024
1 parent 42fe4cf commit f9e65f8
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 21 deletions.
3 changes: 3 additions & 0 deletions linkerd/app/inbound/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ hyper = { version = "0.14", features = ["deprecated", "http1", "http2"] }
linkerd-app-test = { path = "../test" }
arbitrary = { version = "1", features = ["derive"] }
libfuzzer-sys = { version = "0.4", features = ["arbitrary-derive"] }
linkerd-meshtls-rustls = { path = "../../meshtls/rustls", features = [
"test-util",
] }

[dev-dependencies]
hyper = { version = "0.14", features = ["deprecated", "http1", "http2"] }
Expand Down
1 change: 0 additions & 1 deletion linkerd/app/inbound/fuzz/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

[package]
name = "linkerd-app-inbound-fuzz"
version = "0.0.0"
Expand Down
27 changes: 11 additions & 16 deletions linkerd/app/inbound/src/http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ pub mod fuzz {
test_util::{support::connect::Connect, *},
Config, Inbound,
};
use hyper::{client::conn::Builder as ClientBuilder, Body, Request, Response};
use hyper::{Body, Request, Response};
use libfuzzer_sys::arbitrary::Arbitrary;
use linkerd_app_core::{
identity, io,
Expand All @@ -41,9 +41,8 @@ pub mod fuzz {
}

pub async fn fuzz_entry_raw(requests: Vec<HttpRequestSpec>) {
let mut server = hyper::server::conn::Http::new();
server.http1_only(true);
let mut client = ClientBuilder::new();
let server = hyper::server::conn::http1::Builder::new();
let mut client = hyper::client::conn::http1::Builder::new();
let connect =
support::connect().endpoint_fn_boxed(Target::addr(), hello_fuzz_server(server));
let profiles = profile::resolver();
Expand All @@ -55,7 +54,7 @@ pub mod fuzz {
let cfg = default_config();
let (rt, _shutdown) = runtime();
let server = build_fuzz_server(cfg, rt, profiles, connect).new_service(Target::HTTP1);
let (mut client, bg) = http_util::connect_and_accept(&mut client, server).await;
let (mut client, bg) = http_util::connect_and_accept_http1(&mut client, server).await;

// Now send all of the requests
for inp in requests.iter() {
Expand All @@ -74,14 +73,7 @@ pub mod fuzz {
.header(header_name, header_value)
.body(Body::default())
{
let rsp = client
.ready()
.await
.expect("HTTP client poll_ready failed")
.call(req)
.await
.expect("HTTP client request failed");
tracing::info!(?rsp);
let rsp = client.send_request(req).await;
tracing::info!(?rsp);
if let Ok(rsp) = rsp {
let body = http_util::body_to_string(rsp.into_body()).await;
Expand All @@ -93,18 +85,18 @@ pub mod fuzz {
}
}

drop(client);
// It's okay if the background task returns an error, as this would
// indicate that the proxy closed the connection --- which it will do on
// invalid inputs. We want to ensure that the proxy doesn't crash in the
// face of these inputs, and the background task will panic in this
// case.
let res = bg.await;
drop(client);
let res = bg.join_all().await;
tracing::info!(?res, "background tasks completed")
}

fn hello_fuzz_server(
http: hyper::server::conn::Http,
http: hyper::server::conn::http1::Builder,
) -> impl Fn(Remote<ServerAddr>) -> io::Result<io::BoxedIo> {
move |_endpoint| {
let (client_io, server_io) = support::io::duplex(4096);
Expand Down Expand Up @@ -235,6 +227,9 @@ pub mod fuzz {
kind: "server".into(),
name: "testsrv".into(),
}),
local_rate_limit: Arc::new(
linkerd_proxy_server_policy::LocalRateLimit::default(),
),
},
);
policy
Expand Down
7 changes: 3 additions & 4 deletions linkerd/app/test/src/http_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,11 @@ type BoxServer = svc::BoxTcp<io::DuplexStream>;
///
/// Returns a tuple containing (1) a [`SendRequest`] that can be used to transmit a request and
/// await a response, and (2) a [`JoinSet<T>`] running background tasks.
#[allow(deprecated)] // linkerd/linkerd2#8733
pub async fn connect_and_accept(
client_settings: &mut hyper::client::conn::Builder,
pub async fn connect_and_accept_http1(
client_settings: &mut hyper::client::conn::http1::Builder,
server: BoxServer,
) -> (
hyper::client::conn::SendRequest<hyper::Body>,
hyper::client::conn::http1::SendRequest<hyper::Body>,
JoinSet<Result<(), Error>>,
) {
tracing::info!(settings = ?client_settings, "connecting client with");
Expand Down

0 comments on commit f9e65f8

Please sign in to comment.