Proxy: Improve error reporting for invalid environment variables (#23)

* Proxy: Improve error reporting for invalid environment variables

Previously when an environment variable had an invalid value the
process would exit with an error that did not mention which
environment variable is invalid.

Start fixing this by routing environment variable parsing through
functions that always know the name of the environment variable when
they report errors.

I validated this change manually.

* Proxy: Improve configuration URL parsing

Previously there was a bit of duplicated logic between parsing `Addr`
and `HostAndPort` values.

Factor out the common logic. In the process, improve the error
reporting in the cases where parsing fails.
This commit is contained in:
Brian Smith 2017-12-08 12:32:43 -06:00 committed by GitHub
parent 205e4d4915
commit b4ace4642a
1 changed files with 86 additions and 88 deletions

View File

@ -61,9 +61,19 @@ pub struct Addr(SocketAddr);
/// Errors produced when loading a `Config` struct.
#[derive(Clone, Debug)]
pub enum Error {
InvalidAddr,
ControlPlaneConfigError(String, UrlError),
NotANumber(String),
InvalidEnvVar {
name: String,
value: Option<String>,
parse_error: ParseError,
},
}
#[derive(Clone, Debug)]
pub enum ParseError {
NotANumber,
HostIsNotAnIpAddress,
NotUnicode,
UrlError(UrlError),
}
#[derive(Clone, Copy, Debug)]
@ -119,52 +129,41 @@ const DEFAULT_RESOLV_CONF: &str = "/etc/resolv.conf";
impl Config {
/// Load a `Config` by reading ENV variables.
pub fn load_from_env() -> Result<Self, Error> {
let event_buffer_capacity = match env::var(ENV_EVENT_BUFFER_CAPACITY).ok() {
None => DEFAULT_EVENT_BUFFER_CAPACITY,
Some(c) => match c.parse() {
Ok(c) => c,
Err(_) => return Err(Error::NotANumber(c)),
},
};
let event_buffer_capacity = env_var_parse(ENV_EVENT_BUFFER_CAPACITY, parse_number)?
.unwrap_or(DEFAULT_EVENT_BUFFER_CAPACITY);
let metrics_flush_interval = match env::var(ENV_METRICS_FLUSH_INTERVAL_SECS).ok() {
None => Duration::from_secs(DEFAULT_METRICS_FLUSH_INTERVAL_SECS),
Some(c) => match c.parse() {
Ok(c) => Duration::from_secs(c),
Err(_) => return Err(Error::NotANumber(c)),
},
};
let metrics_flush_interval = Duration::from_secs(
env_var_parse(ENV_METRICS_FLUSH_INTERVAL_SECS, parse_number)?
.unwrap_or(DEFAULT_METRICS_FLUSH_INTERVAL_SECS));
Ok(Config {
private_listener: Listener {
addr: Addr::from_env_or(ENV_PRIVATE_LISTENER, DEFAULT_PRIVATE_LISTENER)?,
addr: env_var_parse(ENV_PRIVATE_LISTENER, str::parse)?
.unwrap_or_else(|| Addr::from_str(DEFAULT_PRIVATE_LISTENER).unwrap()),
},
public_listener: Listener {
addr: Addr::from_env_or(ENV_PUBLIC_LISTENER, DEFAULT_PUBLIC_LISTENER)?,
addr: env_var_parse(ENV_PUBLIC_LISTENER, str::parse)?
.unwrap_or_else(|| Addr::from_str(DEFAULT_PUBLIC_LISTENER).unwrap()),
},
control_listener: Listener {
addr: Addr::from_env_or(ENV_CONTROL_LISTENER, DEFAULT_CONTROL_LISTENER)?,
addr: env_var_parse(ENV_CONTROL_LISTENER, str::parse)?
.unwrap_or_else(|| Addr::from_str(DEFAULT_CONTROL_LISTENER).unwrap()),
},
private_forward: Addr::from_env_opt(ENV_PRIVATE_FORWARD)?,
private_forward: env_var_parse(ENV_PRIVATE_FORWARD, str::parse)?,
public_connect_timeout: env::var(ENV_PUBLIC_CONNECT_TIMEOUT)
.ok()
.and_then(|c| c.parse().ok())
public_connect_timeout: env_var_parse(ENV_PUBLIC_CONNECT_TIMEOUT, parse_number)?
.map(Duration::from_millis),
private_connect_timeout: env::var(ENV_PRIVATE_CONNECT_TIMEOUT)
.ok()
.and_then(|c| c.parse().ok())
private_connect_timeout: env_var_parse(ENV_PRIVATE_CONNECT_TIMEOUT, parse_number)?
.map(Duration::from_millis),
resolv_conf_path: env::var(ENV_RESOLV_CONF)
.unwrap_or_else(|_| DEFAULT_RESOLV_CONF.into())
resolv_conf_path: env_var(ENV_RESOLV_CONF)?
.unwrap_or(DEFAULT_RESOLV_CONF.into())
.into(),
control_host_and_port: control_host_and_port_from_env(
ENV_CONTROL_URL,
DEFAULT_CONTROL_URL,
)?,
control_host_and_port: env_var_parse(ENV_CONTROL_URL, parse_url)?
.unwrap_or_else(|| parse_url(DEFAULT_CONTROL_URL).unwrap()),
event_buffer_capacity,
metrics_flush_interval,
})
@ -173,41 +172,23 @@ impl Config {
// ===== impl Addr =====
impl Addr {
fn from_env_opt(key: &str) -> Result<Option<Addr>, Error> {
match env::var(key) {
Ok(a) => a.parse().map(Some),
Err(_) => Ok(None),
}
}
fn from_env_or(key: &str, default: &str) -> Result<Addr, Error> {
let s = env::var(key).unwrap_or_else(|_| default.into());
s.parse()
}
}
impl FromStr for Addr {
type Err = Error;
type Err = ParseError;
fn from_str(s: &str) -> Result<Self, Self::Err> {
match Url::parse(s) {
Err(_) => Err(Error::InvalidAddr),
Ok(u) => match u.scheme() {
"tcp" => match u.with_default_port(|_| Err(())) {
Ok(HostAndPort {
host: Host::Ipv4(ip),
port,
}) => Ok(Addr(SocketAddr::new(ip.into(), port))),
Ok(HostAndPort {
host: Host::Ipv6(ip),
port,
}) => Ok(Addr(SocketAddr::new(ip.into(), port))),
_ => Err(Error::InvalidAddr),
},
_ => Err(Error::InvalidAddr),
},
match parse_url(s)? {
HostAndPort {
host: Host::Ipv4(ip),
port,
} => Ok(Addr(SocketAddr::new(ip.into(), port))),
HostAndPort {
host: Host::Ipv6(ip),
port,
} => Ok(Addr(SocketAddr::new(ip.into(), port))),
HostAndPort {
host: Host::Domain(_),
..
} => Err(ParseError::HostIsNotAnIpAddress),
}
}
}
@ -218,39 +199,56 @@ impl From<Addr> for SocketAddr {
}
}
fn control_host_and_port_from_env(key: &str, default: &str) -> Result<HostAndPort, Error> {
let s = env::var(key).unwrap_or_else(|_| default.into());
let url = Url::parse(&s).map_err(|_| {
Error::ControlPlaneConfigError(s.clone(), UrlError::SyntaxError)
})?;
fn parse_number<T>(s: &str) -> Result<T, ParseError> where T: FromStr {
s.parse().map_err(|_| ParseError::NotANumber)
}
fn parse_url(s: &str) -> Result<HostAndPort, ParseError> {
let url = Url::parse(&s).map_err(|_| ParseError::UrlError(UrlError::SyntaxError))?;
let host = url.host()
.ok_or_else(|| {
Error::ControlPlaneConfigError(s.clone(), UrlError::MissingHost)
})?
.ok_or_else(|| ParseError::UrlError(UrlError::MissingHost))?
.to_owned();
if url.scheme() != "tcp" {
return Err(Error::ControlPlaneConfigError(
s.clone(),
UrlError::UnsupportedScheme,
));
return Err(ParseError::UrlError(UrlError::UnsupportedScheme));
}
let port = url.port().ok_or_else(|| {
Error::ControlPlaneConfigError(s.clone(), UrlError::MissingPort)
})?;
let port = url.port().ok_or_else(|| ParseError::UrlError(UrlError::MissingPort))?;
if url.path() != "/" {
return Err(Error::ControlPlaneConfigError(
s.clone(),
UrlError::PathNotAllowed,
));
return Err(ParseError::UrlError(UrlError::PathNotAllowed));
}
if url.fragment().is_some() {
return Err(Error::ControlPlaneConfigError(
s.clone(),
UrlError::FragmentNotAllowed,
));
return Err(ParseError::UrlError(UrlError::FragmentNotAllowed));
}
Ok(HostAndPort {
host,
port,
})
}
fn env_var(name: &str) -> Result<Option<String>, Error> {
match env::var(name) {
Ok(value) => Ok(Some(value)),
Err(env::VarError::NotPresent) => Ok(None),
Err(env::VarError::NotUnicode(_)) => Err(Error::InvalidEnvVar {
name: name.to_owned(),
value: None,
parse_error: ParseError::NotUnicode,
}),
}
}
fn env_var_parse<T, Parse>(name: &str, parse: Parse) -> Result<Option<T>, Error>
where Parse: FnOnce(&str) -> Result<T, ParseError> {
match env_var(name)? {
Some(ref s) => {
let r = parse(s).map_err(|parse_error| {
Error::InvalidEnvVar {
name: name.to_owned(),
value: Some(s.to_owned()),
parse_error,
}
})?;
Ok(Some(r))
},
None => Ok(None),
}
}