Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement stub for inspect hostcall #417

Merged
merged 3 commits into from
Aug 16, 2024
Merged

Conversation

ulyssa
Copy link
Contributor

@ulyssa ulyssa commented Aug 16, 2024

This adds a small stub for the inspect hostcall that makes sure its input looks okay and then always returns an allow response. I've put the "allow" value into the Session so that in the future it should be possible to make it configurable, so that folks can change it to block when testing denied requests.

@ulyssa ulyssa requested review from elliottt, acw and kpfleming August 16, 2024 00:02
Copy link
Contributor

@elliottt elliottt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! There's only one change I'd recommend for the adapter, which is to rebuild the flags and config values for the wit-generated types.

ds_req,
ds_body,
info_mask,
info,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that info will need to be rebuilt as a fastly::api::http_req::InspectConfig. There's an example of the same over here:

macro_rules! make_vec {
($ptr_field:ident, $len_field:ident) => {
unsafe {
let len = usize::try_from((*config).$len_field).trapping_unwrap();
Vec::from_raw_parts((*config).$ptr_field as *mut _, len, len)
}
};
}
let config = http_req::DynamicBackendConfig {
host_override: make_vec!(host_override, host_override_len),
connect_timeout: unsafe { (*config).connect_timeout_ms },
first_byte_timeout: unsafe { (*config).first_byte_timeout_ms },
between_bytes_timeout: unsafe { (*config).between_bytes_timeout_ms },
ssl_min_version: unsafe { (*config).ssl_min_version }.try_into().ok(),
ssl_max_version: unsafe { (*config).ssl_max_version }.try_into().ok(),
cert_hostname: make_vec!(cert_hostname, cert_hostname_len),
ca_cert: make_vec!(ca_cert, ca_cert_len),
ciphers: make_vec!(ciphers, ciphers_len),
sni_hostname: make_vec!(sni_hostname, sni_hostname_len),
client_cert: make_vec!(client_certificate, client_certificate_len),
client_key: unsafe { (*config).client_key },
};
let res = http_req::register_dynamic_backend(name_prefix, target, options, &config);
std::mem::forget(config);
convert_result(res)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for pointing me to that! I've set up something similar for InspectConfigOptions and InspectConfig in af24988.

Copy link
Contributor

@elliottt elliottt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great!

@ulyssa ulyssa merged commit 6755370 into main Aug 16, 2024
15 checks passed
@ulyssa ulyssa deleted the ulyssa/inspect-hostcall-stub branch August 16, 2024 01:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants