Skip to content

Commit

Permalink
Rewrite env filter parsing to be stricter
Browse files Browse the repository at this point in the history
  • Loading branch information
@CAD97
CAD97 committed Sep 21, 2021
1 parent 5fdbcbf commit e8a9d2f
Show file tree
Hide file tree
Showing 4 changed files with 691 additions and 118 deletions.
4 changes: 1 addition & 3 deletions tracing-subscriber/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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", "tracing"]
fmt = ["registry"]
ansi = ["fmt", "ansi_term"]
registry = ["sharded-slab", "thread_local"]
Expand All @@ -36,9 +36,7 @@ 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" }

# fmt
tracing-log = { path = "../tracing-log", version = "0.2", optional = true, default-features = false, features = ["log-tracer", "std"] }
Expand Down
123 changes: 23 additions & 100 deletions tracing-subscriber/src/filter/env/directive.rs
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
use super::super::level::{self, LevelFilter};
use super::{field, FieldMap, FilterVec};
use lazy_static::lazy_static;
use regex::Regex;
use super::{
field,
parsing::{parse_one_directive, ParseErrorKind},
FieldMap, FilterVec,
};
use std::{cmp::Ordering, error::Error, fmt, iter::FromIterator, str::FromStr};
use tracing_core::{span, Level, Metadata};

Expand All @@ -10,8 +12,8 @@ use tracing_core::{span, Level, Metadata};
#[derive(Debug, Eq, PartialEq)]
#[cfg_attr(docsrs, doc(cfg(feature = "env-filter")))]
pub struct Directive {
in_span: Option<String>,
fields: FilterVec<field::Match>,
pub(super) in_span: Option<String>,
pub(super) fields: FilterVec<field::Match>,
pub(crate) target: Option<String>,
pub(crate) level: LevelFilter,
}
Expand Down Expand Up @@ -58,13 +60,6 @@ pub struct ParseError {
kind: ParseErrorKind,
}

#[derive(Debug)]
enum ParseErrorKind {
Field(Box<dyn Error + Send + Sync>),
Level(level::ParseError),
Other,
}

impl Directive {
pub(super) fn has_name(&self) -> bool {
self.in_span.is_some()
Expand Down Expand Up @@ -176,94 +171,12 @@ impl Match for Directive {
impl FromStr for Directive {
type Err = ParseError;
fn from_str(from: &str) -> Result<Self, Self::Err> {
lazy_static! {
static ref DIRECTIVE_RE: Regex = Regex::new(
r"(?x)
^(?P<global_level>(?i:trace|debug|info|warn|error|off|[0-5]))$ |
# ^^^.
# `note: we match log level names case-insensitively
^
(?: # target name or span name
(?P<target>[\w:-]+)|(?P<span>\[[^\]]*\])
){1,2}
(?: # level or nothing
=(?P<level>(?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<name>[^\]\{]+)?(?:\{(?P<fields>[^\}]*)\})?"#).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)?;

if let Some(level) = caps
.name("global_level")
.and_then(|s| s.as_str().parse().ok())
{
return Ok(Directive {
level,
..Default::default()
});
let (parsed, after) = parse_one_directive(from);
if after != from.len() {
Err(ParseError::new())
} else {
Ok(parsed?)
}

let target = caps.name("target").and_then(|c| {
let s = c.as_str();
if s.parse::<LevelFilter>().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::<Result<FilterVec<_>, _>>()
})
.unwrap_or_else(|| Ok(FilterVec::new()));
Some((span, fields))
})
.unwrap_or_else(|| (None, Ok(FilterVec::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);

Ok(Directive {
level,
target,
in_span,
fields: fields?,
})
}
}

Expand Down Expand Up @@ -643,6 +556,9 @@ impl fmt::Display for ParseError {
ParseErrorKind::Other => f.pad("invalid filter directive"),
ParseErrorKind::Level(ref l) => l.fmt(f),
ParseErrorKind::Field(ref e) => write!(f, "invalid field filter: {}", e),
ParseErrorKind::UnexpectedSyntax(ref c) => {
write!(f, "unexpected semantic character: {}", c)
}
}
}
}
Expand All @@ -657,6 +573,7 @@ impl Error for ParseError {
ParseErrorKind::Other => None,
ParseErrorKind::Level(ref l) => Some(l),
ParseErrorKind::Field(ref n) => Some(n.as_ref()),
ParseErrorKind::UnexpectedSyntax(_) => None,
}
}
}
Expand All @@ -677,6 +594,12 @@ impl From<level::ParseError> for ParseError {
}
}

impl From<ParseErrorKind> for ParseError {
fn from(kind: ParseErrorKind) -> Self {
Self { kind }
}
}

// ===== impl DynamicMatch =====

impl CallsiteMatcher {
Expand Down Expand Up @@ -1106,7 +1029,7 @@ mod test {

#[test]
fn parse_directives_with_special_characters_in_span_name() {
let span_name = "!\"#$%&'()*+-./:;<=>?@^_`|~[}";
let span_name = r##"!#$%&'()*+-.:;<>?@^_`|~"##; // forbidden: "/[]{}=

let dirs = parse_directives(format!("target[{}]=info", span_name));
assert_eq!(dirs.len(), 1, "\nparsed: {:#?}", dirs);
Expand Down
40 changes: 25 additions & 15 deletions tracing-subscriber/src/filter/env/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,14 @@ pub use self::{
};
mod directive;
mod field;
mod parsing;

use crate::{
filter::LevelFilter,
subscribe::{Context, Subscribe},
sync::RwLock,
};
use std::{cell::RefCell, collections::HashMap, env, error::Error, fmt, str::FromStr};
use std::{cell::RefCell, collections::HashMap, env, error::Error, fmt, iter, str::FromStr};
use tracing_core::{
callsite,
collect::{Collect, Interest},
Expand Down Expand Up @@ -151,24 +152,32 @@ impl EnvFilter {
/// Returns a new `EnvFilter` from the directives in the given string,
/// ignoring any that are invalid.
pub fn new<S: AsRef<str>>(dirs: S) -> Self {
let directives = dirs.as_ref().split(',').filter_map(|s| match s.parse() {
Ok(d) => Some(d),
Err(err) => {
eprintln!("ignoring `{}`: {}", s, err);
None
let mut dirs = dirs.as_ref();
let directives = iter::from_fn(move || {
if dirs.is_empty() {
return None;
}
});
let (parsed, after) = parsing::parse_one_directive(dirs);
dirs = &dirs[after..];
Some(parsed)
})
.filter_map(Result::ok);
Self::from_directives(directives)
}

/// Returns a new `EnvFilter` from the directives in the given string,
/// or an error if any are invalid.
pub fn try_new<S: AsRef<str>>(dirs: S) -> Result<Self, ParseError> {
let directives = dirs
.as_ref()
.split(',')
.map(|s| s.parse())
.collect::<Result<Vec<_>, _>>()?;
let mut dirs = dirs.as_ref();
let directives = iter::from_fn(move || {
if dirs.is_empty() {
return None;
}
let (parsed, after) = parsing::parse_one_directive(dirs);
dirs = &dirs[after..];
Some(parsed)
})
.collect::<Result<Vec<_>, _>>()?;
Ok(Self::from_directives(directives))
}

Expand Down Expand Up @@ -679,7 +688,8 @@ mod tests {

#[test]
fn callsite_enabled_includes_span_directive_multiple_fields() {
let filter = EnvFilter::new("app[mySpan{field=\"value\",field2=2}]=debug")
let filter = EnvFilter::try_new(r#"app[mySpan{field="value",field2=2}]=debug"#)
.unwrap()
.with_collector(NoCollector);
static META: &Metadata<'static> = &Metadata::new(
"mySpan",
Expand All @@ -693,13 +703,13 @@ mod tests {
);

let interest = filter.register_callsite(META);
assert!(interest.is_never());
assert!(interest.is_sometimes());
}

#[test]
fn roundtrip() {
let f1: EnvFilter =
"[span1{foo=1}]=error,[span2{bar=2 baz=false}],crate2[{quux=\"quuux\"}]=debug"
"[span1{foo=1}]=error,[span2{bar=2, baz=false}],crate2[{quux=\"quuux\"}]=debug"
.parse()
.unwrap();
let f2: EnvFilter = format!("{}", f1).parse().unwrap();
Expand Down
Loading

0 comments on commit e8a9d2f

Please sign in to comment.