Skip to content

Commit f9e65f8

Browse files
authored
chore(fuzz): address hyper deprecations in fuzz tests (#3455)
* 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 <kate@buoyant.io> * 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 <kate@buoyant.io> * chore(fuzz): address hyper deprecation in inbound fuzz tests Signed-off-by: katelyn martin <kate@buoyant.io> * 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 <kate@buoyant.io> * nit(fuzz): remove stray newline from manifest Signed-off-by: katelyn martin <kate@buoyant.io> --------- Signed-off-by: katelyn martin <kate@buoyant.io>
1 parent 42fe4cf commit f9e65f8

File tree

4 files changed

+17
-21
lines changed

4 files changed

+17
-21
lines changed

linkerd/app/inbound/Cargo.toml

+3
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,9 @@ hyper = { version = "0.14", features = ["deprecated", "http1", "http2"] }
4949
linkerd-app-test = { path = "../test" }
5050
arbitrary = { version = "1", features = ["derive"] }
5151
libfuzzer-sys = { version = "0.4", features = ["arbitrary-derive"] }
52+
linkerd-meshtls-rustls = { path = "../../meshtls/rustls", features = [
53+
"test-util",
54+
] }
5255

5356
[dev-dependencies]
5457
hyper = { version = "0.14", features = ["deprecated", "http1", "http2"] }

linkerd/app/inbound/fuzz/Cargo.toml

-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
21
[package]
32
name = "linkerd-app-inbound-fuzz"
43
version = "0.0.0"

linkerd/app/inbound/src/http.rs

+11-16
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ pub mod fuzz {
1818
test_util::{support::connect::Connect, *},
1919
Config, Inbound,
2020
};
21-
use hyper::{client::conn::Builder as ClientBuilder, Body, Request, Response};
21+
use hyper::{Body, Request, Response};
2222
use libfuzzer_sys::arbitrary::Arbitrary;
2323
use linkerd_app_core::{
2424
identity, io,
@@ -41,9 +41,8 @@ pub mod fuzz {
4141
}
4242

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

6059
// Now send all of the requests
6160
for inp in requests.iter() {
@@ -74,14 +73,7 @@ pub mod fuzz {
7473
.header(header_name, header_value)
7574
.body(Body::default())
7675
{
77-
let rsp = client
78-
.ready()
79-
.await
80-
.expect("HTTP client poll_ready failed")
81-
.call(req)
82-
.await
83-
.expect("HTTP client request failed");
84-
tracing::info!(?rsp);
76+
let rsp = client.send_request(req).await;
8577
tracing::info!(?rsp);
8678
if let Ok(rsp) = rsp {
8779
let body = http_util::body_to_string(rsp.into_body()).await;
@@ -93,18 +85,18 @@ pub mod fuzz {
9385
}
9486
}
9587

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

10698
fn hello_fuzz_server(
107-
http: hyper::server::conn::Http,
99+
http: hyper::server::conn::http1::Builder,
108100
) -> impl Fn(Remote<ServerAddr>) -> io::Result<io::BoxedIo> {
109101
move |_endpoint| {
110102
let (client_io, server_io) = support::io::duplex(4096);
@@ -235,6 +227,9 @@ pub mod fuzz {
235227
kind: "server".into(),
236228
name: "testsrv".into(),
237229
}),
230+
local_rate_limit: Arc::new(
231+
linkerd_proxy_server_policy::LocalRateLimit::default(),
232+
),
238233
},
239234
);
240235
policy

linkerd/app/test/src/http_util.rs

+3-4
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,11 @@ type BoxServer = svc::BoxTcp<io::DuplexStream>;
1313
///
1414
/// Returns a tuple containing (1) a [`SendRequest`] that can be used to transmit a request and
1515
/// await a response, and (2) a [`JoinSet<T>`] running background tasks.
16-
#[allow(deprecated)] // linkerd/linkerd2#8733
17-
pub async fn connect_and_accept(
18-
client_settings: &mut hyper::client::conn::Builder,
16+
pub async fn connect_and_accept_http1(
17+
client_settings: &mut hyper::client::conn::http1::Builder,
1918
server: BoxServer,
2019
) -> (
21-
hyper::client::conn::SendRequest<hyper::Body>,
20+
hyper::client::conn::http1::SendRequest<hyper::Body>,
2221
JoinSet<Result<(), Error>>,
2322
) {
2423
tracing::info!(settings = ?client_settings, "connecting client with");

0 commit comments

Comments
 (0)