diff options
| -rw-r--r-- | .gitignore | 1 | ||||
| -rw-r--r-- | Cargo.lock | 9 | ||||
| -rw-r--r-- | crates/arti-rpc-client-core/Cargo.toml | 6 | ||||
| -rw-r--r-- | crates/arti-rpc-client-core/README.md | 1 | ||||
| -rw-r--r-- | crates/arti-rpc-client-core/arti-rpc-client-core.h | 410 | ||||
| -rw-r--r-- | crates/arti-rpc-client-core/cbindgen.toml | 178 | ||||
| -rw-r--r-- | crates/arti-rpc-client-core/cbindgen.warnings | 52 | ||||
| -rw-r--r-- | crates/arti-rpc-client-core/src/conn.rs | 30 | ||||
| -rw-r--r-- | crates/arti-rpc-client-core/src/ffi.rs | 174 | ||||
| -rw-r--r-- | crates/arti-rpc-client-core/src/ffi/err.rs | 436 | ||||
| -rw-r--r-- | crates/arti-rpc-client-core/src/ffi/util.rs | 592 | ||||
| -rw-r--r-- | crates/arti-rpc-client-core/src/lib.rs | 7 | ||||
| -rw-r--r-- | crates/arti-rpc-client-core/src/msgs/response.rs | 30 | ||||
| -rw-r--r-- | crates/arti-rpc-client-core/src/util.rs | 60 | ||||
| -rwxr-xr-x | maint/cbindgen | 172 | ||||
| -rwxr-xr-x | maint/list_crates | 10 |
16 files changed, 2137 insertions, 31 deletions
diff --git a/.gitignore b/.gitignore index ba4b47ece..85e02c3ed 100644 --- a/.gitignore +++ b/.gitignore @@ -9,6 +9,7 @@ /crates/*/fuzz/Cargo.lock /crates/*/fuzz-*.log *~ +*.tmp .#* \#*\# .cargo diff --git a/Cargo.lock b/Cargo.lock index eb78f8d41..8e5269df5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -332,9 +332,11 @@ version = "0.20.0" name = "arti-rpc-client-core" version = "0.19.0" dependencies = [ + "c_str_macro", "caret", "derive_more", "educe", + "paste", "rand", "rand_chacha", "serde", @@ -342,6 +344,7 @@ dependencies = [ "socketpair", "thiserror", "tor-basic-utils", + "void", ] [[package]] @@ -911,6 +914,12 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a12916984aab3fa6e39d655a33e09c0071eb36d6ab3aea5c2d78551f1df6d952" [[package]] +name = "c_str_macro" +version = "1.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c6d44951c469019e225e7667d799052f67fb8ea358d086878f3582b39f0de5e5" + +[[package]] name = "caret" version = "0.4.6" diff --git a/crates/arti-rpc-client-core/Cargo.toml b/crates/arti-rpc-client-core/Cargo.toml index 9a9da51ac..897d52761 100644 --- a/crates/arti-rpc-client-core/Cargo.toml +++ b/crates/arti-rpc-client-core/Cargo.toml @@ -14,12 +14,15 @@ repository = "https://gitlab.torproject.org/tpo/core/arti.git/" [dependencies] +c_str_macro = { version = "1", optional = true } caret = { path = "../caret", version = "0.4.5" } derive_more = "0.99.3" educe = "0.4.6" +paste = { version = "1", optional = true } serde = { version = "1.0.103", features = ["derive"] } serde_json = "1.0.104" thiserror = "1" +void = "1" [dev-dependencies] rand = "0.8" @@ -28,7 +31,8 @@ socketpair = "0.19" tor-basic-utils = { path = "../tor-basic-utils", version = "0.20.0" } [features] -full = [] +full = ["ffi"] +ffi = ["c_str_macro", "paste"] [package.metadata.docs.rs] all-features = true diff --git a/crates/arti-rpc-client-core/README.md b/crates/arti-rpc-client-core/README.md index a2fc0773f..5898a8cef 100644 --- a/crates/arti-rpc-client-core/README.md +++ b/crates/arti-rpc-client-core/README.md @@ -22,6 +22,7 @@ Notes so far: * [ ] More tests. * [ ] update this readme. * [x] interface for connecting to arti + * [x] Initial C FFI wrappers. * [ ] C FFI wrappers for everything reasonable * [x] enable the usual warnings. * [ ] Finish this readme. diff --git a/crates/arti-rpc-client-core/arti-rpc-client-core.h b/crates/arti-rpc-client-core/arti-rpc-client-core.h new file mode 100644 index 000000000..d7bb581a4 --- /dev/null +++ b/crates/arti-rpc-client-core/arti-rpc-client-core.h @@ -0,0 +1,410 @@ +/** + * # Arti RPC core library header. + * + * (TODO RPC: This is still a work in progress; please don't rely on it + * being the final API.) + * + * ## What this library does + * + * The Arti RPC system works by establishing connections to an Arti instance, + * and then exchanging requests and replies in a format inspired by + * JSON-RPC. This library takes care of the work of connecting to an Arti + * instance, authenticating, validating outgoing JSON requests, and matching + * their corresponding JSON responses as they arrive. + * + * This library _does not_ do the work of creating well-formed requests, + * or interpreting the responses. + * + * (Note: Despite this library being exposed via a set of C functions, + * we don't actually expect you to use it from C. It's probably a better + * idea to wrap it in a higher-level language and then use it from there.) + * + * ## Using this library + * + * TODO RPC Explain better. + * + * Your connection to Arti is represented by an `ArtiRpcConn *`. Use + * `arti_rpc_connect()` to create one of these. + * + * Once you have a connection, you can sent Arti various requests in + * JSON format. See (TODO RPC: Add a link to a list of comments.) + * Use `arti_rpc_execute()` to send a simple request; the function will + * return when the request succeeds, or fails. + * + * TODO: Explain handles and other APIs once I add those APIs. + * + * Except when noted otherwise, all functions in this library are thread-safe. + * + * ## Error handling + * + * On success, fallible functions return `ARTI_RPC_STATUS_SUCCESS`. On failure, + * they return some other error code, and set an `* error_out` parameter + * to a newly allocated `ArtiRpcError` object. + * (If `error_out==NULL`, then no error is allocated.) + * + * You can access information about the an `ArtiRpcError` + * by calling `arti_rpc_err_{status,message,response}()` on it. + * When you are done with an error, you should free it with + * `arti_rpc_err_free()`. + * + * The `error_out` parameter always appears last. + * + * ## Interface conventions + * + * - All functions check for NULL pointers in their arguments. + * - As in C tor, `foo_free()` functions treat `foo_free(NULL)` as a no-op. + * + * - All input strings should be valid UTF-8. (The library will check.) + * All output strings will be valid UTF-8. + * + * - Fallible functions return an ArtiStatus. + * + * - All identifiers are prefixed with `ARTI_RPC`, `ArtiRpc`, or `arti_rpc` as appropriate. + * + * - Newly allocated objects are returned via out-parameters, + * with `out` in their names. + * (For example, `ArtiRpcObject **out`). In such cases, `* out` will be set to a resulting object, + * or to NULL if no such object is returned. Any earlier value of `*out` will be replaced + * without freeing it. + * (If `out` is NULL, then any object the library would have returned will instead be discareded.) + * discarded. + * While the function is running, + * `*out` and `**out` may not be read or written by any other part of the program, + * and they may not alias any other arguments.) + * - Note that `*out` will be set to NULL if an error occurs + * or the function's inputs are invalid. + * (The `*error_out` parameter, of course, + * is set to NULL when there is _no_ error, and to an error otherwise.) + * + * - When any object is exposed as a non-const pointer, + * the application becomes the owner of that object. + * The application is expected to eventually free that object via the corresponding `arti_rpc_*_free()` function. + * + * - When any object is exposed via a const pointer, + * that object is *not* owned by the application. + * That object's lifetime will be as documented. + * The application must not modify or free such an object. + * + * - If a function should be considered a method on a given type of object, + * it will take a pointer to that object as its first argument. + * + * - If a function consumes (takes ownership of) one of its inputs, + * it does so regardless of whether the function succeeds or fails. + * + * ## Correctness requirements + * + * If any correctness requirements stated here or elsewhere are violated, + * it is Undefined Behaviour. + * Violations will not be detected by the library. + * + * - Basic C rules apply: + * - If you pass a non-NULL pointer to a function, the pointer must be properly aligned. + * It must point to valid, initialized data of the correct type. + * - As an exception, functions that take a `Type **out` parameter allow the value of `*out` + * (but not `out` itself!) to be uninitialized. + * - If you receive data via a `const *`, you must not modify that data. + * - If you receive a pointer of type `struct Type *`, + * and we do not give you the definition of `struct Type`, + * you must not attempt to dereference the pointer. + * - You may not call any `_free()` function on an object that is currently in use. + * - After you have `_freed()` an object, you may not use it again. + * - Every object allocated by this library has a corresponding `*_free()` function: + * You must not use libc's free() to free such objects. + * - All objects passed as input to a library function must not be mutated + * while that function is running. + * - All objects passed as input to a library function via a non-const pointer + * must not be mutated, inspected, or passed to another library function + * while the function is running. + * - Furthermore, if a function takes any non-const pointer arguments, + * those arguments must not alias one another, + * and must not alias any const arguments passed to the function. + * - All `const char*` passed as inputs to library functions + * are nul-terminated strings. + * Additionally, they must be no larger than `SSIZE_MAX`, + including the nul. + * - If a function takes any mutable pointers + **/ + +#ifndef ARTI_RPC_CLIENT_CORE_H_ +#define ARTI_RPC_CLIENT_CORE_H_ + +/* Automatically generated by cbindgen. Don't modify manually. */ + +#include <stdarg.h> +#include <stdbool.h> +#include <stdint.h> +#include <stdlib.h> + +/** + * A string that is guaranteed to be UTF-8 and NUL-terminated, + * for fast access as either type. + */ +typedef struct Utf8CString Utf8CString; + +/** + * A status code returned by an Arti RPC function. + * + * On success, a function will return `ARTI_SUCCESS (0)`. + * On failure, a function will return some other status code. + */ +typedef uint32_t ArtiRpcStatus; + +/** + * An open connection to Arti over an a RPC protocol. + * + * This is a thread-safe type: you may safely use it from multiple threads at once. + * + * Once you are no longer going to use this connection at all, you must free + * it with [`arti_rpc_conn_free`] + */ +typedef struct ArtiRpcConn ArtiRpcConn; + +/** + * An error returned by the Arti RPC code, exposed as an object. + * + * When a function returns an [`ArtiRpcStatus`] other than [`ARTI_RPC_STATUS_SUCCESS`], + * it will also expose a newly allocated value of this type + * via its `error_out` parameter. + */ +typedef struct ArtiRpcError ArtiRpcError; + +/** + * An owned string, returned by this library. + * + * This string must be released with `arti_rpc_str_free`. + * You can inspect it with `arti_rpc_str_get`, but you may not modify it. + * The string is guaranteed to be UTF-8 and NUL-terminated. + */ +typedef struct Utf8CString ArtiRpcStr; + +/** + * The function has returned successfully. + */ +#define ARTI_RPC_STATUS_SUCCESS 0 + +/** + * One or more of the inputs to a library function was invalid. + * + * (This error was generated by the library, before any request was sent.) + */ +#define ARTI_RPC_STATUS_INVALID_INPUT 1 + +/** + * Tried to use some functionality + * (for example, an authentication method or connection scheme) + * that wasn't available on this platform or build. + * + * (This error was generated by the library, before any request was sent.) + */ +#define ARTI_RPC_STATUS_NOT_SUPPORTED 2 + +/** + * Tried to connect to Arti, but an IO error occurred. + * + * This may indicate that Arti wasn't running, + * or that Arti was built without RPC support, + * or that Arti wasn't running at the specified location. + * + * (This error was generated by the library.) + */ +#define ARTI_RPC_STATUS_CONNECT_IO 3 + +/** + * We tried to authenticate with Arti, but it rejected our attempt. + * + * (This error was sent by the peer.) + */ +#define ARTI_RPC_STATUS_BAD_AUTH 4 + +/** + * Our peer has, in some way, violated the Arti-RPC protocol. + * + * (This error was generated by the library, + * based on a response from Arti that appeared to be invalid.) + */ +#define ARTI_RPC_STATUS_PEER_PROTOCOL_VIOLATION 5 + +/** + * The peer has closed our connection; possibly because it is shutting down. + * + * (This error was generated by the library, + * based on the connection being closed or reset from the peer.) + */ +#define ARTI_RPC_STATUS_SHUTDOWN 6 + +/** + * An internal error occurred in the arti rpc client. + * + * (This error was generated by the library. + * If you see it, there is probably a bug in the library.) + */ +#define ARTI_RPC_STATUS_INTERNAL 7 + +/** + * The peer reports that one of our requests has failed. + * + * (This error was sent by the peer, in response to one of our requests. + * No further responses to that request will be received or accepted.) + */ +#define ARTI_RPC_STATUS_REQUEST_FAILED 8 + +/** + * Tried to check the status of a request and found that it was no longer running. + * + * TODO RPC: We should make sure that this is the actual semantics we want for this + * error! Revisit after we have implemented real cancellation. + */ +#define ARTI_RPC_STATUS_REQUEST_CANCELLED 9 + + + + + + + + + + + + + + + +#ifdef __cplusplus +extern "C" { +#endif // __cplusplus + +/** + * Try to open a new connection to an Arti instance. + * + * The location of the instance and the method to connect to it are described in + * `connection_string`. + * + * (TODO RPC: Document the format of this string better!) + * + * On success, return `ARTI_RPC_STATUS_SUCCESS` and set `*rpc_conn_out` to a new ArtiRpcConn. + * Otherwise return some other status code, set `*rpc_conn_out` to NULL, and set + * `*error_out` (if provided) to a newly allocated error object. + * + * + * # Ownership + * + * The caller is responsible for making sure that `*rpc_conn_out` and `*error_out`, + * if set, are eventually freed. + */ +ArtiRpcStatus arti_rpc_connect(const char *connection_string, + ArtiRpcConn **rpc_conn_out, + ArtiRpcError **error_out); + +/** + * Run an RPC request over `rpc_conn` and wait for a successful response. + * + * The message `msg` should be a valid RPC request in JSON format. + * If you omit its `id` field, one will be generated: this is typically the best way to use this function. + * + * On success, return `ARTI_RPC_STATUS_SUCCESS` and set `*response_out` to a newly allocated string + * containing the JSON response to your request (including `id` and `response` fields). + * + * Otherwise return some other status code, set `*response_out` to NULL, + * and set `*error_out` (if provided) to a newly allocated error object. + * + * (If response_out is set to NULL, then any successful response will be ignored.) + * + * # Ownership + * + * The caller is responsible for making sure that `*error_out`, if set, is eventually freed. + */ +ArtiRpcStatus arti_rpc_conn_execute(const ArtiRpcConn *rpc_conn, + const char *msg, + ArtiRpcStr **response_out, + ArtiRpcError **error_out); + +/** + * Free a string returned by the Arti RPC API. + */ +void arti_rpc_str_free(ArtiRpcStr *string); + +/** + * Return a const pointer to the underlying nul-terminated string from an `ArtiRpcStr`. + * + * The resulting string is guaranteed to be valid UTF-8. + * + * (Returns NULL if the input is NULL.) + * + * # Correctness requirements + * + * The resulting string pointer is valid only for as long as the input `string` is not freed. + */ +const char *arti_rpc_str_get(const ArtiRpcStr *string); + +/** + * Close and free an open Arti RPC connection. + */ +void arti_rpc_conn_free(ArtiRpcConn *rpc_conn); + +/** + * Return a string representing the meaning of a given `ArtiRpcStatus`. + * + * The result will always be non-NULL, even if the status is unrecognized. + */ +const char *arti_status_to_str(ArtiRpcStatus status); + +/** + * Return the status code associated with a given error. + * + * If `err` is NULL, return [`ARTI_RPC_STATUS_INVALID_INPUT`]. + */ +ArtiRpcStatus arti_rpc_err_status(const ArtiRpcError *err); + +/** + * Return a human-readable error message associated with a given error. + * + * The format of these messages may change arbitrarily between versions of this library; + * it is a mistake to depend on the actual contents of this message. + * + * Return NULL if the input `err` is NULL. + * + * # Correctness requirements + * + * The resulting string pointer is valid only for as long as the input `err` is not freed. + */ +const char *arti_rpc_err_message(const ArtiRpcError *err); + +/** + * Return a Json-formatted error response associated with a given error. + * + * These messages are full responses, including the `error` field, + * and the `id` field (if present). + * + * Return NULL if the specified error does not represent an RPC error response. + * + * Return NULL if the input `err` is NULL. + * + * # Correctness requirements + * + * The resulting string pointer is valid only for as long as the input `err` is not freed. + */ +const char *arti_rpc_err_response(const ArtiRpcError *err); + +/** + * Make and return copy of a provided error. + * + * Return NULL if the input is NULL. + * + * # Ownership + * + * The caller is responsible for making sure that the returned object + * is eventually freed with `arti_rpc_err_free()`. + */ +ArtiRpcError *arti_rpc_err_clone(const ArtiRpcError *err); + +/** + * Release storage held by a provided error. + */ +void arti_rpc_err_free(ArtiRpcError *err); + +#ifdef __cplusplus +} // extern "C" +#endif // __cplusplus + +#endif /* ARTI_RPC_CLIENT_CORE_H_ */ diff --git a/crates/arti-rpc-client-core/cbindgen.toml b/crates/arti-rpc-client-core/cbindgen.toml new file mode 100644 index 000000000..b55db210f --- /dev/null +++ b/crates/arti-rpc-client-core/cbindgen.toml @@ -0,0 +1,178 @@ + +# We emit a C header by default. +language = "C" + +# We use this macro to prevent double-includes of our header. +include_guard = "ARTI_RPC_CLIENT_CORE_H_" + +# This appears at the top of the file. +header = """\ +/** + * # Arti RPC core library header. + * + * (TODO RPC: This is still a work in progress; please don't rely on it + * being the final API.) + * + * ## What this library does + * + * The Arti RPC system works by establishing connections to an Arti instance, + * and then exchanging requests and replies in a format inspired by + * JSON-RPC. This library takes care of the work of connecting to an Arti + * instance, authenticating, validating outgoing JSON requests, and matching + * their corresponding JSON responses as they arrive. + * + * This library _does not_ do the work of creating well-formed requests, + * or interpreting the responses. + * + * (Note: Despite this library being exposed via a set of C functions, + * we don't actually expect you to use it from C. It's probably a better + * idea to wrap it in a higher-level language and then use it from there.) + * + * ## Using this library + * + * TODO RPC Explain better. + * + * Your connection to Arti is represented by an `ArtiRpcConn *`. Use + * `arti_rpc_connect()` to create one of these. + * + * Once you have a connection, you can sent Arti various requests in + * JSON format. See (TODO RPC: Add a link to a list of comments.) + * Use `arti_rpc_execute()` to send a simple request; the function will + * return when the request succeeds, or fails. + * + * TODO: Explain handles and other APIs once I add those APIs. + * + * Except when noted otherwise, all functions in this library are thread-safe. + * + * ## Error handling + * + * On success, fallible functions return `ARTI_RPC_STATUS_SUCCESS`. On failure, + * they return some other error code, and set an `* error_out` parameter + * to a newly allocated `ArtiRpcError` object. + * (If `error_out==NULL`, then no error is allocated.) + * + * You can access information about the an `ArtiRpcError` + * by calling `arti_rpc_err_{status,message,response}()` on it. + * When you are done with an error, you should free it with + * `arti_rpc_err_free()`. + * + * The `error_out` parameter always appears last. + * + * ## Interface conventions + * + * - All functions check for NULL pointers in their arguments. + * - As in C tor, `foo_free()` functions treat `foo_free(NULL)` as a no-op. + * + * - All input strings should be valid UTF-8. (The library will check.) + * All output strings will be valid UTF-8. + * + * - Fallible functions return an ArtiStatus. + * + * - All identifiers are prefixed with `ARTI_RPC`, `ArtiRpc`, or `arti_rpc` as appropriate. + * + * - Newly allocated objects are returned via out-parameters, + * with `out` in their names. + * (For example, `ArtiRpcObject **out`). In such cases, `* out` will be set to a resulting object, + * or to NULL if no such object is returned. Any earlier value of `*out` will be replaced + * without freeing it. + * (If `out` is NULL, then any object the library would have returned will instead be discareded.) + * discarded. + * While the function is running, + * `*out` and `**out` may not be read or written by any other part of the program, + * and they may not alias any other arguments.) + * - Note that `*out` will be set to NULL if an error occurs + * or the function's inputs are invalid. + * (The `*error_out` parameter, of course, + * is set to NULL when there is _no_ error, and to an error otherwise.) + * + * - When any object is exposed as a non-const pointer, + * the application becomes the owner of that object. + * The application is expected to eventually free that object via the corresponding `arti_rpc_*_free()` function. + * + * - When any object is exposed via a const pointer, + * that object is *not* owned by the application. + * That object's lifetime will be as documented. + * The application must not modify or free such an object. + * + * - If a function should be considered a method on a given type of object, + * it will take a pointer to that object as its first argument. + * + * - If a function consumes (takes ownership of) one of its inputs, + * it does so regardless of whether the function succeeds or fails. + * + * ## Correctness requirements + * + * If any correctness requirements stated here or elsewhere are violated, + * it is Undefined Behaviour. + * Violations will not be detected by the library. + * + * - Basic C rules apply: + * - If you pass a non-NULL pointer to a function, the pointer must be properly aligned. + * It must point to valid, initialized data of the correct type. + * - As an exception, functions that take a `Type **out` parameter allow the value of `*out` + * (but not `out` itself!) to be uninitialized. + * - If you receive data via a `const *`, you must not modify that data. + * - If you receive a pointer of type `struct Type *`, + * and we do not give you the definition of `struct Type`, + * you must not attempt to dereference the pointer. + * - You may not call any `_free()` function on an object that is currently in use. + * - After you have `_freed()` an object, you may not use it again. + * - Every object allocated by this library has a corresponding `*_free()` function: + * You must not use libc's free() to free such objects. + * - All objects passed as input to a library function must not be mutated + * while that function is running. + * - All objects passed as input to a library function via a non-const pointer + * must not be mutated, inspected, or passed to another library function + * while the function is running. + * - Furthermore, if a function takes any non-const pointer arguments, + * those arguments must not alias one another, + * and must not alias any const arguments passed to the function. + * - All `const char*` passed as inputs to library functions + * are nul-terminated strings. + * Additionally, they must be no larger than `SSIZE_MAX`, + including the nul. + * - If a function takes any mutable pointers + **/""" + +# This appears "between major sections" +autogen_warning = "/* Automatically generated by cbindgen. Don't modify manually. */" + +# make sure our header can be included in C++. +cpp_compat = true + +# Consistency with Arti. +tab_width = 8 + +[defines] +# This is where we would add mappings from `cfg()` to `#ifdef`. +# But the only relevant cfg we have is `cfg(feature="ffi")`, +# which we want to assume is always present if you're using the header. + +[export] +# These structs are not ones we want to expose under their actual names, +# or ones that we don't want to expose at all. +exclude = ["RpcErrorCode", "RpcConn", "FfiError", "Utf8CStr"] + +[export.rename] +# Having not declared these structs, we can give them new names in the +# typedefs that assign them their real names. +"RpcConn" = "struct ArtiRpcConn" +"FfiError" = "struct ArtiRpcError" +"Utf8CStr" = "struct ArtiRpcStr" + +[fn] +# Lay out one argument per line. +args = "vertical" + +[parse] + +[parse.expand] +# We need to run our crate through macro expansion in order to get all +# of the right functions and constants. +# +# (Unfortunately, this requires us to use nightly rust, so that cbindgen +# can invoke rustc with `-Zunpretty=expanded`.) +crates = ["arti-rpc-client-core"] + +# Run macro-expansion with --all-features so that we see `ffi`. +all_features = true diff --git a/crates/arti-rpc-client-core/cbindgen.warnings b/crates/arti-rpc-client-core/cbindgen.warnings new file mode 100644 index 000000000..4d6df5252 --- /dev/null +++ b/crates/arti-rpc-client-core/cbindgen.warnings @@ -0,0 +1,52 @@ +WARN: Skip arti-rpc-client-core::_ - (not `pub`). +WARN: Skip arti-rpc-client-core::_ - (not `pub`). +WARN: Skip arti-rpc-client-core::_ - (not `pub`). +WARN: Skip arti-rpc-client-core::_ - (not `pub`). +WARN: Skip arti-rpc-client-core::_ - (not `pub`). +WARN: Skip arti-rpc-client-core::_ - (not `pub`). +WARN: Skip arti-rpc-client-core::_ - (not `pub`). +WARN: Skip arti-rpc-client-core::_ - (not `pub`). +WARN: Skip arti-rpc-client-core::_ - (not `pub`). +WARN: Skip arti-rpc-client-core::_ - (not `pub`). +WARN: Skip arti-rpc-client-core::_ - (not `pub`). +WARN: Skip arti-rpc-client-core::_ - (not `pub`). +WARN: Skip arti-rpc-client-core::_ - (not `pub`). +WARN: Skip arti-rpc-client-core::_ - (not `pub`). +WARN: Skip arti-rpc-client-core::_ - (not `pub`). +WARN: Skip arti-rpc-client-core::_ - (not `pub`). +WARN: Skip arti-rpc-client-core::_ - (not `pub`). +WARN: Cannot find a mangling for generic path GenericPath { path: Path { name: "Map" }, export_name: "Map", generics: [Type(Path(GenericPath { path: Path { name: "String" }, export_name: "String", generics: [], ctype: None })), Type(Path(GenericPath { path: Path { name: "Value" }, export_name: "Value", generics: [], ctype: None }))], ctype: None }. This usually means that a type referenced by this generic was incompatible or not found. +WARN: Can't find RpcConn. This usually means that this type was incompatible or not found. +WARN: Can't find FfiError. This usually means that this type was incompatible or not found. +WARN: Can't find RpcErrorCode. This usually means that this type was incompatible or not found. +WARN: Can't find RpcErrorCode. This usually means that this type was incompatible or not found. +WARN: Can't find RpcErrorCode. This usually means that this type was incompatible or not found. +WARN: Can't find RpcErrorCode. This usually means that this type was incompatible or not found. +WARN: Can't find RpcErrorCode. This usually means that this type was incompatible or not found. +WARN: Can't find RpcErrorCode. This usually means that this type was incompatible or not found. +WARN: Can't find RpcErrorCode. This usually means that this type was incompatible or not found. +WARN: Missing `[defines]` entry for `feature = "ffi"` in cbindgen config. +WARN: Missing `[defines]` entry for `feature = "ffi"` in cbindgen config. +WARN: Missing `[defines]` entry for `feature = "ffi"` in cbindgen config. +WARN: Missing `[defines]` entry for `feature = "ffi"` in cbindgen config. +WARN: Missing `[defines]` entry for `feature = "ffi"` in cbindgen config. +WARN: Missing `[defines]` entry for `feature = "ffi"` in cbindgen config. +WARN: Missing `[defines]` entry for `feature = "ffi"` in cbindgen config. +WARN: Missing `[defines]` entry for `feature = "ffi"` in cbindgen config. +WARN: Missing `[defines]` entry for `feature = "ffi"` in cbindgen config. +WARN: Missing `[defines]` entry for `feature = "ffi"` in cbindgen config. +WARN: Missing `[defines]` entry for `feature = "ffi"` in cbindgen config. +WARN: Missing `[defines]` entry for `feature = "ffi"` in cbindgen config. +WARN: Missing `[defines]` entry for `feature = "ffi"` in cbindgen config. +WARN: Missing `[defines]` entry for `feature = "ffi"` in cbindgen config. +WARN: Missing `[defines]` entry for `feature = "ffi"` in cbindgen config. +WARN: Missing `[defines]` entry for `feature = "ffi"` in cbindgen config. +WARN: Missing `[defines]` entry for `feature = "ffi"` in cbindgen config. +WARN: Missing `[defines]` entry for `feature = "ffi"` in cbindgen config. +WARN: Missing `[defines]` entry for `feature = "ffi"` in cbindgen config. +WARN: Missing `[defines]` entry for `feature = "ffi"` in cbindgen config. +WARN: Missing `[defines]` entry for `feature = "ffi"` in cbindgen config. +WARN: Missing `[defines]` entry for `feature = "ffi"` in cbindgen config. +WARN: Missing `[defines]` entry for `feature = "ffi"` in cbindgen config. +WARN: Missing `[defines]` entry for `feature = "ffi"` in cbindgen config. +WARN: Missing `[defines]` entry for `feature = "ffi"` in cbindgen config. diff --git a/crates/arti-rpc-client-core/src/conn.rs b/crates/arti-rpc-client-core/src/conn.rs index d0f06b609..3f1603d6c 100644 --- a/crates/arti-rpc-client-core/src/conn.rs +++ b/crates/arti-rpc-client-core/src/conn.rs @@ -21,6 +21,7 @@ use crate::{ mod auth; mod connimpl; +use crate::util::Utf8CString; pub use connimpl::RpcConn; /// A handle to an open request. @@ -43,8 +44,6 @@ pub struct RequestHandle { // // I am not at all pleased with these types; we should revise them. // -// TODO RPC: Possibly, convert these to hold CString internally. -// // TODO RPC: Possibly, all of these should be reconstructed // from their serde_json::Values rather than forwarded verbatim. // (But why would we our json to be more canonical than arti's? See #1491.) @@ -57,9 +56,9 @@ pub struct RequestHandle { // // Invariant: it is valid JSON and contains no NUL bytes or newlines. // TODO RPC: check that the newline invariant is enforced in constructors. -// TODO RPC consider changing this to CString. -#[derive(Clone, Debug, derive_more::AsRef)] -pub struct SuccessResponse(String); +#[derive(Clone, Debug, derive_more::AsRef, derive_more::Into)] +#[as_ref(forward)] +pub struct SuccessResponse(Utf8CString); /// An Update Response from Arti, with information about the progress of a request. /// @@ -69,7 +68,8 @@ pub struct SuccessResponse(String); // TODO RPC: check that the newline invariant is enforced in constructors. // TODO RPC consider changing this to CString. #[derive(Clone, Debug, derive_more::AsRef)] -pub struct UpdateResponse(String); +#[as_ref(forward)] +pub struct UpdateResponse(Utf8CString); /// A Error Response from Arti, indicating that an error occurred. /// @@ -84,26 +84,33 @@ pub struct UpdateResponse(String); // Otherwise the `decode` method may panic. // // TODO RPC: check that the newline invariant is enforced in constructors. -// TODO RPC consider changing this to CString. #[derive(Clone, Debug, derive_more::AsRef)] +#[as_ref(forward)] // TODO: If we keep this, it should implement Error. -pub struct ErrorResponse(String); +pub struct ErrorResponse(Utf8CString); impl ErrorResponse { /// Construct an ErrorResponse from the Error reply. /// /// This not a From impl because we want it to be crate-internal. - pub(crate) fn from_validated_string(s: String) -> Self { + pub(crate) fn from_validated_string(s: Utf8CString) -> Self { ErrorResponse(s) } /// Try to interpret this response as an [`RpcError`]. pub fn decode(&self) -> RpcError { - crate::msgs::response::try_decode_response_as_err(&self.0) + crate::msgs::response::try_decode_response_as_err(self.0.as_ref()) .expect("Could not decode response that was already decoded as an error?") .expect("Could not extract error from response that was already decoded as an error?") } } +impl std::fmt::Display for ErrorResponse { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let e = self.decode(); + write!(f, "Peer said {:?}", e.message()) + } +} + /// A final response -- that is, the last one that we expect to receive for a request. /// type FinalResponse = Result<SuccessResponse, ErrorResponse>; @@ -384,9 +391,6 @@ pub enum ConnectError { /// IO error while connecting to Arti. #[error("Unable to make a connection: {0}")] CannotConnect(Arc<std::io::Error>), - /// One of our protocol negotiation messages was rejected. - #[error("Arti rejected our negotiation attempts: {0:?}")] - NegotiationRejected(ErrorResponse), /// One of our authentication messages was rejected. #[error("Arti rejected our authentication: {0:?}")] AuthenticationRejected(ErrorResponse), diff --git a/crates/arti-rpc-client-core/src/ffi.rs b/crates/arti-rpc-client-core/src/ffi.rs new file mode 100644 index 000000000..2dc774212 --- /dev/null +++ b/crates/arti-rpc-client-core/src/ffi.rs @@ -0,0 +1,174 @@ +//! Exposed C APIs for arti-rpc-client-core. +//! +//! See top-level documentation in header file for C conventions that affect the safety of these functions. +//! (These include things like "all input pointers must be valid" and so on.) + +pub mod err; +mod util; + +use err::{ArtiRpcError, InvalidInput}; +use std::ffi::c_char; +use util::{ffi_body_raw, ffi_body_with_err, OptOutPtrExt as _, OutPtr}; + +use crate::{util::Utf8CString, RpcConnBuilder}; + +/// A status code returned by an Arti RPC function. +/// +/// On success, a function will return `ARTI_SUCCESS (0)`. +/// On failure, a function will return some other status code. +pub type ArtiRpcStatus = u32; + +/// An open connection to Arti over an a RPC protocol. +/// +/// This is a thread-safe type: you may safely use it from multiple threads at once. +/// +/// Once you are no longer going to use this connection at all, you must free +/// it with [`arti_rpc_conn_free`] +pub type ArtiRpcConn = crate::RpcConn; + +/// An owned string, returned by this library. +/// +/// This string must be released with `arti_rpc_str_free`. +/// You can inspect it with `arti_rpc_str_get`, but you may not modify it. +/// The string is guaranteed to be UTF-8 and NUL-terminated. +pub type ArtiRpcStr = Utf8CString; + +/// Try to open a new connection to an Arti instance. +/// +/// The location of the instance and the method to connect to it are described in +/// `connection_string`. +/// +/// (TODO RPC: Document the format of this string better!) +/// +/// On success, return `ARTI_RPC_STATUS_SUCCESS` and set `*rpc_conn_out` to a new ArtiRpcConn. +/// Otherwise return some other status code, set `*rpc_conn_out` to NULL, and set +/// `*error_out` (if provided) to a newly allocated error object. +/// +/// +/// # Ownership +/// +/// The caller is responsible for making sure that `*rpc_conn_out` and `*error_out`, +/// if set, are eventually freed. +#[allow(clippy::missing_safety_doc)] +#[no_mangle] +pub unsafe extern "C" fn arti_rpc_connect( + connection_string: *const c_char, + rpc_conn_out: *mut *mut ArtiRpcConn, + error_out: *mut *mut ArtiRpcError, +) -> ArtiRpcStatus { + ffi_body_with_err!( + { + let connection_string: Option<&str> [in_str_opt]; + let rpc_conn_out: Option<OutPtr<ArtiRpcConn>> [out_ptr_opt]; + err error_out : Option<OutPtr<ArtiRpcError>>; + } in { + let connection_string = connection_string + .ok_or(InvalidInput::NullPointer)?; + + let builder = RpcConnBuilder::from_connect_string(connection_string)?; + + let conn = builder.connect()?; + + rpc_conn_out.write_value_if_ptr_set(conn); + } + ) +} + +/// Run an RPC request over `rpc_conn` and wait for a successful response. +/// +/// The message `msg` should be a valid RPC request in JSON format. +/// If you omit its `id` field, one will be generated: this is typically the best way to use this function. +/// +/// On success, return `ARTI_RPC_STATUS_SUCCESS` and set `*response_out` to a newly allocated string +/// containing the JSON response to your request (including `id` and `response` fields). +/// +/// Otherwise return some other status code, set `*response_out` to NULL, +/// and set `*error_out` (if provided) to a newly allocated error object. +/// +/// (If response_out is set to NULL, then any successful response will be ignored.) +/// +/// # Ownership +/// +/// The caller is responsible for making sure that `*error_out`, if set, is eventually freed. +#[allow(clippy::missing_safety_doc)] +#[no_mangle] +pub unsafe extern "C" fn arti_rpc_conn_execute( + rpc_conn: *const ArtiRpcConn, + msg: *const c_char, + response_out: *mut *mut ArtiRpcStr, + error_out: *mut *mut ArtiRpcError, +) -> ArtiRpcStatus { + ffi_body_with_err!( + { + let rpc_conn: Option<&ArtiRpcConn> [in_ptr_opt]; + let msg: Option<&str> [in_str_opt]; + let response_out: Option<OutPtr<ArtiRpcStr>> [out_ptr_opt]; + err error_out: Option<OutPtr<ArtiRpcError>>; + } in { + let rpc_conn = rpc_conn.ok_or(InvalidInput::NullPointer)?; + let msg = msg.ok_or(InvalidInput::NullPointer)?; + + let success = rpc_conn.execute(msg)??; + response_out.write_value_if_ptr_set(Utf8CString::from(success)); + } + ) +} + +/// Free a string returned by the Arti RPC API. +#[allow(clippy::missing_safety_doc)] +#[no_mangle] +pub unsafe extern "C" fn arti_rpc_str_free(string: *mut ArtiRpcStr) { + ffi_body_raw!( + { + let string: Option<Box<ArtiRpcStr>> [in_ptr_consume_opt]; + } in { + drop(string); + // Safety: Return value is (); trivially safe. + () + } + ); +} + +/// Return a const pointer to the underlying nul-terminated string from an `ArtiRpcStr`. +/// +/// The resulting string is guaranteed to be valid UTF-8. +/// +/// (Returns NULL if the input is NULL.) +/// +/// # Correctness requirements +/// +/// The resulting string pointer is valid only for as long as the input `string` is not freed. +#[allow(clippy::missing_safety_doc)] +#[no_mangle] +pub unsafe extern "C" fn arti_rpc_str_get(string: *const ArtiRpcStr) -> *const c_char { + ffi_body_raw!( + { + let string: Option<&ArtiRpcStr> [in_ptr_opt]; + } in { + // Safety: returned pointer is null, or semantically borrowed from `string`. + // It is only null if `string` was null. + // The caller is not allowed to modify it. + match string { + Some(s) => s.as_ptr(), + None => std::ptr::null(), + } + + } + ) +} + +/// Close and free an open Arti RPC connection. +#[allow(clippy::missing_safety_doc)] +#[no_mangle] +pub unsafe extern "C" fn arti_rpc_conn_free(rpc_conn: *mut ArtiRpcConn) { + ffi_body_raw!( + { + let rpc_conn: Option<Box<ArtiRpcConn>> [in_ptr_consume_opt]; + } in { + drop(rpc_conn); + // Safety: Return value is (); trivially safe. + () + + } + ); +} diff --git a/crates/arti-rpc-client-core/src/ffi/err.rs b/crates/arti-rpc-client-core/src/ffi/err.rs new file mode 100644 index 000000000..0bad4e442 --- /dev/null +++ b/crates/arti-rpc-client-core/src/ffi/err.rs @@ -0,0 +1,436 @@ +//! Error handling logic for our ffi code. + +use c_str_macro::c_str; +use paste::paste; +use std::ffi::{c_char, CStr}; +use std::fmt::Display; +use std::panic::{catch_unwind, UnwindSafe}; + +use crate::conn::ErrorResponse; +use crate::util::Utf8CString; + +use super::util::{ffi_body_raw, OptOutPtrExt as _, OutPtr}; +use super::ArtiRpcStatus; + +/// Helper: +/// Given a restricted enum defining FfiStatus, also define a series of constants for its variants, +/// and a string conversion function. + +// NOTE: I tried to use derive_deftly here, but ran into trouble when defining the constants. +// I wanted to have them be "pub const ARTI_FOO = FfiStatus::$vname", +// but that doesn't work with cbindgen, which won't expose a constant unless it is a public type +// it can recognize. +// There is no way to use derive_deftly to look at the explicit discriminant of an enum. +macro_rules! define_ffi_status { + { + $(#[$tm:meta])* + pub(crate) enum FfiStatus { + $( + $(#[$m:meta])* + [$s:expr] + $id:ident = $e:expr, + )+ + } + + } => {paste!{ + $(#[$tm])* + pub(crate) enum FfiStatus { + $( + $(#[$m])* + $id = $e, + )+ + } + + $( + $(#[$m])* + pub const [<ARTI_RPC_STATUS_ $id:snake:upper >] : ArtiRpcStatus = $e; + )+ + + /// Return a string representing the meaning of a given `ArtiRpcStatus`. + /// + /// The result will always be non-NULL, even if the status is unrecognized. + #[no_mangle] + pub extern "C" fn arti_status_to_str(status: ArtiRpcStatus) -> *const c_char { + match status { + $( + [<ARTI_RPC_STATUS_ $id:snake:upper>] => c_str!($s), + )+ + _ => c_str!("(unrecognized status)"), + }.as_ptr() + } + }} +} + +define_ffi_status! { +/// View of FFI status as rust enumeration. +/// +/// Not exposed in the FFI interfaces, except via cast to ArtiStatus. +/// +/// We define this as an enumeration so that we can treat it exhaustively in Rust. +#[derive(Copy, Clone, Debug)] +#[repr(u32)] +pub(crate) enum FfiStatus { + /// The function has returned successfully. + #[allow(dead_code)] + ["Success"] + Success = 0, + + /// One or more of the inputs to a library function was invalid. + /// + /// (This error was generated by the library, before any request was sent.) + ["Invalid input"] + InvalidInput = 1, + + /// Tried to use some functionality + /// (for example, an authentication method or connection scheme) + /// that wasn't available on this platform or build. + /// + /// (This error was generated by the library, before any request was sent.) + ["Not supported"] + NotSupported = 2, + + /// Tried to connect to Arti, but an IO error occurred. + /// + /// This may indicate that Arti wasn't running, + /// or that Arti was built without RPC support, + /// or that Arti wasn't running at the specified location. + /// + /// (This error was generated by the library.) + ["An IO error ocurred while connecting to Arti"] + ConnectIo = 3, + + /// We tried to authenticate with Arti, but it rejected our attempt. + /// + /// (This error was sent by the peer.) + ["Authentication rejected"] + BadAuth = 4, + + /// Our peer has, in some way, violated the Arti-RPC protocol. + /// + /// (This error was generated by the library, + /// based on a response from Arti that appeared to be invalid.) + ["Peer violated the RPC protocol"] + PeerProtocolViolation = 5, + + /// The peer has closed our connection; possibly because it is shutting down. + /// + /// (This error was generated by the library, + /// based on the connection being closed or reset from the peer.) + ["Peer has shut down"] + Shutdown = 6, + + /// An internal error occurred in the arti rpc client. + /// + /// (This error was generated by the library. + /// If you see it, there is probably a bug in the library.) + ["Internal error; possible bug?"] + Internal = 7, + + /// The peer reports that one of our requests has failed. + /// + /// (This error was sent by the peer, in response to one of our requests. + /// No further responses to that request will be received or accepted.) + ["Request has failed"] + RequestFailed = 8, + + /// Tried to check the status of a request and found that it was no longer running. + /// + /// TODO RPC: We should make sure that this is the actual semantics we want for this + /// error! Revisit after we have implemented real cancellation. + ["Request was cancelled"] + RequestCancelled = 9, +} +} + +/// An error as returned by the Arti FFI code. +#[derive(Debug, Clone)] +pub struct FfiError { + /// The status of this error messages + pub(super) status: ArtiRpcStatus, + /// A human-readable message describing this error + message: Utf8CString, + /// If present, a Json-formatted message from our peer that we are representing with this error. + error_response: Option<ErrorResponse>, +} + +impl FfiError { + /// Helper: If this error stems from a resoponse from our RPC peer, + /// return that reponse. + fn error_response_as_ptr(&self) -> Option<*const c_char> { + self.error_response.as_ref().map(|response| { + let cstr: &CStr = response.as_ref(); + cstr.as_ptr() + }) + } +} + +/// Convenience trait to help implement `Into<FfiError>` +/// +/// Any error that implements this trait will be convertible into an [`FfiError`]. +// additional requirements: display doesn't make NULs. +pub(crate) trait IntoFfiError: Display + Sized { + /// Return the status + fn status(&self) -> FfiStatus; + /// Return a message for this error. + /// + /// By default, returns the Display of this error. + fn message(&self) -> String { + self.to_string() + } + /// Consume this error and return an [`ErrorResponse`] + fn into_error_response(self) -> Option<ErrorResponse> { + None + } +} +impl<T: IntoFfiError> From<T> for FfiError { + fn from(value: T) -> Self { + let status = value.status() as u32; + let message = value + .message() + .try_into() + .expect("Error message had a NUL?"); + let error_response = value.into_error_response(); + Self { + status, + message, + error_response, + } + } +} +impl From<void::Void> for FfiError { + fn from(value: void::Void) -> Self { + void::unreachable(value) + } +} + +/// Tried to call a ffi function with a not-permitted argument. +#[derive(Clone, Debug, thiserror::Error)] +pub(super) enum InvalidInput { + /// Tried to convert a NULL pointer to a string. + #[error("Provided string was NULL.")] + NullPointer, + + /// Tried to convert a non-UTF string. + #[error("Provided string was not UTF-8")] + BadUtf8, +} + +impl From<void::Void> for InvalidInput { + fn from(value: void::Void) -> Self { + void::unreachable(value) + } +} + +impl IntoFfiError for InvalidInput { + fn status(&self) -> FfiStatus { + FfiStatus::InvalidInput + } +} + +impl IntoFfiError for crate::ConnectError { + fn status(&self) -> FfiStatus { + use crate::ConnectError as E; + use FfiStatus as F; + match self { + E::SchemeNotSupported => F::NotSupported, + E::CannotConnect(_) => F::ConnectIo, + E::AuthenticationRejected(_) => F::BadAuth, + E::BadMessage(_) => F::PeerProtocolViolation, + E::ProtoError(e) => e.status(), + } + } + + fn into_error_response(self) -> Option<ErrorResponse> { + use crate::ConnectError as E; + match self { + E::AuthenticationRejected(msg) => Some(msg), + _ => None, + } + } +} + +impl IntoFfiError for crate::ProtoError { + fn status(&self) -> FfiStatus { + use crate::ProtoError as E; + use FfiStatus as F; + match self { + E::Shutdown(_) => F::Shutdown, + E::InvalidRequest(_) => F::InvalidInput, + E::RequestIdInUse => F::InvalidInput, + E::RequestCancelled => F::RequestCancelled, + E::DuplicateWait => F::Internal, + E::CouldNotEncode(_) => F::Internal, + } + } +} + +impl IntoFfiError for crate::BuilderError { + fn status(&self) -> FfiStatus { + use crate::BuilderError as E; + use FfiStatus as F; + match self { + E::InvalidConnectString => F::InvalidInput, + } + } +} + +impl IntoFfiError for ErrorResponse { + fn status(&self) -> FfiStatus { + FfiStatus::RequestFailed + } + fn into_error_response(self) -> Option<ErrorResponse> { + Some(self) + } +} + +/// An error returned by the Arti RPC code, exposed as an object. +/// +/// When a function returns an [`ArtiRpcStatus`] other than [`ARTI_RPC_STATUS_SUCCESS`], +/// it will also expose a newly allocated value of this type +/// via its `error_out` parameter. +pub type ArtiRpcError = FfiError; + +/// Return the status code associated with a given error. +/// +/// If `err` is NULL, return [`ARTI_RPC_STATUS_INVALID_INPUT`]. +#[allow(clippy::missing_safety_doc)] +#[no_mangle] +pub unsafe extern "C" fn arti_rpc_err_status(err: *const ArtiRpcError) -> ArtiRpcStatus { + ffi_body_raw!( + { + let err: Option<&ArtiRpcError> [in_ptr_opt]; + } in { + err.map(|e| e.status) + .unwrap_or(ARTI_RPC_STATUS_INVALID_INPUT) + // Safety: Return value is ArtiRpcStatus; trivially safe. + } + ) +} + +/// Return a human-readable error message associated with a given error. +/// +/// The format of these messages may change arbitrarily between versions of this library; +/// it is a mistake to depend on the actual contents of this message. +/// +/// Return NULL if the input `err` is NULL. +/// +/// # Correctness requirements +/// +/// The resulting string pointer is valid only for as long as the input `err` is not freed. +#[allow(clippy::missing_safety_doc)] +#[no_mangle] +pub unsafe extern "C" fn arti_rpc_err_message(err: *const ArtiRpcError) -> *const c_char { + ffi_body_raw!( + { + let err: Option<&ArtiRpcError> [in_ptr_opt]; + } in { + err.map(|e| e.message.as_ptr()) + .unwrap_or(std::ptr::null()) + // Safety: returned pointer is null, or semantically borrowed from `err`. + // It is only null if `err` was null. + // The caller is not allowed to modify it. + } + ) +} + +/// Return a Json-formatted error response associated with a given error. +/// +/// These messages are full responses, including the `error` field, +/// and the `id` field (if present). +/// +/// Return NULL if the specified error does not represent an RPC error response. +/// +/// Return NULL if the input `err` is NULL. +/// +/// # Correctness requirements +/// +/// The resulting string pointer is valid only for as long as the input `err` is not freed. +#[allow(clippy::missing_safety_doc)] +#[no_mangle] +pub unsafe extern "C" fn arti_rpc_err_response(err: *const ArtiRpcError) -> *const c_char { + ffi_body_raw!( + { + let err: Option<&ArtiRpcError> [in_ptr_opt]; + } in { + err.and_then(ArtiRpcError::error_response_as_ptr) + .unwrap_or(std::ptr::null()) + // Safety: returned pointer is null, or semantically borrowed from `err`. + // It is only null if `err` was null, or if `err` contained no response field. + // The caller is not allowed to modify it. + } + ) +} + +/// Make and return copy of a provided error. +/// +/// Return NULL if the input is NULL. +/// +/// # Ownership +/// +/// The caller is responsible for making sure that the returned object +/// is eventually freed with `arti_rpc_err_free()`. +#[allow(clippy::missing_safety_doc)] +#[no_mangle] +pub unsafe extern "C" fn arti_rpc_err_clone(err: *const ArtiRpcError) -> *mut ArtiRpcError { + ffi_body_raw!( + { + let err: Option<&ArtiRpcError> [in_ptr_opt]; + } in { + err.map(|e| Box::into_raw(Box::new(e.clone()))) + .unwrap_or(std::ptr::null_mut()) + // Safety: returned pointer is null, or newly allocated via Box::new(). + // It is only null if the input was null. + } + ) +} + +/// Release storage held by a provided error. +#[allow(clippy::missing_safety_doc)] +#[no_mangle] +pub unsafe extern "C" fn arti_rpc_err_free(err: *mut ArtiRpcError) { + ffi_body_raw!( + { + let err: Option<Box<ArtiRpcError>> [in_ptr_consume_opt]; + } in { + drop(err); + // Safety: Return value is (); trivially safe. + () + } + ); +} + +/// Run `body` and catch panics. If one occurs, return the result of `on_err` instead. +/// +/// We wrap the body of every C ffi function with this function +/// (or with `handle_errors`, which uses this function), +/// even if we do not think that the body can actually panic. +pub(super) fn abort_on_panic<F, T>(body: F) -> T +where + F: FnOnce() -> T + UnwindSafe, +{ + #[allow(clippy::print_stderr)] + match catch_unwind(body) { + Ok(x) => x, + Err(_panic_info) => { + eprintln!("Internal panic in arti-rpc library: aborting!"); + std::process::abort(); + } + } +} + +/// Call `body`, converting any errors or panics that occur into an FfiError, +/// and storing that error in `error_out`. +pub(super) fn handle_errors<F>(error_out: Option<OutPtr<FfiError>>, body: F) -> ArtiRpcStatus +where + F: FnOnce() -> Result<(), FfiError> + UnwindSafe, +{ + match abort_on_panic(body) { + Ok(()) => ARTI_RPC_STATUS_SUCCESS, + Err(e) => { + // "body" returned an error. + let status = e.status; + error_out.write_value_if_ptr_set(e); + status + } + } +} diff --git a/crates/arti-rpc-client-core/src/ffi/util.rs b/crates/arti-rpc-client-core/src/ffi/util.rs new file mode 100644 index 000000000..cf7aec99f --- /dev/null +++ b/crates/arti-rpc-client-core/src/ffi/util.rs @@ -0,0 +1,592 @@ +//! Helpers for working with FFI. + +use std::mem::MaybeUninit; + +/// Helper for output parameters represented as `*mut *mut T`. +/// +/// This is for an API which, from a C POV, returns an output via a parameter of type +/// `Foo **foo_out`. When an `OutPtr` is constructed, `*foo_out` is necessarily non-null. +/// +/// If `foo_out` is not NULL, then `*foo_out` is always set to NULL when an `OutPtr` +/// is constructed, so that even if the FFI code panics, the inner pointer will be initialized to +/// _something_. +pub(super) struct OutPtr<'a, T>(&'a mut *mut T); + +impl<'a, T> OutPtr<'a, T> { + /// Construct `Option<Self>` from a possibly NULL pointer; initialize `*ptr` to NULL if possible. + /// + /// # Safety + /// + /// The outer pointer, if set, must be valid, and must not alias any other pointers. + /// + /// See also the requirements on `pointer::as_mut()`. + /// + /// # No panics! + /// + /// This method can be invoked in cases where panicking is not allowed (such as + /// in a FFI method, outside of `handle_errors()` or `catch_panic()`.) + // + // (I have tested this using the `no-panic` crate. But `no-panic` is not suitable + // for use in production, since it breaks when run in debug mode.) + pub(super) unsafe fn from_opt_ptr(ptr: *mut *mut T) -> Option<Self> { + if ptr.is_null() { + None + } else { + // TODO: Use `.as_mut_uninit` once it is stable. + // + // SAFETY: See documentation for [`<*mut *mut T>::as_uninit_mut`] + // at https://doc.rust-lang.org/std/primitive.pointer.html#method.as_uninit_mut : + // This is the same code. + let ptr: &mut MaybeUninit<*mut T> = unsafe { &mut *(ptr as *mut MaybeUninit<*mut T>) }; + let ptr: &mut *mut T = ptr.write(std::ptr::null_mut()); + Some(OutPtr(ptr)) + } + } + + /// Consume this OutPtr and the provided value, writing the value into the outptr. + pub(super) fn write_value(self, value: T) { + // Note that all the unsafety happened when we constructed a &mut from the pointer. + // + // Note also that this method consumes `self`. That's because we want to avoid multiple + // writes to the same OutPtr: If we did that, we would sometimes have to free a previous + // value. + *self.0 = Box::into_raw(Box::new(value)); + } +} + +/// Trait to prevent implementation of OptOutPtrExt inappropriately. +// +// TODO: Move this into a separate module once our MSRV allows us to do so. +// With 1.70, it causes an error. +pub(super) trait Sealed {} +/// Extension trait on `Option<OutPtr<T>>` +#[allow(private_bounds)] +pub(super) trait OptOutPtrExt<T>: Sealed { + /// Consume this `Option<OutPtr<T>>` and the provided value. + /// + /// If this is Some, write the value into the outptr. + /// + /// Otherwise, discard the value. + fn write_value_if_ptr_set(self, value: T); +} +impl<'a, T> Sealed for Option<OutPtr<'a, T>> {} +impl<'a, T> OptOutPtrExt<T> for Option<OutPtr<'a, T>> { + fn write_value_if_ptr_set(self, value: T) { + if let Some(outptr) = self { + outptr.write_value(value); + } + } +} + +/// Implement the body of an FFI function. +/// +/// This macro handles the calling convention of an FFI function. +/// Proper use of this macro will ensure that the FFI function behaves as documented, +/// as regards pointer handling, ownership, lifetimes, and error handling. +/// It also catches panics, making sure that we don't unwind into the FFI caller. +/// I.e. it ensures that correct callers will not experience UB. +/// +/// This variant is for functions that +/// don't pass back an `ArtiRpcError` via an out parameter. +/// See [`ffi_body_with_err!`] for that. +/// +/// This macro is meant to be invoked as follows: +/// +/// ```ignore +/// ffi_body_raw!( +/// { +/// [CONVERSIONS] +/// } in { +/// [BODY] +/// } on invalid { +/// [VALUE_ON_BAD_INPUT] +/// } +/// ) +/// ``` +/// +/// For example: +/// +/// ```ignore +/// pub extern "C" fn arti_rpc_cook_meal( +/// recipe: *const Recipe, +/// special_ingredients: *const Ingredients, +/// n_guests: usize, +/// dietary_constraints: *const c_char, +/// food_out: *mut *mut DeliciousMeal, +/// ) -> usize { +/// ffi_body_raw!( +/// { // [CONVERSIONS] +/// let recipe: Option<&Recipe> [in_ptr_opt]; +/// let ingredients: Option<&Ingredients> [in_ptr_opt]; +/// let dietary_constraints: Option<&str> [in_str_opt]; +/// let food_out: OutPtr<DeliciousMeal> [out_ptr_opt]; +/// } in { +/// // [BODY] +/// let Some(recipe) = recipe else { return 0 }; +/// let delicious_meal = prepare_meal( +/// recipe, ingredients, dietary_constraints, n_guests +/// ); +/// food_out.write_value_if_nonnull(delicious_meal); +/// n_guests +/// } on invalid { +/// // [VALUE_ON_BAD_INPUT] +/// 0 +/// } +/// ) +/// } +/// ``` +/// +/// The first part (`CONVERSIONS`) defines a set of conversions to be done on the function inputs. +/// These are documented below. +/// Each conversion performs an unsafe operation, +/// making certain assumptions about an input variable, +/// in order to produce an output of the specified type. +/// Conversions can reject input values. +/// If they do, the function will return; +/// see discussion of `[VALUE_ON_BAD_INPUT]` +/// +/// Pointer parameters to the outer function *must not be ignored*. +/// Every raw pointer parameter must be processed by this macro. +/// (For raw pointer arguments that are not, +/// no guarantees are made by the macro, +/// and the overall function will probably be unsound. +/// There is no checking that every pointer parameter is properly used, +/// other than Rust's usual detection of unused variables.) +/// +/// The second part (`BODY`) is the body of the function. +/// The body is *outside* `unsafe`, and +/// it should generally be possible to write this body without using unsafe code. +/// The result of this block is the returned value of the function. +/// +/// The third part (`VALUE_ON_BAD_INPUT`) is an expression to be returned +/// as the result of the function if any input pointer has a rejected value. +/// You may omit the entire `on invalid { ... }` part of the macro's input +/// when all of the conversions are infallible. +/// (This is checked statically.) +/// +/// ## Supported conversions +/// +/// All conversions take the following format: +/// +/// `let NAME : TYPE [METHOD] ;` +/// +/// The `NAME` must match one of the inputs to the function. +/// +/// The `TYPE` must match the actual type that the input will be converted to. +/// (These types are generally easy to use ergonomically from safe rust.) +/// +/// The `METHOD` is an identifier explaining how the input is to be converted. +/// +/// The following methods are recognized: +/// +/// | method | input type | converted to | can reject input? | +/// |----------------------|-----------------|--------------------|-------------------| +/// | `in_ptr_opt` | `*const T` | `Option<&T>` | N | +/// | `in_str_opt` | `*const c_char` | `Option<&str>` | Y | +/// | `in_ptr_consume_opt` | `*mut T` | `Option<Box<T>>` | N | +/// | `out_ptr_opt` | `*mut *mut T` | `Option<OutPtr<T>>`| N | +/// +/// > (Note: Other conversion methods are logically possible, but have not been added yet, +/// > since they would not yet be used in this crate.) +/// +/// ## Safety +/// +/// The `in_ptr_opt` method +/// has the safety requirements of +/// [`<*const T>::as_ref`](https://doc.rust-lang.org/std/primitive.pointer.html#method.as_ref). +/// Informally, this means: +/// * If the pointer is not null, it must point +/// to a valid aligned dereferenceable instance of `T`. +/// * The underlying `T` must not be freed or modified for so long as the function is running. +/// +/// The `in_str_opt` method, when its input is non-NULL, +/// has the safety requirements of [`CStr::from_ptr`](std::ffi::CStr::from_ptr). +/// Informally, this means: +/// * If the pointer is not null, it must point to a nul-terminated string. +/// * The string must not be freed or modified for so long as the function is running. +/// +/// Additionally, the `[in_str_opt]` method +/// will detect invalid any string that is not UTF-8. +/// +/// The `in_ptr_consume_opt` method, when its input is non-NULL, +/// has the safety requirements of [`Box::from_raw`]. +/// Informally, this is satisfied when: +/// * If the pointer is not null, it should be +/// the result of an earlier a call to `Box<T>::into_raw`. +/// (Note that using either `out_ptr_*` method +/// will output pointers that can later be consumed in this way.) +/// +/// The `out_ptr_opt` method +/// has the safety requirements of +/// [`<*mut *mut T>::as_uninit_mut`](https://doc.rust-lang.org/std/primitive.pointer.html#method.as_uninit_mut). +/// Informally, this means: +/// * If the pointer (call it "out") is non-NULL, then `*out` must point to aligned +/// "dereferenceable" (q.v.) memory holding a possibly uninitialized "*mut T". +/// +/// (Note that immediately upon conversion, if `out` is non-NULL, +/// `*out` is set to NULL. See documentation for `OptPtr`.) +/// +/// The return value of `BODY` becomes the return value of the C FFI function. +/// It is the macro user's responsibility to ensure +/// that it conforms to the published API. +/// For example, if the return value is a raw pointer, +/// the macro user must ensure it's suitably dereferencable, +/// that its lifetime is documented, +/// and only null when the API says that's allowed. +// +// Design notes: +// - I am keeping the conversions separate from the body below, since we don't want to catch +// InvalidInput from the body. +// - The "on invalid" value must be specified explicitly if it can happen, +// since in general we should force the caller to think about it. +// Getting a 0 or -1 wrong here can have nasty results. +// - The conversion syntax deliberately includes the type of the converted argument, +// on the theory that it makes the functions more readable. +// - The conversion code deliberately shadows the original parameter with the +// converted parameter. +macro_rules! ffi_body_raw { + { + { + $( + let $name:ident : $type:ty [$how:ident] + );* + $(;)? + } in { + $($body:tt)+ + } on invalid { + $err:expr + } + } => { + crate::ffi::err::abort_on_panic(|| { + // run conversions and check for invalid input exceptions. + crate::ffi::util::ffi_initialize!{ + { + $( let $name : $type [$how]; )* + } else with _ignore_err : crate::ffi::err::InvalidInput { + #[allow(clippy::unused_unit)] + return $err; + } + }; + + $($body)+ + + }, + ) + }; + + { + { + $( + let $name:ident : $type:ty [$how:ident] + );* + $(;)? + } in { + $($body:tt)+ + } + } => { + crate::ffi::err::abort_on_panic(|| { + // run conversions and check for invalid input exceptions. + crate::ffi::util::ffi_initialize!{ + { + $( let $name : $type [$how]; )* + } else with impossible_error : void::Void { + void::unreachable(impossible_error); + } + }; + + $($body)+ + + }, + ) + }; + +} +pub(super) use ffi_body_raw; + +/// Implement the body of an FFI function that returns an ArtiRpcStatus. +/// +/// This macro is meant to be invoked as follows: +/// ```text +/// ffi_body_with_err!( +/// { +/// [CONVERSIONS] +/// err [ERRNAME] : OutPtr<ArtiRpcError>; +/// } in { +/// [BODY] +/// } +/// })``` +/// +/// For example: +/// +/// ```ignore +/// pub extern "C" fn arti_rpc_wombat_feed( +/// wombat: *const Wombat, +/// wombat_chow: *const Meal, +/// error_out: *mut *mut ArtiRpcError +/// ) -> ArtiRpcStatus { +/// ffi_body_with_err!( +/// { +/// let wombat: Option<&Wombat> [in_ptr_opt]; +/// let wombat_chow: Option<&Meal> [in_ptr_opt]; +/// err error_out: Option<OutPtr<ArtiRpcError>>; +/// } in { +/// let wombat = wombat.ok_or(InvalidInput::NullPointer)? +/// let wombat_chow = wombat_chow.ok_or(InvalidInput::NullPointer)? +/// wombat.please_enjoy(wombat_chow)?; +/// } +/// ) +/// } +/// ``` +/// +/// The resulting function has the same kinds +/// of conversions as would [`ffi_body_raw!`]. +/// +/// The differences are: +/// * Instead of returning a value, the body can only give errors with `?`. +/// * The function must return ArtiRpcStatus. +/// * Any errors that occur during the conversions or the body +/// are converted into an ArtiRpcError, +/// and given to the user via `error_out` if it is non-NULL. +/// A corresponding ArtiRpcStatus is returned. +/// +/// ## Safety +/// +/// The safety requirements are the same as for `ffi_body_raw`, except that: +/// +/// The safety requirements for the `err` conversion +/// are the same as those for `out_ptr_opt` (q.v.). +/// +/// `ffi_body_with_err` then additionally ensures conformance of +/// the return value with the API's error handling rules. +macro_rules! ffi_body_with_err { + { + { + $( + let $name:ident : $type:ty [$how:ident]; + )* + err $err_out:ident : $err_type:ty $(;)? + } in { + $($body:tt)+ + } + } => {{ + use void::ResultVoidExt as _; + let $err_out: $err_type = + unsafe { crate::ffi::util::arg_conversion::out_ptr_opt($err_out) } + .void_unwrap(); + + crate::ffi::err::handle_errors($err_out, + || { + crate::ffi::util::ffi_initialize!{ + { + $( let $name : $type [$how]; )* + } else with err: crate::ffi::err::ArtiRpcError { + return Err(crate::ffi::err::ArtiRpcError::from(err)); + } + }; + + let () = { $($body)+ }; + + Ok(()) + } + ) + }} +} +pub(super) use ffi_body_with_err; + +/// Implement a set of conversions, trying each one. +/// +/// (It's important that this cannot exit early, +/// since some conversions have side effects: notably, the ones that create an OutPtr +/// can initialize that pointer to NULL, and we want to do that unconditionally. +/// +/// If any conversion fails, run `return ($on_invalid)(error)` +/// _after_ every conversion has succeeded or failed. +/// +/// The syntax is: +/// +/// ```ignore +/// ffi_initialize!{ +/// { [CONVERSIONS] } +/// else with [ERR_IDENT] { [ERR_BODY] } +/// } +/// ``` +/// +/// The `[CONVERSIONS]` have the same syntax and behavior as in [`ffi_body_raw!`]. +/// After every conversion has been tried, if one or more of them failed, +/// then the `[ERR_BODY]` code is run, +/// with `[ERR_IDENT]` bound to an instance of `InvalidInput`. +macro_rules! ffi_initialize { + { + { + $( let $name:ident : $type:ty [$how:ident] ; )* + } else with $err_id:ident: $err_type:ty { + $($on_invalid:tt)* + } + } => { + // General approach + // + // First, we process each `$name` into `Result<$type>`, without doing any early exits. + // This ensures that we process every `$name`, even if some of the processing fails. + // + // Then we convert each `Result<X>` into just `X` + // (with an IEFE that returns a `Result<(X,...)>` - one `Result` with a big tuple. + // We rebinding the `$name`'s to the values from the tuple. + #[allow(unused_parens)] + let ($($name,)*) : ($($type,)*) = { + $( + let $name : Result<$type, _> + = unsafe { crate::ffi::util::arg_conversion::$how($name) }; + )* + + #[allow(clippy::needless_question_mark)] + // Note that the question marks here exit from _this_ closure. + match (|| -> Result<_,$err_type> { + Ok(($($name?,)*)) + })() { + Ok(v) => v, + Err($err_id) => { + $($on_invalid)* + } + } + }; + }; +} + +/// Functions to implement argument conversion. +/// +/// Each of these functions corresponds to a conversion mode used in `ffi_initialize!`. +/// +/// Every function has all of these properties: +/// +/// - It returns `Err(InvalidInput)` if the conversion fails, +/// and `Ok($ty)` if the conversion succeeds. +/// (Infallible conversions always return `Ok`.) +/// +/// Nothing outside of the `ffi_initialize!` macro should actually invoke these functions! +#[allow(clippy::unnecessary_wraps)] +pub(super) mod arg_conversion { + use super::OutPtr; + use crate::ffi::err::InvalidInput; + use std::ffi::{c_char, CStr}; + use void::Void; + + /// Try to convert a const pointer to an optional reference. + /// + /// A null pointer is allowed, and converted to `None`. + /// + /// # Safety + /// + /// As for [`<*const T>::as_ref`](https://doc.rust-lang.org/std/primitive.pointer.html#method.as_ref). + pub(in crate::ffi) unsafe fn in_ptr_opt<'a, T>(input: *const T) -> Result<Option<&'a T>, Void> { + Ok(unsafe { input.as_ref() }) + } + + /// Try to convert a `const char *` to a `&str`. + /// + /// A null pointer is allowed, and converted to `None`. + /// Non-UTF-8 inputs will give an error. + /// + /// # Safety + /// + /// As for [`CStr::from_ptr`](std::ffi::CStr::from_ptr). + pub(in crate::ffi) unsafe fn in_str_opt<'a>( + input: *const c_char, + ) -> Result<Option<&'a str>, InvalidInput> { + if input.is_null() { + return Ok(None); + } + + // Safety: We require that the safety properties of CStr::from_ptr hold. + unsafe { CStr::from_ptr(input) } + .to_str() + .map(Some) + .map_err(|_| InvalidInput::BadUtf8) + } + + /// Try to convert a mutable pointer to a `Option<Box<T>>`. + /// + /// A null pointer is allowed, and converted to `None`. + /// + /// # Safety + /// + /// As for [`Box::from_raw`]. + pub(in crate::ffi) unsafe fn in_ptr_consume_opt<T>( + input: *mut T, + ) -> Result<Option<Box<T>>, Void> { + Ok(if input.is_null() { + None + } else { + Some(unsafe { Box::from_raw(input) }) + }) + } + + /// Try to convert a mutable pointer-to-pointer into an `Option<OutPtr<T>>`. + /// + /// A null pointer is allowed, and converted into None. + /// + /// Whatever the target of the original pointer (`input: *mut *mut T`), if `input` is non-null. + /// then `*input` is initialized to NULL. + /// + /// It is safe for `*input` to be uninitialized. + /// + /// # Safety + /// + /// As for + /// [`<*mut *mut T>::as_uninit_mut`](https://doc.rust-lang.org/std/primitive.pointer.html#method.as_uninit_mut). + pub(in crate::ffi) unsafe fn out_ptr_opt<'a, T>( + input: *mut *mut T, + ) -> Result<Option<OutPtr<'a, T>>, Void> { + Ok(unsafe { crate::ffi::util::OutPtr::from_opt_ptr(input) }) + } +} + +pub(super) use ffi_initialize; + +#[cfg(test)] +mod test { + // @@ begin test lint list maintained by maint/add_warning @@ + #![allow(clippy::bool_assert_comparison)] + #![allow(clippy::clone_on_copy)] + #![allow(clippy::dbg_macro)] + #![allow(clippy::mixed_attributes_style)] + #![allow(clippy::print_stderr)] + #![allow(clippy::print_stdout)] + #![allow(clippy::single_char_pattern)] + #![allow(clippy::unwrap_used)] + #![allow(clippy::unchecked_duration_subtraction)] + #![allow(clippy::useless_vec)] + #![allow(clippy::needless_pass_by_value)] + //! <!-- @@ end test lint list maintained by maint/add_warning @@ --> + + use super::*; + + unsafe fn outptr_user(ptr: *mut *mut i8, set_to_val: Option<i8>) { + let ptr = unsafe { OutPtr::from_opt_ptr(ptr) }; + + if let Some(v) = set_to_val { + ptr.write_value_if_ptr_set(v); + } + } + + #[test] + fn outptr() { + let mut ptr_to_int: *mut i8 = 7 as _; // This is a junk dangling pointer. It will get overwritten. + + // Case 1: Don't set to anything. + unsafe { outptr_user(&mut ptr_to_int as _, None) }; + assert!(ptr_to_int.is_null()); + + // Cases 2, 3: Provide a null pointer for the output pointer. + ptr_to_int = 7 as _; // make it junk again. + unsafe { outptr_user(std::ptr::null_mut(), None) }; + assert_eq!(ptr_to_int, 7 as _); // we didn't pass this in, so it wasn't set. + unsafe { outptr_user(std::ptr::null_mut(), Some(5)) }; + assert_eq!(ptr_to_int, 7 as _); // we didn't pass this in, so it wasn't set. + + // Case 4: Actually set something. + unsafe { outptr_user(&mut ptr_to_int as _, Some(123)) }; + assert!(!ptr_to_int.is_null()); + let boxed = unsafe { Box::from_raw(ptr_to_int) }; + assert_eq!(*boxed, 123); + } +} diff --git a/crates/arti-rpc-client-core/src/lib.rs b/crates/arti-rpc-client-core/src/lib.rs index 615b9d403..b919042cd 100644 --- a/crates/arti-rpc-client-core/src/lib.rs +++ b/crates/arti-rpc-client-core/src/lib.rs @@ -39,8 +39,13 @@ #![allow(clippy::result_large_err)] // temporary workaround for arti#587 #![allow(clippy::needless_raw_string_hashes)] // complained-about code is fine, often best //! <!-- @@ end lint list maintained by maint/add_warning @@ --> -//! + +// TODO RPC: Possibly add this to our big list of lints. +#![deny(unsafe_op_in_unsafe_fn)] + mod conn; +#[cfg(feature = "ffi")] +pub mod ffi; pub mod llconn; mod msgs; #[macro_use] diff --git a/crates/arti-rpc-client-core/src/msgs/response.rs b/crates/arti-rpc-client-core/src/msgs/response.rs index c89cfbc7c..fe86e5d62 100644 --- a/crates/arti-rpc-client-core/src/msgs/response.rs +++ b/crates/arti-rpc-client-core/src/msgs/response.rs @@ -5,7 +5,10 @@ use std::sync::Arc; use serde::Deserialize; use super::AnyRequestId; -use crate::{conn::ErrorResponse, util::define_from_for_arc}; +use crate::{ + conn::ErrorResponse, + util::{define_from_for_arc, Utf8CString}, +}; /// An unparsed and unvalidated response, as received from Arti. /// @@ -29,7 +32,7 @@ impl UnparsedResponse { #[derive(Clone, Debug)] pub(crate) struct ValidatedResponse { /// The text of this response. - pub(crate) msg: String, + pub(crate) msg: Utf8CString, /// The metadata from this response. pub(crate) meta: ResponseMeta, } @@ -58,10 +61,11 @@ impl UnparsedResponse { /// return it as a ValidatedResponse. pub(crate) fn try_validate(self) -> Result<ValidatedResponse, DecodeResponseError> { let meta = response_meta(self.as_ref())?; - Ok(ValidatedResponse { - msg: self.msg, - meta, - }) + let msg = self.msg.try_into().map_err(|_| { + // (This should be impossible; serde_json rejects NULs.) + DecodeResponseError::ProtocolViolation("Unexpected NUL in validated message") + })?; + Ok(ValidatedResponse { msg, meta }) } } @@ -81,12 +85,6 @@ impl ValidatedResponse { } } -impl From<ValidatedResponse> for String { - fn from(value: ValidatedResponse) -> Self { - value.msg - } -} - /// Metadata extracted from a response while decoding it. #[derive(Clone, Debug)] #[cfg_attr(test, derive(Eq, PartialEq))] @@ -158,7 +156,13 @@ pub(crate) fn response_meta(s: &str) -> Result<ResponseMeta, DecodeResponseError let ResponseMetaDe { id, body } = serde_json::from_str(s)?; match (id, body) { (None, Body::Error(_ignore)) => { - Err(E::Fatal(ErrorResponse::from_validated_string(s.to_owned()))) + let msg = s.to_owned().try_into().map_err(|_| { + // (This should be impossible; serde_json rejects NULs.) + DecodeResponseError::ProtocolViolation( + "Unexpected NUL in validated fatal error message", + ) + })?; + Err(E::Fatal(ErrorResponse::from_validated_string(msg))) } (None, _) => Err(E::ProtocolViolation("Missing ID field")), (Some(id), body) => Ok(ResponseMeta { diff --git a/crates/arti-rpc-client-core/src/util.rs b/crates/arti-rpc-client-core/src/util.rs index 3d22d097d..33d15f425 100644 --- a/crates/arti-rpc-client-core/src/util.rs +++ b/crates/arti-rpc-client-core/src/util.rs @@ -14,4 +14,64 @@ macro_rules! define_from_for_arc { } }; } +use std::ffi::{CStr, CString, NulError}; + pub(crate) use define_from_for_arc; + +/// A string that is guaranteed to be UTF-8 and NUL-terminated, +/// for fast access as either type. +// +// TODO RPC: Rename so we can expose it more sensibly. +#[derive(Clone, Debug)] +pub struct Utf8CString { + /// The body of this string. + /// + /// # Safety + /// + /// INVARIANT: This string must be valid UTF-8. + /// + /// (We do not _yet_ depend on this invariant for safety in our rust code, but we do promise in + /// our C ffi that it will hold.) + string: Box<CStr>, +} + +impl AsRef<CStr> for Utf8CString { + fn as_ref(&self) -> &CStr { + &self.string + } +} + +impl AsRef<str> for Utf8CString { + fn as_ref(&self) -> &str { + // TODO: We might someday decide to implement this using unsafe methods, to avoid walking + // over the string to enforce properties that are already there. + self.string.to_str().expect("Utf8CString was not UTF-8‽") + } +} + +// TODO: In theory we could have an unchecked version of this function, if we are 100% +// sure that serde_json will reject every string that contains a NUL. But let's not do +// that unless the NUL check shows up in profiles. +impl TryFrom<String> for Utf8CString { + type Error = NulError; + + fn try_from(value: String) -> Result<Self, Self::Error> { + // Safety: Since `value` is a `String`, it is guaranteed to be UTF-8. + Ok(Utf8CString { + string: CString::new(value)?.into_boxed_c_str(), + }) + } +} + +/// Ffi-related functionality for Utf8CStr +#[cfg(feature = "ffi")] +pub(crate) mod ffi { + use std::ffi::c_char; + + impl super::Utf8CString { + /// Expose this Utf8CStr as a C string. + pub(crate) fn as_ptr(&self) -> *const c_char { + self.string.as_ptr() + } + } +} diff --git a/maint/cbindgen b/maint/cbindgen new file mode 100755 index 000000000..895a71ac2 --- /dev/null +++ b/maint/cbindgen @@ -0,0 +1,172 @@ +#!/usr/bin/env bash +# +# Run cbindgen in each appropriate crate (the ones with a ffi interface), +# and inspect the output for changes. +# +# This script will, by default, replace the headers +# if the warnings have not changed. +# You can replace the old headers _and_ the old saved warnings +# with the "--force" option. +# You can use "--check" to make any changes (to headers or warnings) a failure. +# +# Our cbindgen configurations require nightly rust—otherwise, we can't +# expand macros. If $RUSTC_NIGHTLY is set, or if $RUSTC is nightly, +# or if "rustc" is nightly, we'll use that. Otherwise, we'll try to +# ask `rustup` if there is a nightly toolchain. + +# TODO: Use maint/common helpers once they are merged. + +set -eou pipefail + +TOPLEVEL=$(realpath "$(dirname "$0")"/..) + +: "${CBINDGEN_RUSTC:=}" +: "${RUSTC:=rustc}" + +replace=1 +emit_all_warnings=0 +fail_on_change=0 +overwrite_warnings=0 + +function usage() +{ + cat <<EOF +cbindgen: Run cbindgen in appropriate crates, and inspect outputs for changes. + +Usage: + cbindgen [opts] + +Options: + -h: Print this message. + -v: Do not suppress expected warnings. + + -f: Overwrite files, even if there are new warnings. + -c: Give a nonzero exit status if any files have changed; do not overwrite. +EOF +} + +set +e +options=$(getopt -ohfvc --long help,force,verbose,check -- "$@") +status="$?" +set -e +if test "$status" != 0; then + usage + exit 1 +fi + +eval set -- "$options" + +while true; do + case "$1" in + -h | --help) + usage + exit 1 + ;; + -f | --force) + overwrite_warnings=1 + shift + ;; + -v | --verbose) + emit_all_warnings=1 + shift + ;; + -c | --check) + fail_on_change=0 + replace=0 + shift + ;; + --) + shift + break + ;; + esac +done + +# First, find a suitable rustc. +if test "$CBINDGEN_RUSTC" != ""; then + # Supposedly we have one. Use that. + : +elif $RUSTC --version |grep -q nightly; then + # Our rustc is apparently nightly. That'll do. + CBINDGEN_RUSTC="$RUSTC" +else + # Maybe rustup can help us? + # + # TODO: If we start to see differences depending on the version + # of nightly, we may want to change this to instead look at a + # particular version of nightly. + : "${CBINDGEN_TOOLCHAIN:=nightly}" + CBINDGEN_RUSTC=$(rustup "+$CBINDGEN_TOOLCHAIN" which rustc) +fi + +ANY_DIFFERENCES=0 +SUGGEST_FORCE=0 + +for cratedir in $("$TOPLEVEL/maint/list_crates" --subdir); do + crate=$(basename "$cratedir") + if ! test -e "$cratedir/cbindgen.toml"; then + # no cbindgen.toml here; we don't run in this crate. + continue + fi + + echo "$crate ..." + pushd "$TOPLEVEL/$cratedir">/dev/null + + set +e + RUSTC="${CBINDGEN_RUSTC}" cbindgen --lockfile ../../Cargo.lock >"$crate.h.tmp" 2>cbindgen.warnings.tmp + status="$?" + set -e + if test "$emit_all_warnings" = 1 || test "$status" != 0; then + cat cbindgen.warnings.tmp 1>&2 + if "$status" != 0; then + echo "cbindgen failed with status $status: See warnings above." + exit "$status" + fi + fi + + changed_this_time=0 + new_warnings_this_time=0 + if diff -uN "$crate.h" "$crate.h.tmp"; then + echo " (No changes in the header)" + else + changed_this_time=1 + ANY_DIFFERENCES=1 + fi + if diff -uN "cbindgen.warnings" "cbindgen.warnings.tmp"; then + echo " (No changes in the warnings)" + else + new_warnings_this_time=1 + changed_this_time=1 + ANY_DIFFERENCES=1 + fi + + if test "$changed_this_time" = 1 && test "$replace" = 1; then + if test "$new_warnings_this_time" = 1 && test "$overwrite_warnings" = 0; then + # TODO: Possibly we should only refuse to overwrite when there + # are _new_ warnings; not when warnings have gone away or been + # re-ordered. + echo " CHANGED WARNINGS; not overwriting." + SUGGEST_FORCE=1 + else + echo " Overwriting warnings and header file." + mv -f cbindgen.warnings.tmp cbindgen.warnings + mv -f "$crate.h.tmp" "$crate.h" + fi + fi + + popd>/dev/null +done + +if test "$ANY_DIFFERENCES" = 1; then + echo "------------------------------" + echo "At least one file changed; see above for diffs." + if test "$replace" = 0; then + echo "(Running in 'check' mode, so nothing was changed.)" + elif test "$SUGGEST_FORCE" = 1; then + echo "(Did not overwrite files in which the warnings changed.)" + echo "Make sure to look over the list of warnings, and then run with -f" + fi + if test "$fail_on_change" = 1; then + exit 2 + fi +fi diff --git a/maint/list_crates b/maint/list_crates index bcea8fcc4..71a7643a8 100755 --- a/maint/list_crates +++ b/maint/list_crates @@ -17,6 +17,7 @@ WORKSPACE_TOML = os.path.join(TOPDIR, "Cargo.toml") class Crate: '''Information about one crate''' name: str + subdir: str publish: bool version: str raw_metadata: Any @@ -26,14 +27,14 @@ def list_crates() -> Iterator[Crate]: Iterator over all the crates in the workspace. ''' t = toml.decoder.load(WORKSPACE_TOML) - for path in t['workspace']['members']: - pt = toml.decoder.load(path + "/Cargo.toml") + for subdir in t['workspace']['members']: + pt = toml.decoder.load(subdir + "/Cargo.toml") package = pt["package"]["name"] publish = pt["package"].get("publish") version = pt["package"]["version"] if publish is None: publish = True - yield Crate(package, publish, version, pt) + yield Crate(package, subdir, publish, version, pt) def print_crates(args: argparse.Namespace) -> None: shown_any = False @@ -44,6 +45,8 @@ def print_crates(args: argparse.Namespace) -> None: shown_any = True if args.version: print("%-23s %s" % (c.name, c.version)) + elif args.subdir: + print(c.subdir) else: print(c.name) @@ -56,6 +59,7 @@ if __name__ == '__main__': ) parser.add_argument('--all', action='store_true', help='list even unpublished crates') parser.add_argument('--version', action='store_true', help='print versions') + parser.add_argument('--subdir', action='store_true', help="print relative directories") parser.add_argument('-p', '--package', help='specific crate') args = parser.parse_args() |