Skip to content

Commit

Permalink
Set SO_KEEPALIVE for IPA connections
Browse files Browse the repository at this point in the history
See private-attribution#758 for details
  • Loading branch information
akoshelev committed Aug 9, 2023
1 parent 9d0a1b1 commit bc879ad
Show file tree
Hide file tree
Showing 5 changed files with 121 additions and 36 deletions.
15 changes: 2 additions & 13 deletions src/cli/ipa_output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,9 @@ pub struct QueryResult {
pub input_size: QuerySize,
pub config: IpaQueryConfig,
#[serde(
serialize_with = "duration_to_secs",
deserialize_with = "secs_to_duration"
serialize_with = "crate::serde::duration::to_secs",
deserialize_with = "crate::serde::duration::from_secs"
)]
pub latency: Duration,
pub breakdowns: Vec<u32>,
}

#[cfg(feature = "enable-serde")]
pub fn duration_to_secs<S: serde::Serializer>(d: &Duration, s: S) -> Result<S::Ok, S::Error> {
s.serialize_f64(d.as_secs_f64())
}

#[cfg(feature = "enable-serde")]
pub fn secs_to_duration<'de, D: serde::Deserializer<'de>>(d: D) -> Result<Duration, D::Error> {
let secs = serde::Deserialize::deserialize(d)?;
Ok(Duration::from_secs_f64(secs))
}
82 changes: 75 additions & 7 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use std::{
iter::Zip,
path::PathBuf,
slice,
time::Duration,
};

#[derive(Debug, thiserror::Error)]
Expand Down Expand Up @@ -105,7 +106,7 @@ impl NetworkConfig {
#[derive(Clone, Debug, Deserialize)]
pub struct PeerConfig {
/// Peer URL
#[serde(with = "crate::uri")]
#[serde(with = "crate::serde::uri")]
pub url: Uri,

/// Peer's TLS certificate
Expand Down Expand Up @@ -279,6 +280,28 @@ pub trait HyperClientConfigurator {
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct ClientConfig {
pub http_config: HttpClientConfigurator,
/// Enable SO_KEEPALIVE. Default value is 90 seconds to match [`Hyper`] value. Note that because
/// IPA builds [`http`] connector manually, keep-alive is not enabled by Hyper. It is somewhat
/// confusing that Hyper turns it on inside [`build_http`] method only.
///
/// ## Serialization notes
///
/// IPA uses TOML for configuration files that does not support "unsetting a key": [`toml_issue`].
/// For this reason, if value is not present in the configuration file, it will be set to `None`.
/// It is up to the config creator to ensure that value is specified when `network.toml` is created.
///
/// [`Hyper`]: https://docs.rs/hyper/0.14.27/hyper/client/struct.Builder.html#method.pool_idle_timeout
/// [`http`]: https://docs.rs/hyper/0.14.27/hyper/client/struct.Builder.html#method.build
/// [`build_http`]: https://docs.rs/hyper/0.14.27/hyper/client/struct.Builder.html#method.build_http
/// [`toml_issue`]: https://github.com/toml-lang/toml/issues/30
#[serde(
rename = "keep_alive_interval_secs",
default,
serialize_with = "crate::serde::duration::to_secs",
deserialize_with = "crate::serde::duration::from_secs_optional",
skip_serializing_if = "Option::is_none"
)]
pub keep_alive_interval: Option<Duration>,
}

impl Default for ClientConfig {
Expand All @@ -288,18 +311,32 @@ impl Default for ClientConfig {
}

impl ClientConfig {
#[must_use]
pub fn use_http2() -> Self {
fn new(http_config: HttpClientConfigurator) -> Self {
Self {
http_config: HttpClientConfigurator::http2(),
http_config,
keep_alive_interval: Self::default_keep_alive_interval(),
}
}

#[allow(clippy::unnecessary_wraps)] // default value may be None
fn default_keep_alive_interval() -> Option<Duration> {
Some(Duration::from_secs(90))
}

#[must_use]
pub fn use_http2() -> Self {
Self::new(HttpClientConfigurator::http2())
}

#[must_use]
pub fn use_http1() -> Self {
Self {
http_config: HttpClientConfigurator::http1(),
}
Self::new(HttpClientConfigurator::http1())
}

#[must_use]
pub fn with_keep_alive_interval<I: Into<Option<Duration>>>(mut self, interval: I) -> Self {
self.keep_alive_interval = interval.into();
self
}
}

Expand Down Expand Up @@ -379,6 +416,7 @@ impl Debug for Http2Configurator {

#[cfg(all(test, unit_test))]
mod tests {
use super::*;
use crate::{config::HpkeClientConfig, helpers::HelperIdentity, net::test::TestConfigBuilder};
use hpke::{kem::X25519HkdfSha256, Kem};
use hyper::Uri;
Expand Down Expand Up @@ -416,4 +454,34 @@ mod tests {
let config = HpkeClientConfig { public_key };
assert_eq!(format!("{config:?}"), "HpkeClientConfig { public_key: \"2bd9da78f01d8bc6948bbcbe44ec1e7163d05083e267d110cdb2e75d847e3b6f\" }");
}

#[test]
fn client_config_serde() {
fn assert_config_eq(config_str: &str, expected: &ClientConfig) {
let actual: ClientConfig = serde_json::from_str(config_str).unwrap();
assert_eq!(actual.keep_alive_interval, expected.keep_alive_interval);

match (&expected.http_config, &actual.http_config) {
(HttpClientConfigurator::Http1(_), HttpClientConfigurator::Http1(_))
| (HttpClientConfigurator::Http2(_), HttpClientConfigurator::Http2(_)) => {}
_ => panic!(
"http config is not the same: {:?} vs {:?}",
expected.http_config, actual.http_config
),
};
}

assert_config_eq(
r#"{ "http_config": { "version": "http2" } }"#,
&ClientConfig::use_http2().with_keep_alive_interval(None),
);
assert_config_eq(
r#"{ "http_config": { "version": "http1" } }"#,
&ClientConfig::use_http1().with_keep_alive_interval(None),
);
assert_config_eq(
r#"{ "http_config": { "version": "http2" }, "keep_alive_interval_secs": 132 }"#,
&ClientConfig::use_http2().with_keep_alive_interval(Duration::from_secs(132)),
);
}
}
4 changes: 2 additions & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,15 @@ pub mod query;
pub mod report;
pub mod secret_sharing;
pub mod telemetry;
#[cfg(all(feature = "enable-serde", feature = "web-app"))]
pub mod uri;

#[cfg(any(test, feature = "test-fixture"))]
pub mod test_fixture;

mod app;
mod exact;
mod seq_join;
#[cfg(feature = "enable-serde")]
mod serde;

pub use app::{HelperApp, Setup as AppSetup};

Expand Down
42 changes: 42 additions & 0 deletions src/serde/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
//! Serialization helpers for Serde
#[cfg(feature = "web-app")]
pub mod uri {
use hyper::Uri;
use serde::{de::Error, Deserialize, Deserializer};

/// # Errors
/// if deserializing from string fails, or if string is not a [`Uri`]
pub fn deserialize<'de, D: Deserializer<'de>>(deserializer: D) -> Result<Uri, D::Error> {
let s: String = Deserialize::deserialize(deserializer)?;
s.parse().map_err(D::Error::custom)
}
}

pub mod duration {
use std::time::Duration;

pub fn to_secs<'a, I, S>(d: I, s: S) -> Result<S::Ok, S::Error>
where
Option<&'a Duration>: From<I>,
S: serde::Serializer,
{
let d = d.into();
match d {
Some(v) => s.serialize_f64(v.as_secs_f64()),
None => s.serialize_none(),
}
}

pub fn from_secs<'de, D: serde::Deserializer<'de>>(d: D) -> Result<Duration, D::Error> {
let secs = serde::Deserialize::deserialize(d)?;
Ok(Duration::from_secs_f64(secs))
}

pub fn from_secs_optional<'de, D: serde::Deserializer<'de>>(
d: D,
) -> Result<Option<Duration>, D::Error> {
let secs: Option<f64> = serde::Deserialize::deserialize(d)?;
Ok(secs.map(Duration::from_secs_f64))
}
}
14 changes: 0 additions & 14 deletions src/uri.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1 @@
use hyper::Uri;
use serde::{de::Error, Deserialize, Deserializer, Serializer};

/// # Errors
/// if serializing to string fails
pub fn serialize<S: Serializer>(uri: &Uri, serializer: S) -> Result<S::Ok, S::Error> {
serializer.serialize_str(&uri.to_string())
}

/// # Errors
/// if deserializing from string fails, or if string is not a [`Uri`]
pub fn deserialize<'de, D: Deserializer<'de>>(deserializer: D) -> Result<Uri, D::Error> {
let s: String = Deserialize::deserialize(deserializer)?;
s.parse().map_err(D::Error::custom)
}

0 comments on commit bc879ad

Please sign in to comment.