From c7407c4f6ab559386eb09f87e250a3e44cce0e80 Mon Sep 17 00:00:00 2001
From: Ma_124 <ma_124@outlook.com>
Date: Fri, 8 Apr 2022 02:27:55 +0200
Subject: [PATCH] subscriber: fix empty string handling in
 `EnvFilter::builder().parse` (#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=
---
 tracing-subscriber/src/filter/env/builder.rs | 22 +++++++++++---------
 tracing-subscriber/src/filter/env/mod.rs     |  8 +++++++
 2 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/tracing-subscriber/src/filter/env/builder.rs b/tracing-subscriber/src/filter/env/builder.rs
index d950b1e419..36b5205431 100644
--- a/tracing-subscriber/src/filter/env/builder.rs
+++ b/tracing-subscriber/src/filter/env/builder.rs
@@ -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)
     }
 
@@ -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))
diff --git a/tracing-subscriber/src/filter/env/mod.rs b/tracing-subscriber/src/filter/env/mod.rs
index a2fc0d112b..6b1a9fa4c3 100644
--- a/tracing-subscriber/src/filter/env/mod.rs
+++ b/tracing-subscriber/src/filter/env/mod.rs
@@ -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());
+    }
 }