Skip to content

Commit

Permalink
subscriber: fix empty string handling in EnvFilter::builder().parse (
Browse files Browse the repository at this point in the history
…#2052)

## Motivation

See issue #2046. When using calling [`Builder::parse`] or
[`Builder::parse_lossy`] with  an empty string an error is produced.
This happens for example when `EnvFilter::from_default_env()` is called,
but the `RUST_LOG` variable is unset. This regression was introduced by
#2035.

## Solution

Filter any empty directives. This allows the whole string to be empty,
as well as leading and trailing commas. A unit test was added for
[`Builder::parse`], but not [`Builder::parse_lossy`] because it (per
definition) doesn't produce any side effects visible from tests when
erroring.

Fixes #2046

[`Builder::parse`]: https://github.com/tokio-rs/tracing/blob/cade7e311848227348c9b3062e4a33db827a0390/tracing-subscriber/src/filter/env/builder.rs#L151=
[`Builder::parse_lossy`]: https://github.com/tokio-rs/tracing/blob/cade7e311848227348c9b3062e4a33db827a0390/tracing-subscriber/src/filter/env/builder.rs#L135=
  • Loading branch information
Ma124 committed Apr 8, 2022
1 parent d0d5dcf commit c7407c4
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 10 deletions.
22 changes: 12 additions & 10 deletions tracing-subscriber/src/filter/env/builder.rs
Expand Up @@ -133,16 +133,17 @@ impl Builder {
/// Returns a new [`EnvFilter`] from the directives in the given string,
/// *ignoring* any that are invalid.
pub fn parse_lossy<S: AsRef<str>>(&self, dirs: S) -> EnvFilter {
let directives =
dirs.as_ref()
.split(',')
.filter_map(|s| match Directive::parse(s, self.regex) {
Ok(d) => Some(d),
Err(err) => {
eprintln!("ignoring `{}`: {}", s, err);
None
}
});
let directives = dirs
.as_ref()
.split(',')
.filter(|s| !s.is_empty())
.filter_map(|s| match Directive::parse(s, self.regex) {
Ok(d) => Some(d),
Err(err) => {
eprintln!("ignoring `{}`: {}", s, err);
None
}
});
self.from_directives(directives)
}

Expand All @@ -155,6 +156,7 @@ impl Builder {
}
let directives = dirs
.split(',')
.filter(|s| !s.is_empty())
.map(|s| Directive::parse(s, self.regex))
.collect::<Result<Vec<_>, _>>()?;
Ok(self.from_directives(directives))
Expand Down
8 changes: 8 additions & 0 deletions tracing-subscriber/src/filter/env/mod.rs
Expand Up @@ -920,4 +920,12 @@ mod tests {
[span2{bar=2 baz=false}],crate2[{quux=\"quuux\"}]=debug",
);
}

#[test]
fn parse_empty_string() {
// There is no corresponding test for [`Builder::parse_lossy`] as failed
// parsing does not produce any observable side effects. If this test fails
// check that [`Builder::parse_lossy`] is behaving correctly as well.
assert!(EnvFilter::builder().parse("").is_ok());
}
}

0 comments on commit c7407c4

Please sign in to comment.