Skip to content

Commit

Permalink
Don't allow directly parsing Listeners from strings
Browse files Browse the repository at this point in the history
Clap parses the arguments twice and that causes issues with systemd
activation file descriptors if the parsing logic is not pure.

The solution is to allow parsing Bindings and make the conversion to
Listeners an explicit action that happens only once.

See: clap-rs/clap#3589
Signed-off-by: Wiktor Kwapisiewicz <wiktor@metacode.biz>
  • Loading branch information
wiktor-k committed May 10, 2022
1 parent c5ae2e4 commit 7f7c193
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 31 deletions.
12 changes: 6 additions & 6 deletions README.md
Expand Up @@ -12,13 +12,13 @@ By design this crate has no dependencies other than what is in `std`.
### Simple parsing

```rust
use service_binding::Listener;
use service_binding::{Binding, Listener};

let host = "tcp://127.0.0.1:8012"; // or "unix:///tmp/socket"

let listener = host.parse().unwrap();
let binding: Binding = host.parse().unwrap();

match listener {
match binding.try_into().unwrap() {
Listener::Unix(listener) => {
// bind to a unix domain socket
},
Expand All @@ -38,12 +38,12 @@ a TCP port:
```rust,no_run
use actix_web::{web, App, HttpServer, Responder};
use clap::Parser;
use service_binding::Listener;
use service_binding::{Binding, Listener};
#[derive(Parser, Debug)]
struct Args {
#[clap(env = "HOST", short = 'H', long, default_value = "tcp://127.0.0.1:8080")]
host: Listener,
host: Binding,
}
async fn greet() -> impl Responder {
Expand All @@ -56,7 +56,7 @@ async fn main() -> std::io::Result<()> {
App::new().route("/", web::get().to(greet))
});
match Args::parse().host {
match Args::parse().host.try_into()? {
Listener::Unix(listener) => server.listen_uds(listener)?,
Listener::Tcp(listener) => server.listen(listener)?,
}.run().await
Expand Down
37 changes: 12 additions & 25 deletions src/service.rs
Expand Up @@ -40,8 +40,9 @@ pub enum Binding {
/// # Examples
///
/// ```
/// # use service_binding::Listener;
/// let listener: Listener = "unix:///tmp/socket".parse().unwrap();
/// # use service_binding::{Binding, Listener};
/// let binding: Binding = "unix:///tmp/socket".parse().unwrap();
/// let listener = binding.try_into().unwrap();
/// assert!(matches!(listener, Listener::Unix(_)));
/// ```
#[derive(Debug)]
Expand Down Expand Up @@ -107,16 +108,20 @@ impl<'a> std::convert::TryFrom<&'a str> for Binding {
}
}

impl std::str::FromStr for Binding {
type Err = Error;

fn from_str(s: &str) -> Result<Self, Self::Err> {
s.try_into()
}
}

impl TryFrom<Binding> for Listener {
type Error = Error;
type Error = std::io::Error;

fn try_from(value: Binding) -> Result<Self, Self::Error> {
match value {
Binding::FileDescriptor(descriptor) => {
if descriptor != 3 {
return Err(Error);
}

use std::os::unix::io::FromRawFd;

Ok(unsafe { UnixListener::from_raw_fd(descriptor) }.into())
Expand All @@ -131,24 +136,6 @@ impl TryFrom<Binding> for Listener {
}
}

impl<'a> std::convert::TryFrom<&'a str> for Listener {
type Error = Error;

fn try_from(s: &'a str) -> Result<Self, Self::Error> {
let binding: Binding = s.try_into()?;
binding.try_into()
}
}

impl std::str::FromStr for Listener {
type Err = Error;

fn from_str(s: &str) -> Result<Self, Self::Err> {
let binding: Binding = s.try_into()?;
binding.try_into()
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down

0 comments on commit 7f7c193

Please sign in to comment.