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

feat(ext): introduce extensions, starting with ConnectionInfo #1592

Closed
wants to merge 1 commit into from

Conversation

seanmonstar
Copy link
Member

  • Adds client::Builder::set_conn_info to opt-in to having connection
    info added to Responses from clients.
  • Adds ext::ConnectionInfo that allows querying types (like a
    Response) for connection info.

Closes #1402


Still WIP...

- Adds `client::Builder::set_conn_info` to opt-in to having connection
  info added to `Response`s from clients.
- Adds `ext::ConnectionInfo` that allows querying types (like a
  `Response`) for connection info.

Closes #1402
@seanmonstar
Copy link
Member Author

Some pieces of information would likely be very common. The two that come to mind are remote_addr and local_addr (is knowing the locally bound port sufficiently useful to be it's own thing)? However, a custom transport could have arbitrary information it could wish to advertise (like some details specific to a certain TLS implementation). I can think of a few ways that all this could be exposed:

1. ConnectionInfo + optional "extra" info in extensions

Have the "common" information be specific fields, and allow a user to get them from a Response by the API exposed here, ConnectionInfo::get(&res). The "extra" custom transport data would just be available through the normal res.extensions() API.

  • Pros:
    • It's very likely most transports can set the common data, like remote_addr, and so if that is all a user is interested in, then changing transports won't affect the existence of the values.
  • Cons:
    • If a transport can provide both the common info, and wants to provide extra info, it results in multiple allocations, storing both Box<ConnectionInfo as Any> and Box<SomeTransportExtra as Any> separately in the extensions.

Example:

// a transport:
let info = Connected::new()
    .remote_addr(remote)
    .local_addr(local)
    .extra(RustlsExtraSomething::from(&conn));
// getting from a Response
let common = hyper::ext::ConnectionInfo::get(&res);
println!("remote={:?}, local={:?}", common.remote_addr(), common.local_addr());
let extra = res.extensions().get::<RustlsExtraSomething>();

2. A single custom "extra" info contains all the values a transport can provided.

Do away with the ConnectionInfo type proposed here, and just have all info from the transport exist as a single value in res.extensions().

  • Pros:
    • Reduces the amount of allocations needed to provide transport info in extensions to just 1.
  • Cons:
    • Changing transports means needing to remember to change what type is fetched from extensions.

Example:

// for a transport
let info = Connected::new()
    .extra(RustlsExtraSomething::from(&conn));
// getting from a Response
if let Some(extra) = res.extensions().get::<RustlsExtraSomething>() {
    println!(
        "remote = {:?}, local = {:?}, cert = {:?}",
        extra.remote,
        extra.local,
        extra.cert,
    );
}

@seanmonstar
Copy link
Member Author

Alternative 2 is explored in #1594.

@seanmonstar seanmonstar deleted the ext-conn-info branch July 17, 2018 17:36
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