From f048fd955f7d762310af4d3c8c4868d01494c50e Mon Sep 17 00:00:00 2001 From: "Adam C. Foltzer" Date: Tue, 19 Sep 2023 15:38:19 -0700 Subject: [PATCH] =?UTF-8?q?=F0=9F=91=BB=20Add=20options=20to=20customize?= =?UTF-8?q?=20behavior=20of=20unknown=20Wasm=20imports?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a new shared option to the CLI with three behavior modes: ``` --unknown-import-behavior Set the behavior for unknown imports. Note that if a program only works with a non-default setting for this flag, it is unlikely to be publishable to Fastly. [default: link-error] Possible values: - link-error: Unknown imports are rejected at link time (default behavior) - trap: Unknown imports trap when called - zero-or-null: Unknown imports return zero or a null pointer, depending on the type ``` This ended up touching more files than I'd expect, because I had to make the integration test harness bubble out link-time errors rather than `.expect()`ing them, so there are quite a few new `?`s across the integration test suite. --- Cargo.lock | 1 + Cargo.toml | 1 + cli/Cargo.toml | 2 +- cli/src/main.rs | 1 + cli/src/opts.rs | 17 ++- cli/tests/integration/async_io.rs | 12 +- cli/tests/integration/body.rs | 6 +- cli/tests/integration/client_certs.rs | 2 +- cli/tests/integration/common.rs | 59 +++++++--- cli/tests/integration/config_store_lookup.rs | 6 +- cli/tests/integration/dictionary_lookup.rs | 6 +- cli/tests/integration/downstream_req.rs | 2 +- cli/tests/integration/env_vars.rs | 2 +- cli/tests/integration/geolocation_lookup.rs | 6 +- cli/tests/integration/grpc.rs | 2 +- cli/tests/integration/http_semantics.rs | 4 +- cli/tests/integration/kv_store.rs | 6 +- cli/tests/integration/logging.rs | 2 +- cli/tests/integration/main.rs | 1 + cli/tests/integration/memory.rs | 6 +- cli/tests/integration/request.rs | 2 +- cli/tests/integration/response.rs | 2 +- cli/tests/integration/secret_store.rs | 2 +- cli/tests/integration/sending_response.rs | 8 +- cli/tests/integration/sleep.rs | 2 +- .../integration/unknown_import_behavior.rs | 81 +++++++++++++ cli/tests/integration/upstream.rs | 16 +-- cli/tests/integration/upstream_async.rs | 2 +- cli/tests/integration/upstream_dynamic.rs | 10 +- cli/tests/integration/upstream_streaming.rs | 2 +- cli/tests/trap-test/Cargo.lock | 107 ++++++++++++++++++ cli/tests/trap-test/src/main.rs | 8 +- lib/Cargo.toml | 1 + lib/src/config.rs | 11 ++ lib/src/execute.rs | 12 +- lib/src/service.rs | 2 +- test-fixtures/src/bin/unknown-import.rs | 28 +++++ 37 files changed, 365 insertions(+), 75 deletions(-) create mode 100644 cli/tests/integration/unknown_import_behavior.rs create mode 100644 test-fixtures/src/bin/unknown-import.rs diff --git a/Cargo.lock b/Cargo.lock index 2d733afc..26e33e7f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2385,6 +2385,7 @@ dependencies = [ "bytes", "bytesize", "cfg-if", + "clap", "cranelift-entity 0.88.2", "fastly-shared", "flate2", diff --git a/Cargo.toml b/Cargo.toml index 604c77c8..55562f62 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,6 +23,7 @@ opt-level = 1 [workspace.dependencies] anyhow = "1.0.31" base64 = "0.21.2" +clap = { version = "^4.0.18", features = ["derive"] } hyper = { version = "=0.14.26", features = ["full"] } itertools = "0.10.5" rustls = { version = "0.21.5", features = ["dangerous_configuration"] } diff --git a/cli/Cargo.toml b/cli/Cargo.toml index a881abf5..8a7a9b97 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -35,7 +35,7 @@ base64 = { workspace = true } hyper = { workspace = true } itertools = { workspace = true } serde_json = { workspace = true } -clap = { version = "^4.0.18", features = ["derive"] } +clap = { workspace = true } rustls = { workspace = true } rustls-pemfile = { workspace = true } tls-listener = { version = "^0.7.0", features = ["rustls", "hyper-h1", "tokio-net", "rt"] } diff --git a/cli/src/main.rs b/cli/src/main.rs index 3bc78634..4fb773e8 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -247,6 +247,7 @@ async fn create_execution_context( args.profiling_strategy(), args.wasi_modules(), guest_profile_path, + args.unknown_import_behavior(), )? .with_log_stderr(args.log_stderr()) .with_log_stdout(args.log_stdout()); diff --git a/cli/src/opts.rs b/cli/src/opts.rs index 028ad7c6..af43b63d 100644 --- a/cli/src/opts.rs +++ b/cli/src/opts.rs @@ -1,5 +1,7 @@ //! Command line arguments. +use viceroy_lib::config::UnknownImportBehavior; + use { clap::{Args, Parser, Subcommand, ValueEnum}, std::net::{IpAddr, Ipv4Addr}, @@ -90,6 +92,12 @@ pub struct SharedArgs { /// Set of experimental WASI modules to link against. #[arg(value_enum, long = "experimental_modules", required = false)] experimental_modules: Vec, + /// Set the behavior for unknown imports. + /// + /// Note that if a program only works with a non-default setting for this flag, it is unlikely + /// to be publishable to Fastly. + #[arg(long = "unknown-import-behavior", value_enum, default_value_t = UnknownImportBehavior::LinkError)] + unknown_import_behavior: UnknownImportBehavior, /// Verbosity of logs for Viceroy. `-v` sets the log level to INFO, /// `-vv` to DEBUG, and `-vvv` to TRACE. This option will not take /// effect if you set RUST_LOG to a value before starting Viceroy @@ -151,16 +159,21 @@ impl SharedArgs { self.log_stderr } - // Whether to enable wasmtime's builtin profiler. + /// Whether to enable wasmtime's builtin profiler. pub fn profiling_strategy(&self) -> ProfilingStrategy { self.profiler.unwrap_or(ProfilingStrategy::None) } - // Set of experimental wasi modules to link against. + /// Set of experimental wasi modules to link against. pub fn wasi_modules(&self) -> HashSet { self.experimental_modules.iter().map(|x| x.into()).collect() } + /// Unknown import behavior + pub fn unknown_import_behavior(&self) -> UnknownImportBehavior { + self.unknown_import_behavior + } + /// Verbosity of logs for Viceroy. `-v` sets the log level to DEBUG and /// `-vv` to TRACE. This option will not take effect if you set RUST_LOG /// to a value before starting Viceroy diff --git a/cli/tests/integration/async_io.rs b/cli/tests/integration/async_io.rs index bf1c1298..2a7e476f 100644 --- a/cli/tests/integration/async_io.rs +++ b/cli/tests/integration/async_io.rs @@ -143,7 +143,7 @@ async fn async_io_methods() -> TestResult { .await; // request_count is 0 here - let resp = test.against_empty().await; + let resp = test.against_empty().await?; assert_eq!(resp.status(), StatusCode::OK); assert_eq!(resp.headers()["Simple-Ready"], "false"); @@ -154,7 +154,7 @@ async fn async_io_methods() -> TestResult { barrier.wait().await; request_count.store(1, Ordering::Relaxed); - let resp = test.against_empty().await; + let resp = test.against_empty().await?; assert_eq!(resp.status(), StatusCode::OK); assert_eq!(resp.headers()["Simple-Ready"], "true"); assert_eq!(resp.headers()["Read-Ready"], "false"); @@ -165,7 +165,7 @@ async fn async_io_methods() -> TestResult { barrier.wait().await; request_count.store(2, Ordering::Relaxed); - let resp = test.against_empty().await; + let resp = test.against_empty().await?; assert_eq!(resp.status(), StatusCode::OK); assert_eq!(resp.headers()["Simple-Ready"], "false"); assert_eq!(resp.headers()["Read-Ready"], "true"); @@ -174,7 +174,7 @@ async fn async_io_methods() -> TestResult { barrier.wait().await; request_count.store(3, Ordering::Relaxed); - let resp = test.against_empty().await; + let resp = test.against_empty().await?; assert_eq!(resp.status(), StatusCode::OK); assert_eq!(resp.headers()["Simple-Ready"], "false"); assert_eq!(resp.headers()["Read-Ready"], "false"); @@ -191,7 +191,7 @@ async fn async_io_methods() -> TestResult { .body(Body::empty()) .unwrap(), ) - .await; + .await?; assert_eq!(resp.status(), StatusCode::INTERNAL_SERVER_ERROR); let resp = test @@ -201,7 +201,7 @@ async fn async_io_methods() -> TestResult { .body(Body::empty()) .unwrap(), ) - .await; + .await?; assert_eq!(resp.status(), StatusCode::OK); assert_eq!(resp.headers()["Ready-Index"], "timeout"); diff --git a/cli/tests/integration/body.rs b/cli/tests/integration/body.rs index d7e76e3f..b86f483d 100644 --- a/cli/tests/integration/body.rs +++ b/cli/tests/integration/body.rs @@ -7,7 +7,9 @@ use { #[tokio::test(flavor = "multi_thread")] async fn bodies_can_be_written_and_appended() -> TestResult { - let resp = Test::using_fixture("write-body.wasm").against_empty().await; + let resp = Test::using_fixture("write-body.wasm") + .against_empty() + .await?; let body = body::to_bytes(resp.into_body()) .await @@ -23,7 +25,7 @@ async fn bodies_can_be_written_and_appended() -> TestResult { async fn bodies_can_be_written_and_read() -> TestResult { let resp = Test::using_fixture("write-and-read-body.wasm") .against_empty() - .await; + .await?; assert_eq!(resp.status(), StatusCode::OK); Ok(()) } diff --git a/cli/tests/integration/client_certs.rs b/cli/tests/integration/client_certs.rs index 440d312d..6931523b 100644 --- a/cli/tests/integration/client_certs.rs +++ b/cli/tests/integration/client_certs.rs @@ -177,7 +177,7 @@ async fn client_certs_work() -> TestResult { .body("Hello, Viceroy!") .unwrap(), ) - .await; + .await?; assert_eq!(resp.status(), StatusCode::OK); assert_eq!( resp.into_body().read_into_string().await?, diff --git a/cli/tests/integration/common.rs b/cli/tests/integration/common.rs index c1846b84..2b6d42dd 100644 --- a/cli/tests/integration/common.rs +++ b/cli/tests/integration/common.rs @@ -8,6 +8,7 @@ use std::{ sync::Arc, }; use tracing_subscriber::filter::EnvFilter; +use viceroy_lib::config::UnknownImportBehavior; use viceroy_lib::{ body::Body, config::{Dictionaries, FastlyConfig, Geolocation, ObjectStores, SecretStores}, @@ -55,6 +56,7 @@ pub struct Test { log_stdout: bool, log_stderr: bool, via_hyper: bool, + unknown_import_behavior: UnknownImportBehavior, } impl Test { @@ -73,6 +75,7 @@ impl Test { log_stdout: false, log_stderr: false, via_hyper: false, + unknown_import_behavior: Default::default(), } } @@ -91,6 +94,7 @@ impl Test { log_stdout: false, log_stderr: false, via_hyper: false, + unknown_import_behavior: Default::default(), } } @@ -114,6 +118,15 @@ impl Test { self } + /// Use the specified [`UnknownImportBehavior`] for this test. + pub fn using_unknown_import_behavior( + mut self, + unknown_import_behavior: UnknownImportBehavior, + ) -> Self { + self.unknown_import_behavior = unknown_import_behavior; + self + } + /// Add a backend definition to this test. /// /// The `name` is the static backend name that can be passed as, for example, the argument to @@ -232,7 +245,7 @@ impl Test { pub async fn against_many( &self, mut reqs: Vec>>, - ) -> Vec> { + ) -> Result>, Error> { let mut responses = Vec::with_capacity(reqs.len()); // Install a tracing subscriber. We use a human-readable event formatter in tests, using a @@ -262,8 +275,8 @@ impl Test { ProfilingStrategy::None, HashSet::new(), None, - ) - .expect("failed to set up execution context") + self.unknown_import_behavior, + )? .with_backends(self.backends.backend_configs().await) .with_dictionaries(self.dictionaries.clone()) .with_geolocation(self.geolocation.clone()) @@ -307,10 +320,7 @@ impl Test { *req.uri_mut() = new_uri; // Pass the request to the server via a Hyper client on the _current_ task: - let resp = hyper::Client::new() - .request(req.map(Into::into)) - .await - .expect("hyper client error making test request"); + let resp = hyper::Client::new().request(req.map(Into::into)).await?; responses.push(resp.map(Into::into)); } @@ -318,7 +328,7 @@ impl Test { tx.send(()) .expect("sender error while shutting down hyper server"); // Reap the task handle to ensure that the server did indeed shut down. - let _ = server_handle.await.expect("hyper server yielded an error"); + let _ = server_handle.await?; } else { for mut req in reqs.drain(..) { // We do not have to worry about an ephemeral port in the non-hyper scenario, but we @@ -339,13 +349,26 @@ impl Test { .clone() .handle_request(req.map(Into::into), Ipv4Addr::LOCALHOST.into()) .await - .map(|result| result.0) - .expect("failed to handle the request"); + .map(|result| { + match result { + (resp, None) => resp, + (_, Some(err)) => { + // Splat the string representation of the runtime error into a synthetic + // 500. This is a bit of a hack, but good enough to check for expected error + // strings. + let body = err.to_string(); + Response::builder() + .status(hyper::StatusCode::INTERNAL_SERVER_ERROR) + .body(Body::from(body.as_bytes())) + .unwrap() + } + } + })?; responses.push(resp); } } - responses + Ok(responses) } /// Pass the given request to a Viceroy execution context defined by this test. @@ -355,15 +378,19 @@ impl Test { /// /// A `Test` can be used repeatedly against different requests. Note, however, that /// a fresh execution context is set up each time. - pub async fn against(&self, req: Request>) -> Response { - self.against_many(vec![req]) - .await + pub async fn against( + &self, + req: Request>, + ) -> Result, Error> { + Ok(self + .against_many(vec![req]) + .await? .pop() - .expect("singleton back from against_many") + .expect("singleton back from against_many")) } /// Pass an empty `GET /` request through this test. - pub async fn against_empty(&self) -> Response { + pub async fn against_empty(&self) -> Result, Error> { self.against(Request::get("/").body("").unwrap()).await } diff --git a/cli/tests/integration/config_store_lookup.rs b/cli/tests/integration/config_store_lookup.rs index 24c6fb90..c5d3b907 100644 --- a/cli/tests/integration/config_store_lookup.rs +++ b/cli/tests/integration/config_store_lookup.rs @@ -18,7 +18,7 @@ async fn json_config_store_lookup_works() -> TestResult { let resp = Test::using_fixture("config_store-lookup.wasm") .using_fastly_toml(FASTLY_TOML)? .against_empty() - .await; + .await?; assert_eq!(resp.status(), StatusCode::OK); assert!(to_bytes(resp.into_body()) @@ -49,7 +49,7 @@ async fn inline_toml_config_store_lookup_works() -> TestResult { let resp = Test::using_fixture("config_store-lookup.wasm") .using_fastly_toml(FASTLY_TOML)? .against_empty() - .await; + .await?; assert_eq!(resp.status(), StatusCode::OK); assert!(to_bytes(resp.into_body()) @@ -72,7 +72,7 @@ async fn missing_config_store_works() -> TestResult { let resp = Test::using_fixture("config_store-lookup.wasm") .using_fastly_toml(FASTLY_TOML)? .against_empty() - .await; + .await?; assert_eq!(resp.status(), StatusCode::INTERNAL_SERVER_ERROR); diff --git a/cli/tests/integration/dictionary_lookup.rs b/cli/tests/integration/dictionary_lookup.rs index 14d48658..35e8f2ba 100644 --- a/cli/tests/integration/dictionary_lookup.rs +++ b/cli/tests/integration/dictionary_lookup.rs @@ -18,7 +18,7 @@ async fn json_dictionary_lookup_works() -> TestResult { let resp = Test::using_fixture("dictionary-lookup.wasm") .using_fastly_toml(FASTLY_TOML)? .against_empty() - .await; + .await?; assert_eq!(resp.status(), StatusCode::OK); assert!(to_bytes(resp.into_body()) @@ -49,7 +49,7 @@ async fn inline_toml_dictionary_lookup_works() -> TestResult { let resp = Test::using_fixture("dictionary-lookup.wasm") .using_fastly_toml(FASTLY_TOML)? .against_empty() - .await; + .await?; assert_eq!(resp.status(), StatusCode::OK); assert!(to_bytes(resp.into_body()) @@ -72,7 +72,7 @@ async fn missing_dictionary_works() -> TestResult { let resp = Test::using_fixture("dictionary-lookup.wasm") .using_fastly_toml(FASTLY_TOML)? .against_empty() - .await; + .await?; assert_eq!(resp.status(), StatusCode::INTERNAL_SERVER_ERROR); diff --git a/cli/tests/integration/downstream_req.rs b/cli/tests/integration/downstream_req.rs index 88e9722f..d6ce355d 100644 --- a/cli/tests/integration/downstream_req.rs +++ b/cli/tests/integration/downstream_req.rs @@ -11,7 +11,7 @@ async fn downstream_request_works() -> TestResult { .body("Hello, world!")?; let resp = Test::using_fixture("downstream-req.wasm") .against(req) - .await; + .await?; assert_eq!(resp.status(), StatusCode::OK); Ok(()) diff --git a/cli/tests/integration/env_vars.rs b/cli/tests/integration/env_vars.rs index 5a85e04d..5b551295 100644 --- a/cli/tests/integration/env_vars.rs +++ b/cli/tests/integration/env_vars.rs @@ -2,7 +2,7 @@ use crate::common::{Test, TestResult}; #[tokio::test] async fn env_vars_are_set() -> TestResult { - let resp = Test::using_fixture("env-vars.wasm").against_empty().await; + let resp = Test::using_fixture("env-vars.wasm").against_empty().await?; assert!(resp.status().is_success()); Ok(()) } diff --git a/cli/tests/integration/geolocation_lookup.rs b/cli/tests/integration/geolocation_lookup.rs index 00b0c1a4..0d5597f6 100644 --- a/cli/tests/integration/geolocation_lookup.rs +++ b/cli/tests/integration/geolocation_lookup.rs @@ -18,7 +18,7 @@ async fn json_geolocation_lookup_works() -> TestResult { let resp = Test::using_fixture("geolocation-lookup.wasm") .using_fastly_toml(FASTLY_TOML)? .against_empty() - .await; + .await?; assert_eq!(resp.status(), StatusCode::OK); assert!(to_bytes(resp.into_body()) @@ -85,7 +85,7 @@ async fn inline_toml_geolocation_lookup_works() -> TestResult { let resp = Test::using_fixture("geolocation-lookup.wasm") .using_fastly_toml(FASTLY_TOML)? .against_empty() - .await; + .await?; assert_eq!(resp.status(), StatusCode::OK); assert!(to_bytes(resp.into_body()) @@ -109,7 +109,7 @@ async fn default_configuration_geolocation_lookup_works() -> TestResult { let resp = Test::using_fixture("geolocation-lookup-default.wasm") .using_fastly_toml(FASTLY_TOML)? .against_empty() - .await; + .await?; assert_eq!(resp.status(), StatusCode::OK); assert!(to_bytes(resp.into_body()) diff --git a/cli/tests/integration/grpc.rs b/cli/tests/integration/grpc.rs index c3da7973..a8875bb2 100644 --- a/cli/tests/integration/grpc.rs +++ b/cli/tests/integration/grpc.rs @@ -30,7 +30,7 @@ // .body("Hello, Viceroy!") // .unwrap(), // ) -// .await; +// .await?; // assert_eq!(resp.status(), StatusCode::OK); // assert_eq!(resp.into_body().read_into_string().await?, "Hello!"); // diff --git a/cli/tests/integration/http_semantics.rs b/cli/tests/integration/http_semantics.rs index 905c861f..ead4cd87 100644 --- a/cli/tests/integration/http_semantics.rs +++ b/cli/tests/integration/http_semantics.rs @@ -23,7 +23,7 @@ async fn framing_headers_are_overridden() -> TestResult { let resp = test .via_hyper() .against(Request::post("/").body("greetings").unwrap()) - .await; + .await?; assert_eq!(resp.status(), StatusCode::OK); @@ -46,7 +46,7 @@ async fn content_length_is_computed_correctly() -> TestResult { }) .await; - let resp = test.via_hyper().against_empty().await; + let resp = test.via_hyper().against_empty().await?; assert_eq!(resp.status(), StatusCode::OK); diff --git a/cli/tests/integration/kv_store.rs b/cli/tests/integration/kv_store.rs index 5ad0cc27..f535b98e 100644 --- a/cli/tests/integration/kv_store.rs +++ b/cli/tests/integration/kv_store.rs @@ -16,7 +16,7 @@ async fn kv_store() -> TestResult { let resp = Test::using_fixture("kv_store.wasm") .using_fastly_toml(FASTLY_TOML)? .against_empty() - .await; + .await?; assert_eq!(resp.status(), StatusCode::OK); assert!(to_bytes(resp.into_body()) @@ -46,7 +46,7 @@ async fn object_stores_backward_compat() -> TestResult { let resp = Test::using_fixture("kv_store.wasm") .using_fastly_toml(FASTLY_TOML)? .against_empty() - .await; + .await?; assert_eq!(resp.status(), StatusCode::OK); assert!(to_bytes(resp.into_body()) @@ -75,7 +75,7 @@ async fn object_store_backward_compat() -> TestResult { let resp = Test::using_fixture("kv_store.wasm") .using_fastly_toml(FASTLY_TOML)? .against_empty() - .await; + .await?; assert_eq!(resp.status(), StatusCode::OK); assert!(to_bytes(resp.into_body()) diff --git a/cli/tests/integration/logging.rs b/cli/tests/integration/logging.rs index 15b38678..be4326ec 100644 --- a/cli/tests/integration/logging.rs +++ b/cli/tests/integration/logging.rs @@ -36,7 +36,7 @@ async fn logging_works() -> TestResult { .log_stderr() .log_stdout() .against_empty() - .await; + .await?; assert_eq!(resp.status(), StatusCode::OK); diff --git a/cli/tests/integration/main.rs b/cli/tests/integration/main.rs index b1c88be9..d8020f22 100644 --- a/cli/tests/integration/main.rs +++ b/cli/tests/integration/main.rs @@ -16,6 +16,7 @@ mod response; mod secret_store; mod sending_response; mod sleep; +mod unknown_import_behavior; mod upstream; mod upstream_async; mod upstream_dynamic; diff --git a/cli/tests/integration/memory.rs b/cli/tests/integration/memory.rs index 654a08f2..461adfb7 100644 --- a/cli/tests/integration/memory.rs +++ b/cli/tests/integration/memory.rs @@ -6,7 +6,7 @@ use hyper::{Request, StatusCode}; async fn direct_wasm_works() -> TestResult { let resp = Test::using_wat_fixture("return_ok.wat") .against_empty() - .await; + .await?; assert_eq!(resp.status(), StatusCode::OK); Ok(()) } @@ -22,7 +22,7 @@ async fn heap_limit_test_ok() -> TestResult { .body("") .unwrap(), ) - .await; + .await?; println!("response: {:?}", resp); assert_eq!(resp.status(), StatusCode::OK); assert_eq!(resp.headers().len(), 16); @@ -45,7 +45,7 @@ async fn heap_limit_test_bad() -> TestResult { .body("") .unwrap(), ) - .await; + .await?; assert_eq!(resp.status(), StatusCode::INTERNAL_SERVER_ERROR); Ok(()) } diff --git a/cli/tests/integration/request.rs b/cli/tests/integration/request.rs index f3d736e8..2e856138 100644 --- a/cli/tests/integration/request.rs +++ b/cli/tests/integration/request.rs @@ -5,7 +5,7 @@ use { #[tokio::test(flavor = "multi_thread")] async fn request_works() -> TestResult { - let resp = Test::using_fixture("request.wasm").against_empty().await; + let resp = Test::using_fixture("request.wasm").against_empty().await?; assert_eq!(resp.status(), StatusCode::OK); Ok(()) } diff --git a/cli/tests/integration/response.rs b/cli/tests/integration/response.rs index eaaf911d..a2f044da 100644 --- a/cli/tests/integration/response.rs +++ b/cli/tests/integration/response.rs @@ -5,7 +5,7 @@ use { #[tokio::test(flavor = "multi_thread")] async fn response_works() -> TestResult { - let resp = Test::using_fixture("response.wasm").against_empty().await; + let resp = Test::using_fixture("response.wasm").against_empty().await?; assert_eq!(resp.status(), StatusCode::OK); Ok(()) } diff --git a/cli/tests/integration/secret_store.rs b/cli/tests/integration/secret_store.rs index ebd0473b..b6d8900f 100644 --- a/cli/tests/integration/secret_store.rs +++ b/cli/tests/integration/secret_store.rs @@ -17,7 +17,7 @@ async fn secret_store_works() -> TestResult { let resp = Test::using_fixture("secret-store.wasm") .using_fastly_toml(FASTLY_TOML)? .against_empty() - .await; + .await?; assert_eq!(resp.status(), StatusCode::OK); assert!(to_bytes(resp.into_body()) diff --git a/cli/tests/integration/sending_response.rs b/cli/tests/integration/sending_response.rs index 4846c8db..35d775fd 100644 --- a/cli/tests/integration/sending_response.rs +++ b/cli/tests/integration/sending_response.rs @@ -19,7 +19,7 @@ use { async fn responses_can_be_sent_downstream() -> TestResult { let resp = Test::using_fixture("teapot-status.wasm") .against_empty() - .await; + .await?; assert_eq!(resp.status(), StatusCode::IM_A_TEAPOT); Ok(()) } @@ -32,7 +32,7 @@ async fn responses_can_be_sent_downstream() -> TestResult { /// [ok]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/200 #[tokio::test(flavor = "multi_thread")] async fn empty_ok_response_by_default() -> TestResult { - let resp = Test::using_fixture("noop.wasm").against_empty().await; + let resp = Test::using_fixture("noop.wasm").against_empty().await?; assert_eq!(resp.status(), StatusCode::OK); assert!(to_bytes(resp.into_body()) @@ -50,7 +50,7 @@ async fn empty_ok_response_by_default() -> TestResult { /// [err]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/500 #[tokio::test(flavor = "multi_thread")] async fn five_hundred_when_guest_panics() -> TestResult { - let resp = Test::using_fixture("panic.wasm").against_empty().await; + let resp = Test::using_fixture("panic.wasm").against_empty().await?; assert_eq!(resp.status(), StatusCode::INTERNAL_SERVER_ERROR); Ok(()) } @@ -61,7 +61,7 @@ async fn responses_can_be_streamed_downstream() -> TestResult { let mut resp = Test::using_fixture("streaming-response.wasm") .via_hyper() .against_empty() - .await; + .await?; assert_eq!(resp.status(), StatusCode::OK); assert!(resp diff --git a/cli/tests/integration/sleep.rs b/cli/tests/integration/sleep.rs index 403d5b8f..4c49c591 100644 --- a/cli/tests/integration/sleep.rs +++ b/cli/tests/integration/sleep.rs @@ -6,7 +6,7 @@ use hyper::{body::to_bytes, StatusCode}; /// `sleep.wasm` is a guest program which sleeps for 100 milliseconds,then returns. #[tokio::test(flavor = "multi_thread")] async fn empty_ok_response_by_default_after_sleep() -> TestResult { - let resp = Test::using_fixture("sleep.wasm").against_empty().await; + let resp = Test::using_fixture("sleep.wasm").against_empty().await?; assert_eq!(resp.status(), StatusCode::OK); assert!(to_bytes(resp.into_body()) diff --git a/cli/tests/integration/unknown_import_behavior.rs b/cli/tests/integration/unknown_import_behavior.rs new file mode 100644 index 00000000..a3bb4cbd --- /dev/null +++ b/cli/tests/integration/unknown_import_behavior.rs @@ -0,0 +1,81 @@ +use hyper::{Request, Response, StatusCode}; +use viceroy_lib::config::UnknownImportBehavior; + +use crate::common::{Test, TestResult}; + +/// A test using the default behavior, where the unknown import will fail to link. +#[tokio::test(flavor = "multi_thread")] +async fn default_behavior_link_failure() -> TestResult { + let res = Test::using_fixture("unknown-import.wasm") + .against_empty() + .await; + + let err = res.expect_err("should be a link failure"); + assert!(err + .to_string() + .contains("unknown import: `unknown_module::unknown_function` has not been defined")); + + Ok(()) +} + +/// A test using the trap behavior, where calling the unknown import will cause a runtime trap. +#[tokio::test(flavor = "multi_thread")] +async fn trap_behavior_function_called() -> TestResult { + let resp = Test::using_fixture("unknown-import.wasm") + .using_unknown_import_behavior(UnknownImportBehavior::Trap) + .against(Request::get("/").header("call-it", "yes").body("").unwrap()) + .await?; + + assert_eq!(resp.status(), StatusCode::INTERNAL_SERVER_ERROR); + let body_bytes = hyper::body::to_bytes(resp.into_body()).await?; + let body = std::str::from_utf8(&body_bytes)?; + // The backtrace contains things like stack addresses and gensyms, so we just look for a couple + // key parts that should be relatively stable across invocations and wasmtime + // versions. Fundamentally though, we're still making assertions based on pretty-printed errors, + // so beware of trivial breakages. + assert!(body.contains("error while executing at wasm backtrace")); + assert!(body.contains("unknown_import::main::")); + + Ok(()) +} + +/// A test using the trap behavior, where not calling the function means execution proceeds normally. +#[tokio::test(flavor = "multi_thread")] +async fn trap_behavior_function_not_called() -> TestResult { + let resp = Test::using_fixture("unknown-import.wasm") + .backend("TheOrigin", "/", None, |_req| { + Response::builder() + .status(StatusCode::OK) + .body(vec![]) + .unwrap() + }) + .await + .using_unknown_import_behavior(UnknownImportBehavior::Trap) + .against_empty() + .await?; + + assert_eq!(resp.status(), StatusCode::OK); + + Ok(()) +} + +/// A test using the zero-or-null value behavior, where calling the function returns an expected +/// zero value and execution proceeds normally. +#[tokio::test(flavor = "multi_thread")] +async fn zero_or_null_behavior_function_called() -> TestResult { + let resp = Test::using_fixture("unknown-import.wasm") + .backend("TheOrigin", "/", None, |_req| { + Response::builder() + .status(StatusCode::OK) + .body(vec![]) + .unwrap() + }) + .await + .using_unknown_import_behavior(UnknownImportBehavior::ZeroOrNull) + .against(Request::get("/").header("call-it", "yes").body("").unwrap()) + .await?; + + assert_eq!(resp.status(), StatusCode::OK); + + Ok(()) +} diff --git a/cli/tests/integration/upstream.rs b/cli/tests/integration/upstream.rs index 9e1d0bc4..bb638d59 100644 --- a/cli/tests/integration/upstream.rs +++ b/cli/tests/integration/upstream.rs @@ -43,7 +43,7 @@ async fn upstream_sync() -> TestResult { .body("Hello, Viceroy!") .unwrap(), ) - .await; + .await?; assert_eq!(resp.status(), StatusCode::OK); assert_eq!( resp.into_body().read_into_string().await?, @@ -62,7 +62,7 @@ async fn upstream_sync() -> TestResult { .body("") .unwrap(), ) - .await; + .await?; assert_eq!(resp.status(), StatusCode::OK); assert_eq!(resp.into_body().read_into_string().await?, "/hello/"); @@ -73,7 +73,7 @@ async fn upstream_sync() -> TestResult { .body("") .unwrap(), ) - .await; + .await?; assert_eq!(resp.status(), StatusCode::OK); assert_eq!(resp.into_body().read_into_string().await?, "/hello/"); @@ -84,7 +84,7 @@ async fn upstream_sync() -> TestResult { .body("") .unwrap(), ) - .await; + .await?; assert_eq!(resp.status(), StatusCode::OK); assert_eq!( resp.into_body().read_into_string().await?, @@ -98,7 +98,7 @@ async fn upstream_sync() -> TestResult { .body("") .unwrap(), ) - .await; + .await?; assert_eq!(resp.status(), StatusCode::OK); assert_eq!( resp.into_body().read_into_string().await?, @@ -116,7 +116,7 @@ async fn upstream_sync() -> TestResult { .body("") .unwrap(), ) - .await; + .await?; assert!(resp.status().is_server_error()); Ok(()) @@ -143,7 +143,7 @@ async fn override_host_works() -> TestResult { .body("") .unwrap(), ) - .await; + .await?; assert_eq!(resp.status(), StatusCode::OK); @@ -170,7 +170,7 @@ async fn transparent_gunzip() -> TestResult { }) .await .against_empty() - .await; + .await?; assert_eq!(resp.status(), StatusCode::OK); assert_eq!( diff --git a/cli/tests/integration/upstream_async.rs b/cli/tests/integration/upstream_async.rs index 50ccfd8c..bb21442a 100644 --- a/cli/tests/integration/upstream_async.rs +++ b/cli/tests/integration/upstream_async.rs @@ -43,7 +43,7 @@ async fn upstream_async_methods() -> TestResult { .await; // The meat of the test is on the guest side; we just check that we made it through successfully - let resp = test.against_empty().await; + let resp = test.against_empty().await?; assert_eq!(resp.status(), StatusCode::OK); Ok(()) } diff --git a/cli/tests/integration/upstream_dynamic.rs b/cli/tests/integration/upstream_dynamic.rs index 479e86c9..757f840b 100644 --- a/cli/tests/integration/upstream_dynamic.rs +++ b/cli/tests/integration/upstream_dynamic.rs @@ -36,7 +36,7 @@ async fn upstream_sync() -> TestResult { .body("Hello, Viceroy!") .unwrap(), ) - .await; + .await?; assert_eq!(resp.status(), StatusCode::OK); assert_eq!( resp.into_body().read_into_string().await?, @@ -54,7 +54,7 @@ async fn upstream_sync() -> TestResult { .body("Hello, Viceroy!") .unwrap(), ) - .await; + .await?; assert_eq!(resp.status(), StatusCode::OK); assert_eq!( resp.into_body().read_into_string().await?, @@ -94,7 +94,7 @@ async fn override_host_works() -> TestResult { .body("") .unwrap(), ) - .await; + .await?; assert_eq!(resp.status(), StatusCode::OK); @@ -120,7 +120,7 @@ async fn duplication_errors_right() -> TestResult { .body("") .unwrap(), ) - .await; + .await?; assert_eq!(resp.status(), StatusCode::CONFLICT); @@ -132,7 +132,7 @@ async fn duplication_errors_right() -> TestResult { .body("") .unwrap(), ) - .await; + .await?; assert_eq!(resp.status(), StatusCode::CONFLICT); diff --git a/cli/tests/integration/upstream_streaming.rs b/cli/tests/integration/upstream_streaming.rs index e4e980cc..f274950a 100644 --- a/cli/tests/integration/upstream_streaming.rs +++ b/cli/tests/integration/upstream_streaming.rs @@ -13,7 +13,7 @@ async fn upstream_streaming() -> TestResult { .await; // Test with an empty request - let mut resp = test.against_empty().await; + let mut resp = test.against_empty().await?; assert_eq!(resp.status(), StatusCode::OK); // accumulate the entire body to a vector diff --git a/cli/tests/trap-test/Cargo.lock b/cli/tests/trap-test/Cargo.lock index 4493e734..880894c1 100644 --- a/cli/tests/trap-test/Cargo.lock +++ b/cli/tests/trap-test/Cargo.lock @@ -76,6 +76,54 @@ dependencies = [ "winapi", ] +[[package]] +name = "anstream" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b1f58811cfac344940f1a400b6e6231ce35171f614f26439e80f8c1465c5cc0c" +dependencies = [ + "anstyle", + "anstyle-parse", + "anstyle-query", + "anstyle-wincon", + "colorchoice", + "utf8parse", +] + +[[package]] +name = "anstyle" +version = "1.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b84bf0a05bbb2a83e5eb6fa36bb6e87baa08193c35ff52bbf6b38d8af2890e46" + +[[package]] +name = "anstyle-parse" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "938874ff5980b03a87c5524b3ae5b59cf99b1d6bc836848df7bc5ada9643c333" +dependencies = [ + "utf8parse", +] + +[[package]] +name = "anstyle-query" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5ca11d4be1bab0c8bc8734a9aa7bf4ee8316d462a08c6ac5052f888fef5b494b" +dependencies = [ + "windows-sys", +] + +[[package]] +name = "anstyle-wincon" +version = "2.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "58f54d10c6dfa51283a066ceab3ec1ab78d13fae00aa49243a45e4571fb79dfd" +dependencies = [ + "anstyle", + "windows-sys", +] + [[package]] name = "anyhow" version = "1.0.75" @@ -282,6 +330,52 @@ dependencies = [ "windows-targets", ] +[[package]] +name = "clap" +version = "4.4.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b1d7b8d5ec32af0fadc644bf1fd509a688c2103b185644bb1e29d164e0703136" +dependencies = [ + "clap_builder", + "clap_derive", +] + +[[package]] +name = "clap_builder" +version = "4.4.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5179bb514e4d7c2051749d8fcefa2ed6d06a9f4e6d69faf3805f5d80b8cf8d56" +dependencies = [ + "anstream", + "anstyle", + "clap_lex", + "strsim", +] + +[[package]] +name = "clap_derive" +version = "4.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0862016ff20d69b84ef8247369fabf5c008a7417002411897d40ee1f4532b873" +dependencies = [ + "heck", + "proc-macro2", + "quote", + "syn 2.0.35", +] + +[[package]] +name = "clap_lex" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cd7cc57abe963c6d3b9d8be5b06ba7c8957a930305ca90304f24ef040aa6f961" + +[[package]] +name = "colorchoice" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "acbf1af155f9b9ef647e42cdc158db4b64a1b61f743629225fde6f3e0be2a7c7" + [[package]] name = "core-foundation" version = "0.9.3" @@ -1934,6 +2028,12 @@ version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a8f112729512f8e442d81f95a8a7ddf2b7c6b8a1a6f509a95864142b30cab2d3" +[[package]] +name = "strsim" +version = "0.10.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "73473c0e59e6d5812c5dfe2a064a6444949f089e20eec9a2e5506596494e4623" + [[package]] name = "syn" version = "1.0.109" @@ -2272,6 +2372,12 @@ dependencies = [ "percent-encoding", ] +[[package]] +name = "utf8parse" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "711b9620af191e0cdc7468a8d14e709c3dcdb115b36f838e601583af800a370a" + [[package]] name = "uuid" version = "1.4.1" @@ -2298,6 +2404,7 @@ dependencies = [ "bytes", "bytesize", "cfg-if", + "clap", "cranelift-entity 0.88.2", "fastly-shared", "flate2", diff --git a/cli/tests/trap-test/src/main.rs b/cli/tests/trap-test/src/main.rs index bd6093fa..1023106c 100644 --- a/cli/tests/trap-test/src/main.rs +++ b/cli/tests/trap-test/src/main.rs @@ -16,7 +16,13 @@ pub type TestResult = Result<(), Error>; #[tokio::test(flavor = "multi_thread")] async fn fatal_error_traps() -> TestResult { let module_path = format!("{RUST_FIXTURE_PATH}/response.wasm"); - let ctx = ExecuteCtx::new(module_path, ProfilingStrategy::None, HashSet::new(), None)?; + let ctx = ExecuteCtx::new( + module_path, + ProfilingStrategy::None, + HashSet::new(), + None, + viceroy_lib::config::UnknownImportBehavior::LinkError, + )?; let req = Request::get("http://127.0.0.1:7676/").body(Body::from(""))?; let resp = ctx .handle_request_with_runtime_error(req, "127.0.0.1".parse().unwrap()) diff --git a/lib/Cargo.toml b/lib/Cargo.toml index d854b54d..8b4b9988 100644 --- a/lib/Cargo.toml +++ b/lib/Cargo.toml @@ -27,6 +27,7 @@ anyhow = { workspace = true } bytes = "^1.2.1" bytesize = "^1.1.0" cfg-if = "^1.0" +clap = { workspace = true } cranelift-entity = "^0.88.1" fastly-shared = "^0.9.3" flate2 = "^1.0.24" diff --git a/lib/src/config.rs b/lib/src/config.rs index 82a8cc38..fb87c0df 100644 --- a/lib/src/config.rs +++ b/lib/src/config.rs @@ -254,3 +254,14 @@ impl TryInto for RawLocalServerConfig { }) } } + +#[derive(Debug, Default, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, clap::ValueEnum, Hash)] +pub enum UnknownImportBehavior { + /// Unknown imports are rejected at link time (default behavior) + #[default] + LinkError, + /// Unknown imports trap when called + Trap, + /// Unknown imports return zero or a null pointer, depending on the type + ZeroOrNull, +} diff --git a/lib/src/execute.rs b/lib/src/execute.rs index 66333fe3..1546055c 100644 --- a/lib/src/execute.rs +++ b/lib/src/execute.rs @@ -4,6 +4,8 @@ use std::time::SystemTime; use wasmtime::GuestProfiler; +use crate::config::UnknownImportBehavior; + use { crate::{ body::Body, @@ -83,12 +85,20 @@ impl ExecuteCtx { profiling_strategy: ProfilingStrategy, wasi_modules: HashSet, guest_profile_path: Option, + unknown_import_behavior: UnknownImportBehavior, ) -> Result { let config = &configure_wasmtime(profiling_strategy); let engine = Engine::new(config)?; let mut linker = Linker::new(&engine); link_host_functions(&mut linker, &wasi_modules)?; let module = Module::from_file(&engine, module_path)?; + match unknown_import_behavior { + UnknownImportBehavior::LinkError => (), + UnknownImportBehavior::Trap => linker.define_unknown_imports_as_traps(&module)?, + UnknownImportBehavior::ZeroOrNull => { + linker.define_unknown_imports_as_default_values(&module)? + } + } let instance_pre = linker.instantiate_pre(&module)?; // Create the epoch-increment thread. @@ -225,7 +235,7 @@ impl ExecuteCtx { /// # use viceroy_lib::{Error, ExecuteCtx, ProfilingStrategy, ViceroyService}; /// # async fn f() -> Result<(), Error> { /// # let req = Request::new(Body::from("")); - /// let ctx = ExecuteCtx::new("path/to/a/file.wasm", ProfilingStrategy::None, HashSet::new(), None)?; + /// let ctx = ExecuteCtx::new("path/to/a/file.wasm", ProfilingStrategy::None, HashSet::new(), None, Default::default())?; /// let resp = ctx.handle_request(req, "127.0.0.1".parse().unwrap()).await?; /// # Ok(()) /// # } diff --git a/lib/src/service.rs b/lib/src/service.rs index 6478cfe3..eb9201be 100644 --- a/lib/src/service.rs +++ b/lib/src/service.rs @@ -43,7 +43,7 @@ impl ViceroyService { /// # use std::collections::HashSet; /// use viceroy_lib::{Error, ExecuteCtx, ProfilingStrategy, ViceroyService}; /// # fn f() -> Result<(), Error> { - /// let ctx = ExecuteCtx::new("path/to/a/file.wasm", ProfilingStrategy::None, HashSet::new(), None)?; + /// let ctx = ExecuteCtx::new("path/to/a/file.wasm", ProfilingStrategy::None, HashSet::new(), None, Default::default())?; /// let svc = ViceroyService::new(ctx); /// # Ok(()) /// # } diff --git a/test-fixtures/src/bin/unknown-import.rs b/test-fixtures/src/bin/unknown-import.rs new file mode 100644 index 00000000..3fa0110c --- /dev/null +++ b/test-fixtures/src/bin/unknown-import.rs @@ -0,0 +1,28 @@ +use fastly::{Request, Response}; +use std::os::raw::{c_float, c_int}; + +/// This test fixture forwards the client request to a backend after calling a custom imported +/// function that is not defined by Viceroy. +fn main() { + let client_req = Request::from_client(); + + if client_req.contains_header("call-it") { + let unknown_result = unsafe { unknown_function(42, 120.0) }; + // With the default mode, we don't even end up running this program. In trapping mode, we don't + // make it past the function call above. It's only in "default value" mode that we make it here, + // where the answer should be zero. + assert_eq!(unknown_result, 0); + } + + // Forward the request to the given backend + client_req + .send("TheOrigin") + .unwrap_or_else(|_| Response::from_status(500)) + .send_to_client(); +} + +#[link(wasm_import_module = "unknown_module")] +extern "C" { + #[link_name = "unknown_function"] + pub fn unknown_function(arg1: c_int, arg2: c_float) -> c_int; +}