From 8c35d38b148f9b0562d86ac4d47944288010812c Mon Sep 17 00:00:00 2001 From: Jake Pittis Date: Sun, 20 Jan 2019 10:31:10 +0000 Subject: [PATCH 1/5] Reformat all code with cargo fmt --- src/error.rs | 4 +- src/lib.rs | 113 +++++++++++++++++++++++++++------------ tests/iptables_test.rs | 118 +++++++++++++++++++++++++++++++++-------- 3 files changed, 178 insertions(+), 57 deletions(-) diff --git a/src/error.rs b/src/error.rs index 3f401ef..8111d1f 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,7 +1,7 @@ -extern crate regex; extern crate nix; +extern crate regex; -use std::{fmt, error, io, convert, num}; +use std::{convert, error, fmt, io, num}; /// Defines the general error type of iptables crate #[derive(Debug)] diff --git a/src/lib.rs b/src/lib.rs index 2c9c2ac..0751744 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -20,19 +20,19 @@ #[macro_use] extern crate lazy_static; -extern crate regex; extern crate nix; +extern crate regex; pub mod error; -use std::process::{Command, Output}; +use error::{IPTError, IPTResult}; +use nix::fcntl::{flock, FlockArg}; use regex::{Match, Regex}; -use error::{IPTResult, IPTError}; +use std::ffi::OsStr; use std::fs::File; use std::os::unix::io::AsRawFd; -use nix::fcntl::{flock, FlockArg}; +use std::process::{Command, Output}; use std::vec::Vec; -use std::ffi::OsStr; // List of built-in chains taken from: man 8 iptables const BUILTIN_CHAINS_FILTER: &'static [&'static str] = &["INPUT", "FORWARD", "OUTPUT"]; @@ -97,24 +97,38 @@ pub fn new(is_ipv6: bool) -> IPTResult { /// Creates a new `IPTables` Result with the command of 'iptables' if `is_ipv6` is `false`, otherwise the command is 'ip6tables'. #[cfg(target_os = "linux")] pub fn new(is_ipv6: bool) -> IPTResult { - let cmd = if is_ipv6 { - "ip6tables" - } else { - "iptables" - }; + let cmd = if is_ipv6 { "ip6tables" } else { "iptables" }; let version_output = Command::new(cmd).arg("--version").output()?; let re = Regex::new(r"v(\d+)\.(\d+)\.(\d+)")?; let version_string = String::from_utf8_lossy(&version_output.stdout).into_owned(); - let versions = re.captures(&version_string).ok_or("invalid version number")?; - let v_major = versions.get(1).ok_or("unable to get major version number")?.as_str().parse::()?; - let v_minor = versions.get(2).ok_or("unable to get minor version number")?.as_str().parse::()?; - let v_patch = versions.get(3).ok_or("unable to get patch version number")?.as_str().parse::()?; + let versions = re + .captures(&version_string) + .ok_or("invalid version number")?; + let v_major = versions + .get(1) + .ok_or("unable to get major version number")? + .as_str() + .parse::()?; + let v_minor = versions + .get(2) + .ok_or("unable to get minor version number")? + .as_str() + .parse::()?; + let v_patch = versions + .get(3) + .ok_or("unable to get patch version number")? + .as_str() + .parse::()?; Ok(IPTables { cmd: cmd, - has_check: (v_major > 1) || (v_major == 1 && v_minor > 4) || (v_major == 1 && v_minor == 4 && v_patch > 10), - has_wait: (v_major > 1) || (v_major == 1 && v_minor > 4) || (v_major == 1 && v_minor == 4 && v_patch > 19), + has_check: (v_major > 1) + || (v_major == 1 && v_minor > 4) + || (v_major == 1 && v_minor == 4 && v_patch > 10), + has_wait: (v_major > 1) + || (v_major == 1 && v_minor > 4) + || (v_major == 1 && v_minor == 4 && v_patch > 19), }) } @@ -123,25 +137,31 @@ impl IPTables { pub fn get_policy(&self, table: &str, chain: &str) -> IPTResult { let builtin_chains = get_builtin_chains(table)?; if !builtin_chains.iter().as_slice().contains(&chain) { - return Err(IPTError::Other("given chain is not a default chain in the given table, can't get policy")); + return Err(IPTError::Other( + "given chain is not a default chain in the given table, can't get policy", + )); } - let output = String::from_utf8_lossy(&self.run(&["-t", table, "-L", chain])?.stdout) - .into_owned(); + let output = + String::from_utf8_lossy(&self.run(&["-t", table, "-L", chain])?.stdout).into_owned(); for item in output.trim().split("\n") { let fields = item.split(" ").collect::>(); if fields.len() > 1 && fields[0] == "Chain" && fields[1] == chain { return Ok(fields[3].replace(")", "")); } } - Err(IPTError::Other("could not find the default policy for table and chain")) + Err(IPTError::Other( + "could not find the default policy for table and chain", + )) } /// Set the default policy for a table/chain. pub fn set_policy(&self, table: &str, chain: &str, policy: &str) -> IPTResult { let builtin_chains = get_builtin_chains(table)?; if !builtin_chains.iter().as_slice().contains(&chain) { - return Err(IPTError::Other("given chain is not a default chain in the given table, can't set policy")); + return Err(IPTError::Other( + "given chain is not a default chain in the given table, can't set policy", + )); } match self.run(&["-t", table, "-P", chain, policy]) { @@ -183,7 +203,13 @@ impl IPTables { /// Inserts `rule` in the `position` to the table/chain. /// Returns `true` if the rule is inserted. pub fn insert(&self, table: &str, chain: &str, rule: &str, position: i32) -> IPTResult { - match self.run(&[&["-t", table, "-I", chain, &position.to_string()], rule.split_quoted().as_slice()].concat()) { + match self.run( + &[ + &["-t", table, "-I", chain, &position.to_string()], + rule.split_quoted().as_slice(), + ] + .concat(), + ) { Ok(output) => Ok(output.status.success()), Err(err) => Err(err), } @@ -191,9 +217,15 @@ impl IPTables { /// Inserts `rule` in the `position` to the table/chain if it does not exist. /// Returns `true` if the rule is inserted. - pub fn insert_unique(&self, table: &str, chain: &str, rule: &str, position: i32) -> IPTResult { + pub fn insert_unique( + &self, + table: &str, + chain: &str, + rule: &str, + position: i32, + ) -> IPTResult { if self.exists(table, chain, rule)? { - return Err(IPTError::Other("the rule exists in the table/chain")) + return Err(IPTError::Other("the rule exists in the table/chain")); } self.insert(table, chain, rule, position) @@ -202,7 +234,13 @@ impl IPTables { /// Replaces `rule` in the `position` to the table/chain. /// Returns `true` if the rule is replaced. pub fn replace(&self, table: &str, chain: &str, rule: &str, position: i32) -> IPTResult { - match self.run(&[&["-t", table, "-R", chain, &position.to_string()], rule.split_quoted().as_slice()].concat()) { + match self.run( + &[ + &["-t", table, "-R", chain, &position.to_string()], + rule.split_quoted().as_slice(), + ] + .concat(), + ) { Ok(output) => Ok(output.status.success()), Err(err) => Err(err), } @@ -221,7 +259,7 @@ impl IPTables { /// Returns `true` if the rule is appended. pub fn append_unique(&self, table: &str, chain: &str, rule: &str) -> IPTResult { if self.exists(table, chain, rule)? { - return Err(IPTError::Other("the rule exists in the table/chain")) + return Err(IPTError::Other("the rule exists in the table/chain")); } self.append(table, chain, rule) @@ -325,7 +363,9 @@ impl IPTables { fn exists_old_version(&self, table: &str, chain: &str, rule: &str) -> IPTResult { match self.run(&["-t", table, "-S"]) { - Ok(output) => Ok(String::from_utf8_lossy(&output.stdout).into_owned().contains(&format!("-A {} {}", chain, rule))), + Ok(output) => Ok(String::from_utf8_lossy(&output.stdout) + .into_owned() + .contains(&format!("-A {} {}", chain, rule))), Err(err) => Err(err), } } @@ -352,14 +392,19 @@ impl IPTables { let mut need_retry = true; while need_retry { - match flock(file_lock.as_ref().unwrap().as_raw_fd(), FlockArg::LockExclusiveNonblock) { + match flock( + file_lock.as_ref().unwrap().as_raw_fd(), + FlockArg::LockExclusiveNonblock, + ) { Ok(_) => need_retry = false, - Err(e) => if e.errno() == nix::errno::EAGAIN { - // FIXME: may cause infinite loop - need_retry = true; - } else { - return Err(IPTError::Nix(e)); - }, + Err(e) => { + if e.errno() == nix::errno::EAGAIN { + // FIXME: may cause infinite loop + need_retry = true; + } else { + return Err(IPTError::Nix(e)); + } + } } } output = output_cmd.args(args).output()?; diff --git a/tests/iptables_test.rs b/tests/iptables_test.rs index 7f70d48..5f72b6e 100644 --- a/tests/iptables_test.rs +++ b/tests/iptables_test.rs @@ -10,17 +10,24 @@ fn test_new() { #[test] fn test_old() { - nat(iptables::IPTables{ - cmd: "iptables", - has_wait: false, - has_check: false, - }, "NATOLD", "NATOLD2"); + nat( + iptables::IPTables { + cmd: "iptables", + has_wait: false, + has_check: false, + }, + "NATOLD", + "NATOLD2", + ); - filter(iptables::IPTables{ - cmd: "iptables", - has_wait: false, - has_check: false, - }, "FILTEROLD"); + filter( + iptables::IPTables { + cmd: "iptables", + has_wait: false, + has_check: false, + }, + "FILTEROLD", + ); } fn nat(ipt: iptables::IPTables, old_name: &str, new_name: &str) { @@ -30,15 +37,49 @@ fn nat(ipt: iptables::IPTables, old_name: &str, new_name: &str) { assert_eq!(ipt.exists("nat", new_name, "-j ACCEPT").unwrap(), true); assert_eq!(ipt.delete("nat", new_name, "-j ACCEPT").unwrap(), true); assert_eq!(ipt.insert("nat", new_name, "-j ACCEPT", 1).unwrap(), true); - assert_eq!(ipt.append("nat", new_name, "-m comment --comment \"double-quoted comment\" -j ACCEPT").unwrap(), true); - assert_eq!(ipt.exists("nat", new_name, "-m comment --comment \"double-quoted comment\" -j ACCEPT").unwrap(), true); - assert_eq!(ipt.append("nat", new_name, "-m comment --comment 'single-quoted comment' -j ACCEPT").unwrap(), true); + assert_eq!( + ipt.append( + "nat", + new_name, + "-m comment --comment \"double-quoted comment\" -j ACCEPT" + ) + .unwrap(), + true + ); + assert_eq!( + ipt.exists( + "nat", + new_name, + "-m comment --comment \"double-quoted comment\" -j ACCEPT" + ) + .unwrap(), + true + ); + assert_eq!( + ipt.append( + "nat", + new_name, + "-m comment --comment 'single-quoted comment' -j ACCEPT" + ) + .unwrap(), + true + ); // The following `exists`-check has to use double-quotes, since the iptables output (if it // doesn't have the check-functionality) will use double quotes. - assert_eq!(ipt.exists("nat", new_name, "-m comment --comment \"single-quoted comment\" -j ACCEPT").unwrap(), true); + assert_eq!( + ipt.exists( + "nat", + new_name, + "-m comment --comment \"single-quoted comment\" -j ACCEPT" + ) + .unwrap(), + true + ); assert_eq!(ipt.flush_chain("nat", new_name).unwrap(), true); assert_eq!(ipt.exists("nat", new_name, "-j ACCEPT").unwrap(), false); - assert!(ipt.execute("nat", &format!("-A {} -j ACCEPT", new_name)).is_ok()); + assert!(ipt + .execute("nat", &format!("-A {} -j ACCEPT", new_name)) + .is_ok()); assert_eq!(ipt.exists("nat", new_name, "-j ACCEPT").unwrap(), true); assert_eq!(ipt.flush_chain("nat", new_name).unwrap(), true); assert_eq!(ipt.chain_exists("nat", new_name).unwrap(), true); @@ -54,14 +95,48 @@ fn filter(ipt: iptables::IPTables, name: &str) { assert_eq!(ipt.exists("filter", name, "-j ACCEPT").unwrap(), false); assert_eq!(ipt.delete("filter", name, "-j DROP").unwrap(), true); assert_eq!(ipt.list("filter", name).unwrap().len(), 1); - assert!(ipt.execute("filter", &format!("-A {} -j ACCEPT", name)).is_ok()); + assert!(ipt + .execute("filter", &format!("-A {} -j ACCEPT", name)) + .is_ok()); assert_eq!(ipt.exists("filter", name, "-j ACCEPT").unwrap(), true); - assert_eq!(ipt.append("filter", name, "-m comment --comment \"double-quoted comment\" -j ACCEPT").unwrap(), true); - assert_eq!(ipt.exists("filter", name, "-m comment --comment \"double-quoted comment\" -j ACCEPT").unwrap(), true); - assert_eq!(ipt.append("filter", name, "-m comment --comment 'single-quoted comment' -j ACCEPT").unwrap(), true); + assert_eq!( + ipt.append( + "filter", + name, + "-m comment --comment \"double-quoted comment\" -j ACCEPT" + ) + .unwrap(), + true + ); + assert_eq!( + ipt.exists( + "filter", + name, + "-m comment --comment \"double-quoted comment\" -j ACCEPT" + ) + .unwrap(), + true + ); + assert_eq!( + ipt.append( + "filter", + name, + "-m comment --comment 'single-quoted comment' -j ACCEPT" + ) + .unwrap(), + true + ); // The following `exists`-check has to use double-quotes, since the iptables output (if it // doesn't have the check-functionality) will use double quotes. - assert_eq!(ipt.exists("filter", name, "-m comment --comment \"single-quoted comment\" -j ACCEPT").unwrap(), true); + assert_eq!( + ipt.exists( + "filter", + name, + "-m comment --comment \"single-quoted comment\" -j ACCEPT" + ) + .unwrap(), + true + ); assert_eq!(ipt.flush_chain("filter", name).unwrap(), true); assert_eq!(ipt.chain_exists("filter", name).unwrap(), true); assert_eq!(ipt.delete_chain("filter", name).unwrap(), true); @@ -117,7 +192,8 @@ fn test_set_policy() { }); // Reset the policy to the retained value - ipt.set_policy("mangle", "FORWARD", ¤t_policy).unwrap(); + ipt.set_policy("mangle", "FORWARD", ¤t_policy) + .unwrap(); // "Rethrow" a potential caught panic assert!(result.is_ok()); From bfc634d3df7bf9f8a354cd564e6a9466c6b231ab Mon Sep 17 00:00:00 2001 From: Jake Pittis Date: Sun, 20 Jan 2019 10:53:08 +0000 Subject: [PATCH 2/5] Create BadExitStatus error so callers can switch on the exit status Before when the exit status was reported as a bool, callers were unable to distinguish between permission denied failures and whether or not a given rule exists. Now we use an error to represent a non-zero exit status and a bool to represent business logic like existence. --- src/error.rs | 3 +++ src/lib.rs | 59 +++++++++++++++++++++++++++++----------------------- 2 files changed, 36 insertions(+), 26 deletions(-) diff --git a/src/error.rs b/src/error.rs index 8111d1f..41e225f 100644 --- a/src/error.rs +++ b/src/error.rs @@ -10,6 +10,7 @@ pub enum IPTError { Regex(regex::Error), Nix(nix::Error), Parse(num::ParseIntError), + BadExitStatus(i32), Other(&'static str), } @@ -23,6 +24,7 @@ impl fmt::Display for IPTError { IPTError::Regex(ref err) => write!(f, "{}", err), IPTError::Nix(ref err) => write!(f, "{}", err), IPTError::Parse(ref err) => write!(f, "{}", err), + IPTError::BadExitStatus(i) => write!(f, "{}", i), IPTError::Other(ref message) => write!(f, "{}", message), } } @@ -35,6 +37,7 @@ impl error::Error for IPTError { IPTError::Regex(ref err) => err.description(), IPTError::Nix(ref err) => err.description(), IPTError::Parse(ref err) => err.description(), + IPTError::BadExitStatus(_) => "iptables exited with a non-zero status.", IPTError::Other(ref message) => message, } } diff --git a/src/lib.rs b/src/lib.rs index 0751744..873e7cc 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -156,7 +156,7 @@ impl IPTables { } /// Set the default policy for a table/chain. - pub fn set_policy(&self, table: &str, chain: &str, policy: &str) -> IPTResult { + pub fn set_policy(&self, table: &str, chain: &str, policy: &str) -> IPTResult<()> { let builtin_chains = get_builtin_chains(table)?; if !builtin_chains.iter().as_slice().contains(&chain) { return Err(IPTError::Other( @@ -165,7 +165,7 @@ impl IPTables { } match self.run(&["-t", table, "-P", chain, policy]) { - Ok(output) => Ok(output.status.success()), + Ok(_) => Ok(()), Err(err) => Err(err), } } @@ -185,7 +185,9 @@ impl IPTables { } match self.run(&[&["-t", table, "-C", chain], rule.split_quoted().as_slice()].concat()) { - Ok(output) => Ok(output.status.success()), + Ok(_) => Ok(true), + Err(IPTError::BadExitStatus(1)) => Ok(false), + Err(IPTError::BadExitStatus(2)) => Ok(false), Err(err) => Err(err), } } @@ -195,14 +197,15 @@ impl IPTables { #[cfg(target_os = "linux")] pub fn chain_exists(&self, table: &str, chain: &str) -> IPTResult { match self.run(&["-t", table, "-L", chain]) { - Ok(output) => Ok(output.status.success()), + Ok(_) => Ok(true), + Err(IPTError::BadExitStatus(1)) => Ok(false), Err(err) => Err(err), } } /// Inserts `rule` in the `position` to the table/chain. /// Returns `true` if the rule is inserted. - pub fn insert(&self, table: &str, chain: &str, rule: &str, position: i32) -> IPTResult { + pub fn insert(&self, table: &str, chain: &str, rule: &str, position: i32) -> IPTResult<()> { match self.run( &[ &["-t", table, "-I", chain, &position.to_string()], @@ -210,7 +213,7 @@ impl IPTables { ] .concat(), ) { - Ok(output) => Ok(output.status.success()), + Ok(_) => Ok(()), Err(err) => Err(err), } } @@ -223,7 +226,7 @@ impl IPTables { chain: &str, rule: &str, position: i32, - ) -> IPTResult { + ) -> IPTResult<()> { if self.exists(table, chain, rule)? { return Err(IPTError::Other("the rule exists in the table/chain")); } @@ -233,7 +236,7 @@ impl IPTables { /// Replaces `rule` in the `position` to the table/chain. /// Returns `true` if the rule is replaced. - pub fn replace(&self, table: &str, chain: &str, rule: &str, position: i32) -> IPTResult { + pub fn replace(&self, table: &str, chain: &str, rule: &str, position: i32) -> IPTResult<()> { match self.run( &[ &["-t", table, "-R", chain, &position.to_string()], @@ -241,23 +244,23 @@ impl IPTables { ] .concat(), ) { - Ok(output) => Ok(output.status.success()), + Ok(_) => Ok(()), Err(err) => Err(err), } } /// Appends `rule` to the table/chain. /// Returns `true` if the rule is appended. - pub fn append(&self, table: &str, chain: &str, rule: &str) -> IPTResult { + pub fn append(&self, table: &str, chain: &str, rule: &str) -> IPTResult<()> { match self.run(&[&["-t", table, "-A", chain], rule.split_quoted().as_slice()].concat()) { - Ok(output) => Ok(output.status.success()), + Ok(_) => Ok(()), Err(err) => Err(err), } } /// Appends `rule` to the table/chain if it does not exist. /// Returns `true` if the rule is appended. - pub fn append_unique(&self, table: &str, chain: &str, rule: &str) -> IPTResult { + pub fn append_unique(&self, table: &str, chain: &str, rule: &str) -> IPTResult<()> { if self.exists(table, chain, rule)? { return Err(IPTError::Other("the rule exists in the table/chain")); } @@ -267,7 +270,7 @@ impl IPTables { /// Appends or replaces `rule` to the table/chain if it does not exist. /// Returns `true` if the rule is appended or replaced. - pub fn append_replace(&self, table: &str, chain: &str, rule: &str) -> IPTResult { + pub fn append_replace(&self, table: &str, chain: &str, rule: &str) -> IPTResult<()> { if self.exists(table, chain, rule)? { self.delete(table, chain, rule)?; } @@ -277,9 +280,9 @@ impl IPTables { /// Deletes `rule` from the table/chain. /// Returns `true` if the rule is deleted. - pub fn delete(&self, table: &str, chain: &str, rule: &str) -> IPTResult { + pub fn delete(&self, table: &str, chain: &str, rule: &str) -> IPTResult<()> { match self.run(&[&["-t", table, "-D", chain], rule.split_quoted().as_slice()].concat()) { - Ok(output) => Ok(output.status.success()), + Ok(_) => Ok(()), Err(err) => Err(err), } } @@ -318,45 +321,45 @@ impl IPTables { /// Creates a new user-defined chain. /// Returns `true` if the chain is created. - pub fn new_chain(&self, table: &str, chain: &str) -> IPTResult { + pub fn new_chain(&self, table: &str, chain: &str) -> IPTResult<()> { match self.run(&["-t", table, "-N", chain]) { - Ok(output) => Ok(output.status.success()), + Ok(_) => Ok(()), Err(err) => Err(err), } } /// Flushes (deletes all rules) a chain. /// Returns `true` if the chain is flushed. - pub fn flush_chain(&self, table: &str, chain: &str) -> IPTResult { + pub fn flush_chain(&self, table: &str, chain: &str) -> IPTResult<()> { match self.run(&["-t", table, "-F", chain]) { - Ok(output) => Ok(output.status.success()), + Ok(_) => Ok(()), Err(err) => Err(err), } } /// Renames a chain in the table. /// Returns `true` if the chain is renamed. - pub fn rename_chain(&self, table: &str, old_chain: &str, new_chain: &str) -> IPTResult { + pub fn rename_chain(&self, table: &str, old_chain: &str, new_chain: &str) -> IPTResult<()> { match self.run(&["-t", table, "-E", old_chain, new_chain]) { - Ok(output) => Ok(output.status.success()), + Ok(_) => Ok(()), Err(err) => Err(err), } } /// Deletes a user-defined chain in the table. /// Returns `true` if the chain is deleted. - pub fn delete_chain(&self, table: &str, chain: &str) -> IPTResult { + pub fn delete_chain(&self, table: &str, chain: &str) -> IPTResult<()> { match self.run(&["-t", table, "-X", chain]) { - Ok(output) => Ok(output.status.success()), + Ok(_) => Ok(()), Err(err) => Err(err), } } /// Flushes all chains in a table. /// Returns `true` if the chains are flushed. - pub fn flush_table(&self, table: &str) -> IPTResult { + pub fn flush_table(&self, table: &str) -> IPTResult<()> { match self.run(&["-t", table, "-F"]) { - Ok(output) => Ok(output.status.success()), + Ok(_) => Ok(()), Err(err) => Err(err), } } @@ -417,6 +420,10 @@ impl IPTables { }; } - Ok(output) + match output.status.code() { + None => Ok(output), + Some(0) => Ok(output), + Some(i) => Err(IPTError::BadExitStatus(i)), + } } } From c5d97c01608d13d98014344e1798cdd9739093ab Mon Sep 17 00:00:00 2001 From: Jake Pittis Date: Sun, 5 May 2019 16:53:39 -0400 Subject: [PATCH 3/5] Use helpers rather than match expression --- src/lib.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 873e7cc..226424e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -358,10 +358,7 @@ impl IPTables { /// Flushes all chains in a table. /// Returns `true` if the chains are flushed. pub fn flush_table(&self, table: &str) -> IPTResult<()> { - match self.run(&["-t", table, "-F"]) { - Ok(_) => Ok(()), - Err(err) => Err(err), - } + self.run(&["-t", table, "-F"]).and_then(|_| Ok(())) } fn exists_old_version(&self, table: &str, chain: &str, rule: &str) -> IPTResult { From d6670394390aa9694c464a6f93ac6e6477f90209 Mon Sep 17 00:00:00 2001 From: Jake Pittis Date: Sun, 5 May 2019 17:03:08 -0400 Subject: [PATCH 4/5] Error message wasnt a valid sentence so should have a period --- src/error.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/error.rs b/src/error.rs index 41e225f..e7e1528 100644 --- a/src/error.rs +++ b/src/error.rs @@ -37,7 +37,7 @@ impl error::Error for IPTError { IPTError::Regex(ref err) => err.description(), IPTError::Nix(ref err) => err.description(), IPTError::Parse(ref err) => err.description(), - IPTError::BadExitStatus(_) => "iptables exited with a non-zero status.", + IPTError::BadExitStatus(_) => "iptables exited with a non-zero status", IPTError::Other(ref message) => message, } } From 63c81d895df31e759dc39481a3b215966aabd7e9 Mon Sep 17 00:00:00 2001 From: Jake Pittis Date: Sun, 5 May 2019 17:17:17 -0400 Subject: [PATCH 5/5] Consistently use and_then pattern accross all code --- src/lib.rs | 55 +++++++++++++++++------------------------------------- 1 file changed, 17 insertions(+), 38 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 226424e..c2dfef6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -164,10 +164,8 @@ impl IPTables { )); } - match self.run(&["-t", table, "-P", chain, policy]) { - Ok(_) => Ok(()), - Err(err) => Err(err), - } + self.run(&["-t", table, "-P", chain, policy]) + .and_then(|_| Ok(())) } /// Executes a given `command` on the chain. @@ -206,16 +204,14 @@ impl IPTables { /// Inserts `rule` in the `position` to the table/chain. /// Returns `true` if the rule is inserted. pub fn insert(&self, table: &str, chain: &str, rule: &str, position: i32) -> IPTResult<()> { - match self.run( + self.run( &[ &["-t", table, "-I", chain, &position.to_string()], rule.split_quoted().as_slice(), ] .concat(), - ) { - Ok(_) => Ok(()), - Err(err) => Err(err), - } + ) + .and_then(|_| Ok(())) } /// Inserts `rule` in the `position` to the table/chain if it does not exist. @@ -237,25 +233,21 @@ impl IPTables { /// Replaces `rule` in the `position` to the table/chain. /// Returns `true` if the rule is replaced. pub fn replace(&self, table: &str, chain: &str, rule: &str, position: i32) -> IPTResult<()> { - match self.run( + self.run( &[ &["-t", table, "-R", chain, &position.to_string()], rule.split_quoted().as_slice(), ] .concat(), - ) { - Ok(_) => Ok(()), - Err(err) => Err(err), - } + ) + .and_then(|_| Ok(())) } /// Appends `rule` to the table/chain. /// Returns `true` if the rule is appended. pub fn append(&self, table: &str, chain: &str, rule: &str) -> IPTResult<()> { - match self.run(&[&["-t", table, "-A", chain], rule.split_quoted().as_slice()].concat()) { - Ok(_) => Ok(()), - Err(err) => Err(err), - } + self.run(&[&["-t", table, "-A", chain], rule.split_quoted().as_slice()].concat()) + .and_then(|_| Ok(())) } /// Appends `rule` to the table/chain if it does not exist. @@ -281,10 +273,8 @@ impl IPTables { /// Deletes `rule` from the table/chain. /// Returns `true` if the rule is deleted. pub fn delete(&self, table: &str, chain: &str, rule: &str) -> IPTResult<()> { - match self.run(&[&["-t", table, "-D", chain], rule.split_quoted().as_slice()].concat()) { - Ok(_) => Ok(()), - Err(err) => Err(err), - } + self.run(&[&["-t", table, "-D", chain], rule.split_quoted().as_slice()].concat()) + .and_then(|_| Ok(())) } /// Deletes all repetition of the `rule` from the table/chain. @@ -322,37 +312,26 @@ impl IPTables { /// Creates a new user-defined chain. /// Returns `true` if the chain is created. pub fn new_chain(&self, table: &str, chain: &str) -> IPTResult<()> { - match self.run(&["-t", table, "-N", chain]) { - Ok(_) => Ok(()), - Err(err) => Err(err), - } + self.run(&["-t", table, "-N", chain]).and_then(|_| Ok(())) } /// Flushes (deletes all rules) a chain. /// Returns `true` if the chain is flushed. pub fn flush_chain(&self, table: &str, chain: &str) -> IPTResult<()> { - match self.run(&["-t", table, "-F", chain]) { - Ok(_) => Ok(()), - Err(err) => Err(err), - } + self.run(&["-t", table, "-F", chain]).and_then(|_| Ok(())) } /// Renames a chain in the table. /// Returns `true` if the chain is renamed. pub fn rename_chain(&self, table: &str, old_chain: &str, new_chain: &str) -> IPTResult<()> { - match self.run(&["-t", table, "-E", old_chain, new_chain]) { - Ok(_) => Ok(()), - Err(err) => Err(err), - } + self.run(&["-t", table, "-E", old_chain, new_chain]) + .and_then(|_| Ok(())) } /// Deletes a user-defined chain in the table. /// Returns `true` if the chain is deleted. pub fn delete_chain(&self, table: &str, chain: &str) -> IPTResult<()> { - match self.run(&["-t", table, "-X", chain]) { - Ok(_) => Ok(()), - Err(err) => Err(err), - } + self.run(&["-t", table, "-X", chain]).and_then(|_| Ok(())) } /// Flushes all chains in a table.