Skip to content

Update network proxy to Rama 0.3.0 - #1034

Open
GraciousGazelles wants to merge 1 commit into
mainfrom
fix/w14939-rama03-network-proxy-20261112
Open

GraciousGazelles wants to merge 1 commit into
mainfrom
fix/w14939-rama03-network-proxy-20261112

Conversation

@GraciousGazelles

Copy link
Copy Markdown
Collaborator

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.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

Comment on lines +23 to +37
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))
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

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()
}

This branch has not been deployed

No deployments
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.

1 participant