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

Validate limits #121

Merged
merged 8 commits into from
Sep 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions limitador-server/src/http_api/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,6 @@
#[allow(clippy::field_reassign_with_default)]
mod request_types;

pub use request_types::Limit as LimitVO;

pub mod server;
106 changes: 97 additions & 9 deletions limitador-server/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,16 +205,22 @@ impl Limiter {
Ok(f) => {
let parsed_limits: Result<Vec<Limit>, _> = serde_yaml::from_reader(f);
match parsed_limits {
Ok(limits) => {
match &self {
Self::Blocking(limiter) => limiter.configure_with(limits)?,
Self::Async(limiter) => limiter.configure_with(limits).await?,
}
if limitador::limit::check_deprecated_syntax_usages_and_reset() {
error!("You are using deprecated syntax for your conditions! See the migration guide https://kudrant.io/migration/limitador/constraints")
Ok(limits) => match find_first_negative_limit(&limits) {
None => {
match &self {
Self::Blocking(limiter) => limiter.configure_with(limits)?,
Self::Async(limiter) => limiter.configure_with(limits).await?,
}
if limitador::limit::check_deprecated_syntax_usages_and_reset() {
error!("You are using deprecated syntax for your conditions! See the migration guide https://kudrant.io/migration/limitador/constraints")
}
Ok(())
}
Ok(())
}
Some(index) => Err(LimitadorServerError::ConfigFile(format!(
".[{}]: invalid value for `max_value`: positive integer expected",
index
))),
},
Err(e) => Err(LimitadorServerError::ConfigFile(format!(
"Couldn't parse: {}",
e
Expand All @@ -230,6 +236,15 @@ impl Limiter {
}
}

fn find_first_negative_limit(limits: &[Limit]) -> Option<usize> {
for (index, limit) in limits.iter().enumerate() {
if limit.max_value() < 0 {
return Some(index);
}
}
None
}

#[actix_rt::main]
async fn main() -> Result<(), Box<dyn std::error::Error>> {
let config = {
Expand Down Expand Up @@ -462,6 +477,12 @@ fn create_config() -> (Configuration, String) {
.display_order(6)
.help("Sets the level of verbosity"),
)
.arg(
Arg::with_name("validate")
eguzki marked this conversation as resolved.
Show resolved Hide resolved
.long("validate")
.display_order(7)
.help("Validates the LIMITS_FILE and exits"),
)
.subcommand(
SubCommand::with_name("memory")
.display_order(1)
Expand Down Expand Up @@ -552,6 +573,47 @@ fn create_config() -> (Configuration, String) {
let matches = cmdline.get_matches();

let limits_file = matches.value_of("LIMITS_FILE").unwrap();

if matches.is_present("validate") {
let error = match std::fs::File::open(limits_file) {
Ok(f) => {
let parsed_limits: Result<Vec<Limit>, _> = serde_yaml::from_reader(f);
match parsed_limits {
Ok(limits) => match find_first_negative_limit(&limits) {
Some(index) => LimitadorServerError::ConfigFile(format!(
".[{}]: invalid value for `max_value`: positive integer expected",
index
)),
None => {
if limitador::limit::check_deprecated_syntax_usages_and_reset() {
eprintln!("Deprecated syntax for conditions corrected!\n")
}

let output: Vec<http_api::LimitVO> =
limits.iter().map(|l| l.into()).collect();
match serde_yaml::to_string(&output) {
Ok(cfg) => {
println!("{}", cfg);
}
Err(err) => {
eprintln!("Config file is valid, but can't be output: {}", err);
}
}
process::exit(0);
}
},
Err(e) => LimitadorServerError::ConfigFile(format!("Couldn't parse: {}", e)),
}
}
Err(e) => LimitadorServerError::ConfigFile(format!(
"Couldn't read file '{}': {}",
limits_file, e
)),
};
eprintln!("{}", error);
process::exit(1);
}

let storage = match matches.subcommand() {
Some(("redis", sub)) => StorageConfiguration::Redis(RedisStorageConfiguration {
url: sub.value_of("URL").unwrap().to_owned(),
Expand Down Expand Up @@ -660,3 +722,29 @@ fn env_option_is_enabled(env_name: &str) -> bool {
Err(_) => false,
}
}

#[cfg(test)]
mod tests {
use crate::find_first_negative_limit;
use limitador::limit::Limit;

#[test]
fn finds_negative_limits() {
let variables: [&str; 0] = [];
let mut limits: Vec<Limit> = vec![
Limit::new::<_, &str>("foo", 42, 10, [], variables),
Limit::new::<_, &str>("foo", -42, 10, [], variables),
];

assert_eq!(find_first_negative_limit(&limits), Some(1));
limits[0].set_max_value(-42);
assert_eq!(find_first_negative_limit(&limits), Some(0));
limits[1].set_max_value(42);
assert_eq!(find_first_negative_limit(&limits), Some(0));
limits[0].set_max_value(42);
assert_eq!(find_first_negative_limit(&limits), None);

let nothing: [Limit; 0] = [];
assert_eq!(find_first_negative_limit(&nothing), None);
}
}
11 changes: 8 additions & 3 deletions limitador/src/limit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -239,9 +239,14 @@ impl From<Condition> for String {
fn from(condition: Condition) -> Self {
let p = &condition.predicate;
let predicate: String = p.clone().into();
let quotes = if condition.operand.contains('"') {
'\''
} else {
'"'
};
format!(
"{} {} '{}'",
condition.var_name, predicate, condition.operand
"{} {} {}{}{}",
condition.var_name, predicate, quotes, condition.operand, quotes
)
}
}
Expand Down Expand Up @@ -967,6 +972,6 @@ mod tests {
operand: "ok".to_string(),
};
let result = serde_json::to_string(&condition).expect("Should serialize");
assert_eq!(result, r#""foobar == 'ok'""#.to_string());
assert_eq!(result, r#""foobar == \"ok\"""#.to_string());
}
}
2 changes: 1 addition & 1 deletion limitador/src/storage/keys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ mod tests {
vec!["app_id"],
);
assert_eq!(
"namespace:{example.com},counters_of_limit:{\"namespace\":\"example.com\",\"seconds\":60,\"conditions\":[\"req.method == 'GET'\"],\"variables\":[\"app_id\"]}",
"namespace:{example.com},counters_of_limit:{\"namespace\":\"example.com\",\"seconds\":60,\"conditions\":[\"req.method == \\\"GET\\\"\"],\"variables\":[\"app_id\"]}",
key_for_counters_of_limit(&limit))
}
}