From 98403f2e4149310c212595ad49595ae468f46aae Mon Sep 17 00:00:00 2001 From: CAD97 Date: Tue, 7 Sep 2021 19:12:32 -0500 Subject: [PATCH] implement stricter env filter parsing --- tracing-core/src/metadata.rs | 2 +- tracing-subscriber/Cargo.toml | 4 +- .../src/filter/env/directive.rs | 155 +++++++++--------- 3 files changed, 76 insertions(+), 85 deletions(-) diff --git a/tracing-core/src/metadata.rs b/tracing-core/src/metadata.rs index 9061cd3759..e5c45fab09 100644 --- a/tracing-core/src/metadata.rs +++ b/tracing-core/src/metadata.rs @@ -683,7 +683,7 @@ impl FromStr for LevelFilter { _ => None, }) .or_else(|| match from { - "" => Some(LevelFilter::ERROR), + "" => Some(LevelFilter::ERROR), // ?? this doesn't match EnvFilter empty directive s if s.eq_ignore_ascii_case("error") => Some(LevelFilter::ERROR), s if s.eq_ignore_ascii_case("warn") => Some(LevelFilter::WARN), s if s.eq_ignore_ascii_case("info") => Some(LevelFilter::INFO), diff --git a/tracing-subscriber/Cargo.toml b/tracing-subscriber/Cargo.toml index db9bf35797..4acb5a5e49 100644 --- a/tracing-subscriber/Cargo.toml +++ b/tracing-subscriber/Cargo.toml @@ -24,7 +24,7 @@ keywords = ["logging", "tracing", "metrics", "subscriber"] [features] default = ["env-filter", "smallvec", "fmt", "ansi", "chrono", "tracing-log", "json"] -env-filter = ["matchers", "regex", "lazy_static", "tracing"] +env-filter = ["matchers", "lazy_static", "tracing", "parse-env-filter"] fmt = ["registry"] ansi = ["fmt", "ansi_term"] registry = ["sharded-slab", "thread_local"] @@ -36,9 +36,9 @@ tracing-core = { path = "../tracing-core", version = "0.2" } # only required by the filter feature tracing = { optional = true, path = "../tracing", version = "0.2", default-features = false, features = ["std"] } matchers = { optional = true, version = "0.1.0" } -regex = { optional = true, version = "1", default-features = false, features = ["std"] } smallvec = { optional = true, version = "1" } lazy_static = { optional = true, version = "1" } +parse-env-filter = { optional = true, version = "0.1" } # fmt tracing-log = { path = "../tracing-log", version = "0.2", optional = true, default-features = false, features = ["log-tracer", "std"] } diff --git a/tracing-subscriber/src/filter/env/directive.rs b/tracing-subscriber/src/filter/env/directive.rs index af5d60db47..05be0d3db4 100644 --- a/tracing-subscriber/src/filter/env/directive.rs +++ b/tracing-subscriber/src/filter/env/directive.rs @@ -1,7 +1,5 @@ use super::super::level::{self, LevelFilter}; use super::{field, FieldMap, FilterVec}; -use lazy_static::lazy_static; -use regex::Regex; use std::{cmp::Ordering, error::Error, fmt, iter::FromIterator, str::FromStr}; use tracing_core::{span, Level, Metadata}; @@ -174,94 +172,87 @@ impl Match for Directive { impl FromStr for Directive { type Err = ParseError; - fn from_str(from: &str) -> Result { - lazy_static! { - static ref DIRECTIVE_RE: Regex = Regex::new( - r"(?x) - ^(?P(?i:trace|debug|info|warn|error|off|[0-5]))$ | - # ^^^. - # `note: we match log level names case-insensitively - ^ - (?: # target name or span name - (?P[\w:-]+)|(?P\[[^\]]*\]) - ){1,2} - (?: # level or nothing - =(?P(?i:trace|debug|info|warn|error|off|[0-5]))? - # ^^^. - # `note: we match log level names case-insensitively - )? - $ - " - ) - .unwrap(); - static ref SPAN_PART_RE: Regex = - Regex::new(r#"(?P[^\]\{]+)?(?:\{(?P[^\}]*)\})?"#).unwrap(); - static ref FIELD_FILTER_RE: Regex = - // TODO(eliza): this doesn't _currently_ handle value matchers that include comma - // characters. We should fix that. - Regex::new(r#"(?x) - ( - # field name - [[:word:]][[[:word:]]\.]* - # value part (optional) - (?:=[^,]+)? - ) - # trailing comma or EOS - (?:,\s?|$) - "#).unwrap(); - } - let caps = DIRECTIVE_RE.captures(from).ok_or_else(ParseError::new)?; + fn from_str(s: &str) -> Result { + let mut filters = parse_env_filter::filters(s); - if let Some(level) = caps - .name("global_level") - .and_then(|s| s.as_str().parse().ok()) - { - return Ok(Directive { - level, - ..Default::default() - }); + let filter = match (filters.next(), filters.next()) { + (Some(Ok(filter)), None) => filter, + _ => { + return Err(ParseError::new()); + } + }; + + if filter.span.is_none() && filter.level.is_none() { + return if filter.target.is_empty() { + // empty directive + Err(ParseError::new()) + } else if let Ok(level) = filter.target.parse() { + // bare level + Ok(Directive { + in_span: None, + fields: FilterVec::new(), + target: None, + level, + }) + } else { + // bare target + Ok(Directive { + in_span: None, + fields: FilterVec::new(), + target: Some(filter.target.into()), + level: LevelFilter::TRACE, + }) + }; } - let target = caps.name("target").and_then(|c| { - let s = c.as_str(); - if s.parse::().is_ok() { - None - } else { - Some(s.to_owned()) - } - }); - - let (in_span, fields) = caps - .name("span") - .and_then(|cap| { - let cap = cap.as_str().trim_matches(|c| c == '[' || c == ']'); - let caps = SPAN_PART_RE.captures(cap)?; - let span = caps.name("name").map(|c| c.as_str().to_owned()); - let fields = caps - .name("fields") - .map(|c| { - FIELD_FILTER_RE - .find_iter(c.as_str()) - .map(|c| c.as_str().parse()) - .collect::, _>>() - }) - .unwrap_or_else(|| Ok(FilterVec::new())); - Some((span, fields)) - }) - .unwrap_or_else(|| (None, Ok(FilterVec::new()))); + let (in_span, fields) = match filter.span { + None => (None, FilterVec::new()), + Some(mut filters) => match (filters.next(), filters.next()) { + (None, None) => (None, FilterVec::new()), + (Some(Ok(filter)), None) => { + let in_span = Some(filter.name).filter(|s| !s.is_empty()).map(Into::into); + let fields = match filter.fields { + None => FilterVec::new(), + Some(mut filters) => match (filters.next(), filters.next()) { + (Some(Ok(filter)), None) => std::iter::once(field::Match { + name: filter.name.to_string(), + value: filter.value.map(str::parse).transpose().map_err(|err| { + ParseError { + kind: ParseErrorKind::Field(Box::new(err)), + } + })?, + }) + .collect(), + _ => { + return Err(ParseError::new()); + } + }, + }; + + (in_span, fields) + } + // NB: we only support one in_span per directive + _ => { + return Err(ParseError::new()); + } + }, + }; - let level = caps - .name("level") - .and_then(|l| l.as_str().parse().ok()) - // Setting the target without the level enables every level for that target - .unwrap_or(LevelFilter::TRACE); + let target = Some(filter.target) + .filter(|s| !s.is_empty()) + .map(Into::into); + + let level = match filter.level { + None | Some("") => LevelFilter::TRACE, + Some(s) => s.parse()?, + }; Ok(Directive { - level, - target, in_span, - fields: fields?, + fields, + target, + level, }) } }