Error handling / how functions return strings

[nyurik]

All string-returning Varnish functions must store strings in a workspace. A macro-generated wrapper does this for String/&str/CString/&CStr values. This results in several failure modes that we might want to rethink. The main issue is that some failures occur after the Rust function exits, thus developer has no way to deal with the errors.

String contains \0

Rust strings may contain NULL characters. The wrapper can either panic, abort the task (responds with HTTP 500?), or truncate the string, dropping anything after the NULL character. Affected return types: String, &str

String is too big to fit into workspace

Running WS_Alloc(ws, size) C function may result in NULL, i.e. out of memory. This might be due to the string being too big, or other issues. The only options here is to panic or abort the task. This affects all string types like String, &str, CString, and &CStr, with the only exception being VCL_STRING because it has been created by the Rust developer in a workspace already. Note that this also affects IP addresses and probes.

Incorrect VCL_STRING creation

VCL_STRING offers a way to handle the above two problems in the Rust code because vmod author can copy content into the workspace directly. The issue is that this struct is defined as pub struct VCL_STRING(pub *const ::std::ffi::c_char) in ffi module, so it can be created incorrectly without the unsafe keyword:

fn foo() -> VCL_STRING {
    let s = CString::from(c"hello");
    VCL_STRING(s.as_ptr())  // s will be dropped on exit - dangling pointer
}

In other words, we do not want VCL_STRING to be instantiated in a safe code. Unfortunately, I cannot make bindgen to generate VCL_STRING(pub(crate) *c_char) - it always creates pub, so I may need to do a workaround - bindgen will create PUB_VCL_STRING (or some other name), and I will implement another VCL_STRING type with unsafe constructor.

Possible solutions

vmod-level attribute settings

We can introduce a new parameter on the #[varnish::vmod] attribute with some sensible defaults:

• on_oom=panic|abort_task - action if WS_Alloc(ws, size) returns NULL. Defaults to abort_task
• on_null_in_str=panic|abort_task|trim - action if a Rust function returns a string with NULL. Not certain of the default.

Encourage Workspace usage

We could encourage users to work directly with the workspace -- which will return VCL_STRING - returnable to Varnish. This way it will be user's responsibility to convert their strings to &CStr, and thus deal with both OOM and NULL characters.

fn get_string_or_err(ws: &mut Workspace) -> Result<VCL_STRING, VclError> {
  // store fn returns a safe value to return, or will fail if OOM
  ws.store_c_str(c"something")
}

fn get_string(ws: &mut Workspace) -> VCL_STRING {
  // same as above, but the user can handle errors (in this case - panic on OOM)
  ws.store_c_str(c"something").unwrap()
}

[nyurik]

P.S. I submitted a PR for bindgen to allow custom accessibility generation. See rust-lang/rust-bindgen#2967