Repository navigation
Update network proxy to Rama 0.3.0 - #1034
GraciousGazelles wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the rama dependencies to version 0.3.0 and refactors the codebase to align with the new APIs. Key changes include replacing ExtensionsMut with ExtensionsRef, migrating TCP requests to network requests, and integrating a system DNS resolver (HickoryDnsResolver) into the TCP connector. Feedback on the changes highlights a potential race condition in the system_hickory_dns_resolver function where concurrent threads could redundantly initialize the resolver; using OnceLock::get_or_try_init is recommended to ensure thread-safe, single-instance initialization.
| fn system_hickory_dns_resolver() -> Result<HickoryDnsResolver, BoxError> { | ||
| static RESOLVER: OnceLock<HickoryDnsResolver> = OnceLock::new(); | ||
|
|
||
| if let Some(resolver) = RESOLVER.get() { | ||
| return Ok(resolver.clone()); | ||
| } | ||
|
|
||
| let resolver = HickoryDnsResolver::try_new_system().map_err(|err| { | ||
| OpaqueError::from_boxed(err) | ||
| .context("initialize system Hickory DNS resolver") | ||
| .into_boxed() | ||
| })?; | ||
| let _ = RESOLVER.set(resolver.clone()); | ||
| Ok(RESOLVER.get().cloned().unwrap_or(resolver)) | ||
| } |
There was a problem hiding this comment.
The current implementation of system_hickory_dns_resolver has a race condition where multiple threads calling this function concurrently when RESOLVER is uninitialized will all invoke HickoryDnsResolver::try_new_system(). Since initializing a system DNS resolver typically involves reading system configuration files and spawning background tasks/threads, this redundant initialization can be expensive and wasteful.
Using OnceLock::get_or_try_init (stabilized in Rust 1.80.0) resolves this race condition by ensuring that only a single thread performs the initialization while other threads block and wait for the result.
fn system_hickory_dns_resolver() -> Result<HickoryDnsResolver, BoxError> {
static RESOLVER: OnceLock<HickoryDnsResolver> = OnceLock::new();
RESOLVER
.get_or_try_init(|| {
HickoryDnsResolver::try_new_system().map_err(|err| {
OpaqueError::from_boxed(err)
.context("initialize system Hickory DNS resolver")
.into_boxed()
})
})
.cloned()
}
Updates the network proxy from Rama 0.3.0-alpha.4 to the maintained 0.3.0 API.
The connector migration keeps Hickory DNS selected explicitly from system resolver configuration instead of accepting Rama's native-resolver default. The direct-dial path still checks every resolved socket address against the existing non-public-address policy; proxy-address routing remains separate. No advisory suppressions are added.
The Cargo lock is intentionally left for the repository's existing hosted lock-generation path. This is not security-cleared until that lock and the existing cargo-deny and focused network-proxy checks pass on this exact head.